From: Martin Renvoize Date: Fri, 22 Mar 2024 08:28:27 +0000 (+0000) Subject: Bug 1853138/Bug 1886954 - Switch to Email::Sender and Email::Address::XS (#103) X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=29135304aab305750c9b6f5449dcdf9148c47858;p=thirdparty%2Fbugzilla.git Bug 1853138/Bug 1886954 - Switch to Email::Sender and Email::Address::XS (#103) * Bug 1853138 - Switch to Email::Address::XS * Bug 1886954 - Port bug 502625 to Harmony - Replace Email::Send with Email::Sender Co-authored-by: Frédéric Buclin Co-authored-by: Dylan William Hardison Co-authored-by: Dave Miller --- diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm index 44c26a4a7..bacea397c 100644 --- a/Bugzilla/Config.pm +++ b/Bugzilla/Config.pm @@ -150,18 +150,19 @@ sub update_params { } # Old mail_delivery_method choices contained no uppercase characters - if (exists $param->{'mail_delivery_method'} - && $param->{'mail_delivery_method'} !~ /[A-Z]/) - { - my $method = $param->{'mail_delivery_method'}; - my %translation = ( - 'sendmail' => 'Sendmail', - 'smtp' => 'SMTP', - 'qmail' => 'Qmail', - 'testfile' => 'Test', - 'none' => 'None' - ); - $param->{'mail_delivery_method'} = $translation{$method}; + my $mta = $param->{'mail_delivery_method'}; + if ($mta) { + if ($mta !~ /[A-Z]/) { + my %translation = ( + 'sendmail' => 'Sendmail', + 'smtp' => 'SMTP', + 'qmail' => 'Qmail', + 'testfile' => 'Test', + 'none' => 'None'); + $param->{'mail_delivery_method'} = $translation{$mta}; + } + # This will force the parameter to be reset to its default value. + delete $param->{'mail_delivery_method'} if $param->{'mail_delivery_method'} eq 'Qmail'; } # Convert the old "ssl" parameter to the new "ssl_redirect" parameter. diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm index b01223b80..19bf9c068 100644 --- a/Bugzilla/Config/Common.pm +++ b/Bugzilla/Config/Common.pm @@ -11,7 +11,7 @@ use 5.10.1; use strict; use warnings; -use Email::Address; +use Email::Address::XS; use Socket; use Bugzilla::Util; @@ -78,7 +78,8 @@ sub check_regexp { sub check_email { my ($value) = @_; - if ($value !~ $Email::Address::mailbox) { + my ($address) = Email::Address::XS->parse($value); + if (!$address->is_valid) { return "must be a valid email address."; } return ""; diff --git a/Bugzilla/Config/MTA.pm b/Bugzilla/Config/MTA.pm index c57989ee4..eda5406ce 100644 --- a/Bugzilla/Config/MTA.pm +++ b/Bugzilla/Config/MTA.pm @@ -37,28 +37,15 @@ use warnings; use Bugzilla::Config::Common; -# Return::Value 1.666002 pollutes the error log with warnings about this -# deprecated module. We have to set NO_CLUCK = 1 before loading Email::Send -# to disable these warnings. -BEGIN { - $Return::Value::NO_CLUCK = 1; -} -use Email::Send; - our $sortkey = 1200; sub get_param_list { my $class = shift; my @param_list = ( { - name => 'mail_delivery_method', - type => 's', - - # Bugzilla is not ready yet to send mails to newsgroups, and 'IO' - # is of no use for now as we already have our own 'Test' mode. - choices => [ - grep { $_ ne 'NNTP' && $_ ne 'IO' } Email::Send->new()->all_mailers(), 'None' - ], + name => 'mail_delivery_method', + type => 's', + choices => ['Sendmail', 'SMTP', 'Test', 'None'], default => 'Sendmail', checker => \&check_mail_delivery_method }, diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm index 1b19a74b5..07ef07fde 100644 --- a/Bugzilla/Hook.pm +++ b/Bugzilla/Hook.pm @@ -1099,9 +1099,6 @@ Params: =item C - The C object that's about to be sent. -=item C - An arrayref that's passed as C to -L. - =back =head2 object_before_create diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm index e2a7d3194..4efc1becd 100644 --- a/Bugzilla/Mailer.pm +++ b/Bugzilla/Mailer.pm @@ -22,20 +22,16 @@ use Bugzilla::Util; use Date::Format qw(time2str); -use Email::Address; +use Email::Address::XS; use Email::MIME; use Encode qw(encode); use Encode::MIME::Header; use List::MoreUtils qw(none); use Try::Tiny; -# Return::Value 1.666002 pollutes the error log with warnings about this -# deprecated module. We have to set NO_CLUCK = 1 before loading Email::Send -# to disable these warnings. -BEGIN { - $Return::Value::NO_CLUCK = 1; -} -use Email::Send; +use Email::Sender::Simple qw(sendmail); +use Email::Sender::Transport::SMTP; +use Bugzilla::Sender::Transport::Sendmail; use Sys::Hostname; use Bugzilla::Version qw(vers_cmp); @@ -119,22 +115,14 @@ sub MessageToMTA { my $from = $email->header('From'); - my ($hostname, @args); - my $mailer_class = $method; + my $hostname; + my $transport; if ($method eq "Sendmail") { - $mailer_class = 'Bugzilla::Send::Sendmail'; if (ON_WINDOWS) { - $Email::Send::Sendmail::SENDMAIL = SENDMAIL_EXE; + $transport = Bugzilla::Sender::Transport::Sendmail->new({ sendmail => SENDMAIL_EXE }); } - push @args, "-i"; - - # We want to make sure that we pass *only* an email address. - if ($from) { - my ($email_obj) = Email::Address->parse($from); - if ($email_obj) { - my $from_email = $email_obj->address; - push(@args, "-f$from_email") if $from_email; - } + else { + $transport = Bugzilla::Sender::Transport::Sendmail->new(); } } else { @@ -160,22 +148,26 @@ sub MessageToMTA { } if ($method eq "SMTP") { - push @args, - Host => Bugzilla->params->{"smtpserver"}, - username => Bugzilla->params->{"smtp_username"}, - password => Bugzilla->params->{"smtp_password"}, - Hello => $hostname, - Debug => Bugzilla->params->{'smtp_debug'}; + my ($host, $port) = split(/:/, Bugzilla->params->{'smtpserver'}, 2); + $transport = Email::Sender::Transport::SMTP->new({ + host => $host, + defined($port) ? (port => $port) : (), + sasl_username => Bugzilla->params->{'smtp_username'}, + sasl_password => Bugzilla->params->{'smtp_password'}, + helo => $hostname, + ssl => Bugzilla->params->{'smtp_ssl'}, + debug => Bugzilla->params->{'smtp_debug'} + }); } - Bugzilla::Hook::process('mailer_before_send', - {email => $email, mailer_args => \@args}); + Bugzilla::Hook::process('mailer_before_send', {email => $email}); try { my $to = $email->header('to') or die qq{Unable to find "To:" address\n}; - my @recipients = Email::Address->parse($to); + my @recipients = Email::Address::XS->parse($to); die qq{Unable to parse "To:" address - $to\n} unless @recipients; die qq{Did not expect more than one "To:" address in $to\n} if @recipients > 1; + die qq{Invalid address in "To:" address - $to\n} if !$recipients[0]->is_valid; my $recipient = $recipients[0]; my $badhosts = Bugzilla::Bloomfilter->lookup("badhosts"); if ($badhosts && $badhosts->test($recipient->host)) { @@ -230,11 +222,12 @@ sub MessageToMTA { close TESTFILE; } else { - # This is useful for both Sendmail and Qmail, so we put it out here. + # This is useful for Sendmail, so we put it out here. local $ENV{PATH} = SENDMAIL_PATH; - my $mailer = Email::Send->new({mailer => $mailer_class, mailer_args => \@args}); - my $retval = $mailer->send($email); - ThrowCodeError('mail_send_error', {msg => $retval, mail => $email}) if !$retval; + eval { sendmail($email, {transport => $transport}) }; + if ($@) { + ThrowCodeError('mail_send_error', {msg => $@->message, mail => $email}); + } } # insert into email_rates diff --git a/Bugzilla/Migrate/Gnats.pm b/Bugzilla/Migrate/Gnats.pm index a562abf12..3e1359337 100644 --- a/Bugzilla/Migrate/Gnats.pm +++ b/Bugzilla/Migrate/Gnats.pm @@ -17,7 +17,7 @@ use Bugzilla::Constants; use Bugzilla::Install::Util qw(indicate_progress); use Bugzilla::Util qw(format_time trim generate_random_password); -use Email::Address; +use Email::Address::XS; use Email::MIME; use File::Basename; use IO::File; @@ -396,10 +396,10 @@ sub _get_gnats_field_data { if ($originator !~ Bugzilla->params->{emailregexp}) { # We use the raw header sometimes, because it looks like "From: user" - # which Email::Address won't parse but we can still use. + # which Email::Address::XS won't parse but we can still use. my $address = $email->header('From'); - my ($parsed) = Email::Address->parse($address); - if ($parsed) { + my ($parsed) = Email::Address::XS->parse($address); + if ($parsed->is_valid) { $address = $parsed->address; } if ($address) { diff --git a/Bugzilla/Send/Sendmail.pm b/Bugzilla/Sender/Transport/Sendmail.pm similarity index 66% rename from Bugzilla/Send/Sendmail.pm rename to Bugzilla/Sender/Transport/Sendmail.pm index fe27a0c70..decae4122 100644 --- a/Bugzilla/Send/Sendmail.pm +++ b/Bugzilla/Sender/Transport/Sendmail.pm @@ -5,60 +5,55 @@ # This Source Code Form is "Incompatible With Secondary Licenses", as # defined by the Mozilla Public License, v. 2.0. -package Bugzilla::Send::Sendmail; +package Bugzilla::Sender::Transport::Sendmail; + use 5.10.1; use strict; use warnings; -use base qw(Email::Send::Sendmail); - -use Return::Value; -use Symbol qw(gensym); +use parent qw(Email::Sender::Transport::Sendmail); -sub send { - my ($class, $message, @args) = @_; - my $mailer = $class->_find_sendmail; +use Email::Sender::Failure; - return failure "Couldn't find 'sendmail' executable in your PATH" - . " and Email::Send::Sendmail::SENDMAIL is not set" - unless $mailer; +sub send_email { + my ($self, $email, $envelope) = @_; - return failure "Found $mailer but cannot execute it" unless -x $mailer; + my $pipe = $self->_sendmail_pipe($envelope); - local $SIG{'CHLD'} = 'DEFAULT'; + my $string = $email->as_string; + $string =~ s/\x0D\x0A/\x0A/g unless $^O eq 'MSWin32'; - my $pipe = gensym; + print $pipe $string + or Email::Sender::Failure->throw("couldn't send message to sendmail: $!"); - open($pipe, '|-', "$mailer -t -oi @args") - || return failure "Error executing $mailer: $!"; - print($pipe $message->as_string) - || return failure "Error printing via pipe to $mailer: $!"; unless (close $pipe) { - return failure "error when closing pipe to $mailer: $!" if $!; + Email::Sender::Failure->throw("error when closing pipe to sendmail: $!") if $!; my ($error_message, $is_transient) = _map_exitcode($? >> 8); if (Bugzilla->get_param_with_override('use_mailer_queue')) { # Return success for errors which are fatal so Bugzilla knows to # remove them from the queue if ($is_transient) { - return failure "error when closing pipe to $mailer: $error_message"; + Email::Sender::Failure->throw( + "error when closing pipe to sendmail: $error_message"); } else { - warn "error when closing pipe to $mailer: $error_message\n"; - return success; + warn "error when closing pipe to sendmail: $error_message\n"; + return $self->success; } } else { - return failure "error when closing pipe to $mailer: $error_message"; + Email::Sender::Failure->throw( + "error when closing pipe to sendmail: $error_message"); } } - return success; + return $self->success; } sub _map_exitcode { # Returns (error message, is_transient) - # from the sendmail source (sendmail/sysexit.h) + # from the sendmail source (sendmail/sysexits.h) my $code = shift; if ($code == 64) { return ("Command line usage error (EX_USAGE)", 1); @@ -112,3 +107,10 @@ sub _map_exitcode { 1; +=head1 B + +=over + +=item send_email + +=back diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 0cd4bd6bd..14d3311f8 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -38,7 +38,7 @@ use Date::Parse; use DateTime::TimeZone; use DateTime; use Digest; -use Email::Address; +use Email::Address::XS; use Encode qw(encode decode resolve_alias); use Encode::Guess; use English qw(-no_match_vars $EGID); @@ -227,13 +227,19 @@ sub html_light_quote { sub email_filter { my ($toencode) = @_; if (!Bugzilla->user->id) { - my @emails = Email::Address->parse($toencode); - if (scalar @emails) { - my @hosts = map { quotemeta($_->host) } @emails; - my $hosts_re = join('|', @hosts); - $toencode =~ s/\@(?:$hosts_re)//g; - return $toencode; + my $email_re = qr/([a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,})/; + my @hosts; + while ($toencode =~ /$email_re/g) { + my @emails = Email::Address::XS->parse($1); + if (scalar @emails) { + my @these_hosts = map { quotemeta($_->host) } @emails; + push @hosts, @these_hosts; + } } + my $hosts_re = join('|', @hosts); + + $toencode =~ s/\@(?:$hosts_re)//g; + return $toencode; } return $toencode; } @@ -740,15 +746,10 @@ sub validate_email_syntax { my $match = Bugzilla->params->{'emailregexp'}; my $email = $addr . Bugzilla->params->{'emailsuffix'}; - # This regexp follows RFC 2822 section 3.4.1. - my $addr_spec = $Email::Address::addr_spec; - - # RFC 2822 section 2.1 specifies that email addresses must - # be made of US-ASCII characters only. - # Email::Address::addr_spec doesn't enforce this. + my $address = Email::Address::XS->parse_bare_address($email); if ( $addr =~ /$match/ + && $address->is_valid && $email !~ /\P{ASCII}/ - && $email =~ /^$addr_spec$/ && length($email) <= 127) { return 1; diff --git a/Dockerfile b/Dockerfile index 202ac9b62..6c034efb2 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM bugzilla/bugzilla-perl-slim:20240320.1 +FROM bugzilla/bugzilla-perl-slim:20240322.1 ENV DEBIAN_FRONTEND noninteractive diff --git a/Makefile.PL b/Makefile.PL index ef465954e..ae5bc4f6f 100755 --- a/Makefile.PL +++ b/Makefile.PL @@ -57,8 +57,9 @@ my %requires = ( 'Devel::NYTProf' => '6.04', 'Digest::SHA' => '5.47', 'EV' => '4.0', + 'Email::Address::XS' => '1.05', 'Email::MIME' => '1.904', - 'Email::Send' => '1.911', + 'Email::Sender' => '2.600', 'FFI::Platypus' => 0, 'Future' => '0.34', 'HTML::Escape' => '1.10', diff --git a/RELEASE_BLOCKERS.md b/RELEASE_BLOCKERS.md index 2b0bd4316..76273ba3a 100644 --- a/RELEASE_BLOCKERS.md +++ b/RELEASE_BLOCKERS.md @@ -35,6 +35,8 @@ their own bugfixes to those modules, but that’s not something we want to do upstream. Version 5.0 rewrote the email code to use currently-supported Perl modules. That needs to be ported into Harmony. +**[COMPLETED]** + # Postgresql Compatibility We suspect, but don’t know for certain, that BMO may have moved to using diff --git a/cpanfile b/cpanfile index 3b47b5846..4ec0886cf 100644 --- a/cpanfile +++ b/cpanfile @@ -22,9 +22,9 @@ requires 'DateTime::TimeZone', '2.11'; requires 'Devel::NYTProf', '6.04'; requires 'Digest::SHA', '5.47'; requires 'EV', '4.0'; -requires 'Email::Address'; +requires 'Email::Address::XS', '1.05'; requires 'Email::MIME', '1.904'; -requires 'Email::Send', '1.911'; +requires 'Email::Sender', '2.600'; requires 'FFI::Platypus'; requires 'File::Copy::Recursive'; requires 'File::MimeInfo::Magic'; diff --git a/email_in.pl b/email_in.pl index 8f94a91e5..6e052a3ec 100755 --- a/email_in.pl +++ b/email_in.pl @@ -24,7 +24,7 @@ BEGIN { } use Data::Dumper; -use Email::Address; +use Email::Address::XS; use Email::Reply qw(reply); use Email::MIME; use Email::MIME::Attachment::Stripper; @@ -121,7 +121,7 @@ sub parse_mail { %fields = %{Bugzilla::Bug::map_fields(\%fields)}; - my ($reporter) = Email::Address->parse($input_email->header('From')); + my ($reporter) = Email::Address::XS->parse($input_email->header('From')); $fields{'reporter'} = $reporter->address; # The summary line only affects us if we're doing a post_bug. diff --git a/extensions/AntiSpam/Config.pm b/extensions/AntiSpam/Config.pm index 18cd3efa2..eec37dde7 100644 --- a/extensions/AntiSpam/Config.pm +++ b/extensions/AntiSpam/Config.pm @@ -13,7 +13,7 @@ use warnings; use constant NAME => 'AntiSpam'; use constant REQUIRED_MODULES => - [{package => 'Email-Address', module => 'Email::Address', version => 0,},]; + [{package => 'Email-Address-XS', module => 'Email::Address::XS', version => 1.01,},]; use constant OPTIONAL_MODULES => []; __PACKAGE__->NAME; diff --git a/extensions/AntiSpam/Extension.pm b/extensions/AntiSpam/Extension.pm index 066a78535..fbe482bba 100644 --- a/extensions/AntiSpam/Extension.pm +++ b/extensions/AntiSpam/Extension.pm @@ -16,7 +16,7 @@ use base qw(Bugzilla::Extension); use Bugzilla::Error; use Bugzilla::Group; use Bugzilla::Util qw(remote_ip); -use Email::Address; +use Email::Address::XS; use Socket; our $VERSION = '1'; @@ -80,7 +80,7 @@ sub _comment_blocking { sub _domain_blocking { my ($self, $login) = @_; - my $address = Email::Address->new(undef, $login); + my $address = Email::Address::XS->new(address => $login); my $blocked = Bugzilla->dbh->selectrow_array( "SELECT 1 FROM antispam_domain_blocklist WHERE domain=?", diff --git a/extensions/UserProfile/Extension.pm b/extensions/UserProfile/Extension.pm index 55ac69e28..2bfc376f6 100644 --- a/extensions/UserProfile/Extension.pm +++ b/extensions/UserProfile/Extension.pm @@ -18,7 +18,7 @@ use Bugzilla::Extension::UserProfile::Util; use Bugzilla::Install::Filesystem; use Bugzilla::User; use Bugzilla::Util qw(datetime_from time_ago); -use Email::Address; +use Email::Address::XS; use Scalar::Util qw(blessed); use List::MoreUtils qw(any); @@ -42,9 +42,7 @@ sub _user_last_statistics_ts { $_[0]->{last_statistics_ts} } sub _user_address { my $mode = Bugzilla->usage_mode; - Email::Address->disable_cache - if any { $mode == $_ } USAGE_MODE_CMDLINE, USAGE_MODE_TEST, USAGE_MODE_EMAIL; - return Email::Address->new(undef, $_[0]->email); + return Email::Address->new(address => $_[0]->email); } sub _user_set_last_activity_ts { @@ -72,7 +70,8 @@ sub _user_clear_last_statistics_ts { # hooks # -sub request_cleanup { Email::Address->purge_cache } +# noop - Email::Address::XS no longer caches +sub request_cleanup {} sub bug_after_create { my ($self, $args) = @_; diff --git a/template/en/default/admin/params/mta.html.tmpl b/template/en/default/admin/params/mta.html.tmpl index 88d8ef981..553141ea5 100644 --- a/template/en/default/admin/params/mta.html.tmpl +++ b/template/en/default/admin/params/mta.html.tmpl @@ -27,7 +27,7 @@ mail_delivery_method => "Defines how email is sent, or if it is sent at all.
  • - 'Sendmail', 'SMTP' and 'Qmail' are all MTAs. + 'Sendmail' and 'SMTP' are both MTAs. You need to install a third-party sendmail replacement if you want to use sendmail on Windows.
    • @@ -36,7 +36,7 @@ in 'data/mailer.testfile' instead of being sent.
  • - 'none' will completely disable email. $terms.Bugzilla continues + 'None' will completely disable email. Bugzilla continues to act as though it is sending mail, but nothing is sent or stored.