Attachment 124765 Details for Bug 28870 – Bug 28870: Move email address validation to a specific class method

[patch] Bug 28870: Move email address validation to a specific class method

Bug-28870-Move-email-address-validation-to-a-speci.patch (text/plain), 8.26 KB, created by Nick Clemens (kidclamp) on 2021-09-10 13:45:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2021-09-10 13:45:53 UTC

Size: 8.26 KB

patch

obsolete

>From e83c74ec47c4824ae2041fd14cbb86c00e1203bd Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Thu, 26 Aug 2021 17:12:58 +0200
>Subject: [PATCH] Bug 28870: Move email address validation to a specific class
> method
>
>To ease testing and future changes if needed.
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> Koha/Email.pm            | 19 ++++++++++++++++---
> about.pl                 |  3 +--
> members/memberentry.pl   |  8 ++++----
> opac/opac-memberentry.pl |  8 ++++----
> opac/opac-shareshelf.pl  |  4 ++--
> t/Koha/Email.t           | 16 +++++++++++++++-
> 6 files changed, 42 insertions(+), 16 deletions(-)
>
>diff --git a/Koha/Email.pm b/Koha/Email.pm
>index 4feeb0ad99..db897e0789 100644
>--- a/Koha/Email.pm
>+++ b/Koha/Email.pm
>@@ -78,7 +78,7 @@ sub create {
>     my $args = {};
>     $args->{from} = $params->{from} || C4::Context->preference('KohaAdminEmailAddress');
>     Koha::Exceptions::BadParameter->throw("Invalid 'from' parameter: ".$args->{from})
>-        unless $args->{from} =~ m/$Email::Address::mailbox/; # from is mandatory
>+        unless Koha::Email->is_valid($args->{from}); # from is mandatory
> 
>     $args->{subject} = $params->{subject} // '';
> 
>@@ -90,7 +90,7 @@ sub create {
>     }
> 
>     Koha::Exceptions::BadParameter->throw("Invalid 'to' parameter: ".$args->{to})
>-        unless $args->{to} =~ m/$Email::Address::mailbox/; # to is mandatory
>+        unless Koha::Email->is_valid($args->{to}); # to is mandatory
> 
>     my $addresses = {};
>     $addresses->{reply_to} = $params->{reply_to};
>@@ -112,7 +112,7 @@ sub create {
>         Koha::Exceptions::BadParameter->throw(
>             "Invalid '$address' parameter: " . $addresses->{$address} )
>           if $addresses->{$address}
>-            and $addresses->{$address} !~ m/$Email::Address::mailbox/;
>+            and !Koha::Email->is_valid($addresses->{$address});
>     }
> 
>     $args->{cc} = $addresses->{cc}
>@@ -174,4 +174,17 @@ sub send_or_die {
>     $self->SUPER::send_or_die($args);
> }
> 
>+=head3 is_valid
>+
>+    my $is_valid = Koha::Email->is_valid($email_address);
>+
>+Return true is the email address passed in parameter is valid following RFC 2822.
>+
>+=cut
>+
>+sub is_valid {
>+    my ( $class, $email ) = @_;
>+    return ($email =~ m{$Email::Address::mailbox}) ? 1 : 0;
>+}
>+
> 1;
>diff --git a/about.pl b/about.pl
>index b95578b4b9..55ffc379fb 100755
>--- a/about.pl
>+++ b/about.pl
>@@ -24,7 +24,6 @@ use Modern::Perl;
> 
> use CGI qw ( -utf8 );
> use DateTime::TimeZone;
>-use Email::Address;
> use File::Slurp qw( read_file );
> use List::MoreUtils qw( any );
> use Module::Load::Conditional qw( can_load );
>@@ -193,7 +192,7 @@ my $warnPrefAnonymousPatronAnonSuggestions_PatronDoesNotExist = ( $AnonymousPatr
> 
> my $warnPrefAnonymousPatronOPACPrivacy_PatronDoesNotExist = ( not $anonymous_patron and Koha::Patrons->search({ privacy => 2 })->count );
> 
>-my $warnPrefKohaAdminEmailAddress = C4::Context->preference('KohaAdminEmailAddress') !~ m/$Email::Address::mailbox/;
>+my $warnPrefKohaAdminEmailAddress = !Koha::Email->is_valid(C4::Context->preference('KohaAdminEmailAddress'));
> 
> my $c = Koha::Items->filter_by_visible_in_opac->count;
> my @warnings = C4::Context->dbh->selectrow_array('SHOW WARNINGS');
>diff --git a/members/memberentry.pl b/members/memberentry.pl
>index eda147c000..135e994d58 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -34,6 +34,7 @@ use C4::Letters qw( SendAlerts );
> use C4::Form::MessagingPreferences;
> use Koha::AuthUtils;
> use Koha::AuthorisedValues;
>+use Koha::Email;
> use Koha::Patron::Debarments qw( AddDebarment DelDebarment GetDebarments );
> use Koha::Cities;
> use Koha::DateUtils qw( dt_from_string output_pref );
>@@ -44,7 +45,6 @@ use Koha::Patron::Categories;
> use Koha::Patron::HouseboundRole;
> use Koha::Patron::HouseboundRoles;
> use Koha::Token;
>-use Email::Address;
> use Koha::SMS::Providers;
> 
> my $input = CGI->new;
>@@ -386,13 +386,13 @@ if ($op eq 'save' || $op eq 'insert'){
>   my $emailalt = $input->param('B_email');
> 
>   if ($emailprimary) {
>-      push (@errors, "ERROR_bad_email") if ($emailprimary !~ m/$Email::Address::mailbox/);
>+      push (@errors, "ERROR_bad_email") unless Koha::Email->is_valid($emailprimary);
>   }
>   if ($emailsecondary) {
>-      push (@errors, "ERROR_bad_email_secondary") if ($emailsecondary !~ m/$Email::Address::mailbox/);
>+      push (@errors, "ERROR_bad_email_secondary") unless Koha::Email->is_valid($emailsecondary);
>   }
>   if ($emailalt) {
>-      push (@errors, "ERROR_bad_email_alternative") if ($emailalt !~ m/$Email::Address::mailbox/);
>+      push (@errors, "ERROR_bad_email_alternative") unless Koha::Email->is_valid($emailalt);
>   }
> 
>   if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
>diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl
>index 5215394b96..cce33d8c8b 100755
>--- a/opac/opac-memberentry.pl
>+++ b/opac/opac-memberentry.pl
>@@ -34,8 +34,8 @@ use Koha::Patron::Consent;
> use Koha::Patron::Modification;
> use Koha::Patron::Modifications;
> use C4::Scrubber;
>-use Email::Address;
> use Koha::DateUtils qw( dt_from_string output_pref );
>+use Koha::Email;
> use Koha::Libraries;
> use Koha::Patron::Attribute::Types;
> use Koha::Patron::Attributes;
>@@ -439,7 +439,7 @@ sub CheckForInvalidFields {
>     my $borrower = shift;
>     my @invalidFields;
>     if ($borrower->{'email'}) {
>-        unless ( $borrower->{'email'} =~ m/$Email::Address::mailbox/ ) {
>+        unless ( Koha::Email->is_valid($borrower->{email}) ) {
>             push(@invalidFields, "email");
>         } elsif ( C4::Context->preference("PatronSelfRegistrationEmailMustBeUnique") ) {
>             my $patrons_with_same_email = Koha::Patrons->search( # FIXME Should be search_limited?
>@@ -465,10 +465,10 @@ sub CheckForInvalidFields {
>         delete $borrower->{'repeat_email'};
>     }
>     if ($borrower->{'emailpro'}) {
>-        push(@invalidFields, "emailpro") if ($borrower->{'emailpro'} !~ m/$Email::Address::mailbox/);
>+        push(@invalidFields, "emailpro") unless Koha::Email->is_valid($borrower->{'emailpro'});
>     }
>     if ($borrower->{'B_email'}) {
>-        push(@invalidFields, "B_email") if ($borrower->{'B_email'} !~ m/$Email::Address::mailbox/);
>+        push(@invalidFields, "B_email") unless Koha::Email->is_valid($borrower->{'B_email'});
>     }
>     if ( defined $borrower->{'password'}
>         and $borrower->{'password'} ne $borrower->{'password2'} )
>diff --git a/opac/opac-shareshelf.pl b/opac/opac-shareshelf.pl
>index 8bb7b60616..57d3538788 100755
>--- a/opac/opac-shareshelf.pl
>+++ b/opac/opac-shareshelf.pl
>@@ -25,13 +25,13 @@ use constant SHELVES_URL =>
>   '/cgi-bin/koha/opac-shelves.pl?display=privateshelves&viewshelf=';
> 
> use CGI qw ( -utf8 );
>-use Email::Address;
> 
> use C4::Auth qw( get_template_and_user );
> use C4::Context;
> use C4::Letters;
> use C4::Output qw( output_html_with_http_headers );
> 
>+use Koha::Email;
> use Koha::Patrons;
> use Koha::Virtualshelves;
> use Koha::Virtualshelfshares;
>@@ -195,7 +195,7 @@ sub process_addrlist {
>     foreach my $a (@temp) {
>         $a =~ s/^\s+//;
>         $a =~ s/\s+$//;
>-        if ( $a =~ m/$Email::Address::mailbox/ ) {
>+        if ( Koha::Email->is_valid($a) ) {
>             push @appr_addr, $a;
>         }
>         else {
>diff --git a/t/Koha/Email.t b/t/Koha/Email.t
>index 03da6eb659..f47a0908cd 100755
>--- a/t/Koha/Email.t
>+++ b/t/Koha/Email.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 3;
>+use Test::More tests => 4;
> 
> use Test::MockModule;
> use Test::Exception;
>@@ -248,3 +248,17 @@ subtest 'send_or_die() tests' => sub {
>     );
>     is( $email->header_str('Bcc'), undef, 'The Bcc header is unset' );
> };
>+
>+subtest 'is_valid' => sub {
>+    plan tests => 8;
>+
>+    is(Koha::Email->is_valid('FrÃ³m <from@example.com>'), 1);
>+    is(Koha::Email->is_valid('from@example.com'), 1);
>+    is(Koha::Email->is_valid('<from@example.com>'), 1);
>+
>+    is(Koha::Email->is_valid('<from@frÃ³m.com>'), 0); # "In accordance with RFC 822 and its descendants, this module demands that email addresses be ASCII only"
>+    isnt(Koha::Email->is_valid('@example.com'), 1);
>+    isnt(Koha::Email->is_valid('example.com'), 1);
>+    isnt(Koha::Email->is_valid('root@localhost'), 1);
>+    isnt(Koha::Email->is_valid('from'), 1);
>+};
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 28870: 124002 | 124003 | 124004 | 124117 | 124118 | 124119 | 124178 | 124179 | 124180 | 124183 | 124184 | 124185 | 124752 | 124753 | 124754 | 124755 | 124756 | 124757 | 124762 | 124763 | 124764 | 124765 | 124766 | 124767 | 127521 | 127522 | 127523 | 127524 | 127525 | 127526 | 127745 | 127746 | 127747 | 127748 | 127749 | 127750