From dae7b009cd370c6816a1dd11dba1c084bf0eadcc Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 26 Aug 2021 17:12:58 +0200 Subject: [PATCH 4/6] Bug 28870: Move email address validation to a specific class method Content-Type: text/plain; charset="utf-8" To ease testing and future changes if needed. Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens --- 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 447661264b..dcf674dacd 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} @@ -185,4 +185,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 b0e7d1cfe3..1d079fdb22 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::Spec; use File::Slurp; use List::MoreUtils qw/ any /; @@ -200,7 +199,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 87eebbc263..6186d30088 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -36,6 +36,7 @@ use C4::Letters; use C4::Form::MessagingPreferences; use Koha::AuthUtils; use Koha::AuthorisedValues; +use Koha::Email; use Koha::Patron::Debarments; use Koha::Cities; use Koha::DateUtils; @@ -46,7 +47,6 @@ use Koha::Patron::Categories; use Koha::Patron::HouseboundRole; use Koha::Patron::HouseboundRoles; use Koha::Token; -use Email::Address; use Koha::SMS::Providers; use vars qw($debug); @@ -400,13 +400,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 8984486327..91b966d5f8 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; +use Koha::Email; use Koha::Libraries; use Koha::Patron::Attribute::Types; use Koha::Patron::Attributes; @@ -444,7 +444,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? @@ -470,10 +470,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 034139aa14..0489c38b41 100755 --- a/opac/opac-shareshelf.pl +++ b/opac/opac-shareshelf.pl @@ -25,7 +25,6 @@ use constant SHELVES_URL => '/cgi-bin/koha/opac-shelves.pl?display=privateshelves&viewshelf='; use CGI qw ( -utf8 ); -use Email::Address; use C4::Auth; use C4::Context; @@ -33,6 +32,7 @@ use C4::Letters; use C4::Members (); use C4::Output; +use Koha::Email; use Koha::Patrons; use Koha::Virtualshelves; use Koha::Virtualshelfshares; @@ -196,7 +196,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 030ce67e6f..0a1295d68a 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; @@ -252,3 +252,17 @@ subtest 'send_or_die() tests' => sub { is( $from, 'sender@example.com', 'If "from" is not explicitly passed, extract from Sender header' ); is( $email->header_str('Sender'), undef, 'The Sender header is unset' ); }; + +subtest 'is_valid' => sub { + plan tests => 8; + + is(Koha::Email->is_valid('Fróm '), 1); + is(Koha::Email->is_valid('from@example.com'), 1); + is(Koha::Email->is_valid(''), 1); + + is(Koha::Email->is_valid(''), 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.25.1