Bug 353711: Move to Email:: modules for email sending

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=glob, a=myk

8 files changed, 94 insertions, 179 deletions

diff --git a/Bugzilla.pm b/Bugzilla.pm
index 374b06e12..5a9d23e14 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -81,6 +81,9 @@ sub init_page {
 
     # Some environment variables are not taint safe
     delete @::ENV{'PATH', 'IFS', 'CDPATH', 'ENV', 'BASH_ENV'};
+    # Some modules throw undefined errors (notably File::Spec::Win32) if
+    # PATH is undefined.
+    $ENV{'PATH'} = '';
 
     # If Bugzilla is shut down, do not allow anything to run, just display a
     # message to the user about the downtime and log out.  Scripts listed in 
diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm
index 1423c1196..0859c5bee 100644
--- a/Bugzilla/Config.pm
+++ b/Bugzilla/Config.pm
@@ -171,6 +171,19 @@ sub update_params {
         delete $param->{'enablequips'};
     }
 
+    # 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};
+    }
+
     # --- DEFAULTS FOR NEW PARAMS ---
 
     _load_params unless %params;
@@ -216,7 +229,7 @@ sub update_params {
     }
 
     if (ON_WINDOWS && !-e SENDMAIL_EXE
-        && $param->{'mail_delivery_method'} eq 'sendmail')
+        && $param->{'mail_delivery_method'} eq 'Sendmail')
     {
         my $smtp = $answer->{'SMTP_SERVER'};
         if (!$smtp) {
@@ -233,7 +246,7 @@ sub update_params {
             }
         }
 
-        $param->{'mail_delivery_method'} = 'smtp';
+        $param->{'mail_delivery_method'} = 'SMTP';
     }
 
     write_params($param);
diff --git a/Bugzilla/Config/MTA.pm b/Bugzilla/Config/MTA.pm
index 53340dc13..a9bc4619c 100644
--- a/Bugzilla/Config/MTA.pm
+++ b/Bugzilla/Config/MTA.pm
@@ -34,6 +34,7 @@ package Bugzilla::Config::MTA;
 use strict;
 
 use Bugzilla::Config::Common;
+use Email::Send;
 
 $Bugzilla::Config::MTA::sortkey = "10";
 
@@ -43,10 +44,8 @@ sub get_param_list {
   {
    name => 'mail_delivery_method',
    type => 's',
-   choices => $^O =~ /MSWin32/i 
-                  ? ['smtp', 'testfile', 'sendmail', 'none']
-                  : ['sendmail', 'smtp', 'qmail', 'testfile', 'none'],
-   default => 'sendmail',
+   choices => [Email::Send->new()->all_mailers(), 'None'],
+   default => 'Sendmail',
    checker => \&check_mail_delivery_method
   },
 
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index b8171d1c1..9aefea429 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -103,6 +103,7 @@ use File::Basename;
     ADMIN_GROUP_NAME
 
     SENDMAIL_EXE
+    SENDMAIL_PATH
 
     FIELD_TYPE_UNKNOWN
     FIELD_TYPE_FREETEXT
@@ -290,6 +291,8 @@ use constant ADMIN_GROUP_NAME => 'admin';
 
 # Path to sendmail.exe (Windows only)
 use constant SENDMAIL_EXE => '/usr/lib/sendmail.exe';
+# Paths to search for the sendmail binary (non-Windows)
+use constant SENDMAIL_PATH => '/usr/lib:/usr/sbin:/usr/ucblib';
 
 # Field types.  Match values in fielddefs.type column.  These are purposely
 # not named after database column types, since Bugzilla fields comprise not
diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index 153149a08..2ecbc9e3c 100644
--- a/Bugzilla/Install/Requirements.pm
+++ b/Bugzilla/Install/Requirements.pm
@@ -79,11 +79,6 @@ sub REQUIRED_MODULES {
         version => '2.12'
     },
     {
-        package => 'MailTools',
-        module  => 'Mail::Mailer',
-        version => '1.67'
-    },
-    {
         package => 'MIME-Base64',
         module  => 'MIME::Base64',
         version => '3.01'
@@ -94,6 +89,17 @@ sub REQUIRED_MODULES {
         module  => ON_WINDOWS ? 'MIME::Tools' : 'MIME::Parser',
         version => '5.406'
     },
+    {
+        package => 'Email-Send',
+        module  => 'Email::Send',
+        version => ON_WINDOWS ? '2.16' : '2.00'
+    },
+    {
+        # This will pull in Email::MIME for us, also. 
+        package => 'Email-MIME-Modifier',
+        module  => 'Email::MIME::Modifier',
+        version => 0
+    },
     );
 
     my $all_modules = _get_extension_requirements(
@@ -187,15 +193,6 @@ sub OPTIONAL_MODULES {
 
     # Inbound Email
     {
-        # Email::MIME::Attachment::Stripper can throw an error with
-        # earlier versions.
-        # This also pulls in Email::MIME and Email::Address for us.
-        package => 'Email-MIME-Modifier',
-        module  => 'Email::MIME::Modifier',
-        version => '1.43',
-        feature => 'Inbound Email'
-    },
-    {
         package => 'Email-MIME-Attachment-Stripper',
         module  => 'Email::MIME::Attachment::Stripper',
         version => 0,
diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm
index 2105d38ae..633964b33 100644
--- a/Bugzilla/Mailer.pm
+++ b/Bugzilla/Mailer.pm
@@ -28,6 +28,7 @@
 #                 Gervase Markham <gerv@gerv.net>
 #                 Byron Jones <bugzilla@glob.com.au>
 #                 Frédéric Buclin <LpSolit@gmail.com>
+#                 Max Kanat-Alexander <mkanat@bugzilla.org>
 
 package Bugzilla::Mailer;
 
@@ -37,181 +38,76 @@ use base qw(Exporter);
 @Bugzilla::Mailer::EXPORT = qw(MessageToMTA);
 
 use Bugzilla::Constants;
+use Bugzilla::Error;
 use Bugzilla::Util;
 
-use Mail::Header;
-use Mail::Mailer;
-use Mail::Address;
-use MIME::Parser;
-use MIME::QuotedPrint;
-use MIME::Base64;
-
+use Encode qw(encode);
+use Email::MIME;
+# Loading this gives us encoding_set.
+use Email::MIME::Modifier;
+use Email::Send;
 
 sub MessageToMTA {
     my ($msg) = (@_);
-    my $params = Bugzilla->params;
-    return if ($params->{'mail_delivery_method'} eq "none");
-
-    my ($header, $body) = $msg =~ /(.*?\n)\n(.*)/s ? ($1, $2) : ('', $msg);
-    my $headers;
+    my $method = Bugzilla->params->{'mail_delivery_method'};
+    return if $method eq 'None';
 
-    if ($params->{'utf8'} 
-        and (!is_7bit_clean($header) or !is_7bit_clean($body))) 
-    {
-        ($headers, $body) = encode_message($msg);
-    } else {
-        my @header_lines = split(/\n/, $header);
-        $headers = new Mail::Header \@header_lines, Modify => 0;
+    my $email = ref($msg) ? $msg : Email::MIME->new($msg);
+    foreach my $part ($email->parts) {
+        $part->charset_set('UTF-8') if Bugzilla->params->{'utf8'};
+        $part->encoding_set('quoted-printable') if !is_7bit_clean($part->body);
     }
 
-    # Use trim to remove any whitespace (incl. newlines)
-    my $from = trim($headers->get('from'));
-
-    if ($params->{"mail_delivery_method"} eq "sendmail" && $^O =~ /MSWin32/i) {
-        my $cmd = '|' . SENDMAIL_EXE . ' -t -i';
-        if ($from) {
-            # We're on Windows, thus no danger of command injection
-            # via $from. In other words, it is safe to embed $from.
-            $cmd .= qq# -f"$from"#;
+    # Encode the headers correctly in quoted-printable
+    foreach my $header qw(From To Cc Reply-To Sender Errors-To Subject) {
+        if (my $value = $email->header($header)) {
+            my $encoded = encode('MIME-Q', $value);
+            $email->header_set($header, $encoded);
         }
-        open(SENDMAIL, $cmd) ||
-            die "Failed to execute " . SENDMAIL_EXE . ": $!\n";
-        print SENDMAIL $headers->as_string;
-        print SENDMAIL "\n";
-        print SENDMAIL $body;
-        close SENDMAIL;
-        return;
     }
 
-    my @args;
-    if ($params->{"mail_delivery_method"} eq "sendmail") {
-        push @args, "-i";
-        if ($from) {
-            push(@args, "-f$from");
-        }
-    }
-    if ($params->{"mail_delivery_method"} eq "sendmail" 
-        && !$params->{"sendmailnow"}) 
-    {
-        push @args, "-ODeliveryMode=deferred";
-    }
-    if ($params->{"mail_delivery_method"} eq "smtp") {
-        push @args, Server => $params->{"smtpserver"};
-        if ($from) {
-            $ENV{'MAILADDRESS'} = $from;
+    my $from = $email->header('From');
+
+    my ($hostname, @args);
+    if ($method eq "Sendmail") {
+        if (ON_WINDOWS) {
+            $Email::Send::Sendmail::SENDMAIL = SENDMAIL_EXE;
         }
+        push @args, "-i";
+        push(@args, "-f$from") if $from;
+        push(@args, "-ODeliveryMode=deferred")
+            if !Bugzilla->params->{"sendmailnow"};
     }
-    my $mailer = new Mail::Mailer($params->{"mail_delivery_method"}, @args);
-    if ($params->{"mail_delivery_method"} eq "testfile") {
-        $Mail::Mailer::testfile::config{outfile} = 
-            bz_locations()->{'datadir'} . '/mailer.testfile';
-    }
-    
-    $mailer->open($headers->header_hashref);
-    print $mailer $body;
-    $mailer->close;
-}
-
-sub encode_message {
-    my ($msg) = @_;
-
-    my $parser = MIME::Parser->new;
-    $parser->output_to_core(1);
-    $parser->tmp_to_core(1);
-    my $entity = $parser->parse_data($msg);
-    $entity = encode_message_entity($entity);
-
-    my @header_lines = split(/\n/, $entity->header_as_string);
-    my $head = new Mail::Header \@header_lines, Modify => 0;
-
-    my $body = $entity->body_as_string;
-
-    return ($head, $body);
-}
-
-sub encode_message_entity {
-    my ($entity) = @_;
-
-    my $head = $entity->head;
-
-    # encode the subject
-
-    my $subject = $head->get('subject');
-    if (defined $subject && !is_7bit_clean($subject)) {
-        $subject =~ s/[\r\n]+$//;
-        $head->replace('subject', encode_qp_words($subject));
+    else {
+        # Sendmail will automatically append our hostname to the From
+        # address, but other mailers won't.
+        my $urlbase = Bugzilla->params->{'urlbase'};
+        $urlbase =~ m|//([^/]+)/?|;
+        $hostname = $1;
+        $from .= "\@$hostname" if $from !~ /@/;
+        $email->header_set('From', $from);
     }
 
-    # encode addresses
-
-    foreach my $field (qw(from to cc reply-to sender errors-to)) {
-        my $high = $head->count($field) - 1;
-        foreach my $index (0..$high) {
-            my $value = $head->get($field, $index);
-            my @addresses;
-            my $changed = 0;
-            foreach my $addr (Mail::Address->parse($value)) {
-                my $phrase = $addr->phrase;
-                if (is_7bit_clean($phrase)) {
-                    push @addresses, $addr->format;
-                } else {
-                    push @addresses, encode_qp_phrase($phrase) . 
-                        ' <' . $addr->address . '>';
-                    $changed = 1;
-                }
-            }
-            $changed && $head->replace($field, join(', ', @addresses), $index);
-        }
+    if ($method eq "SMTP") {
+        push @args, Host  => Bugzilla->params->{"smtpserver"},
+                    Hello => $hostname;
     }
 
-    # process the body
-
-    if (scalar($entity->parts)) {
-        my $newparts = [];
-        foreach my $part ($entity->parts) {
-            my $newpart = encode_message_entity($part);
-            push @$newparts, $newpart;
-        }
-        $entity->parts($newparts);
+    if ($method eq "Test") {
+        my $filename = bz_locations()->{'datadir'} . '/mailer.testfile';
+        open TESTFILE, '>>', $filename;
+        print TESTFILE "\n\n---\n\n" . $email->as_string;
+        close TESTFILE;
     }
     else {
-        # Extract the body from the entity, for examination
-        # At this point, we can rely on MIME::Tools to do our encoding for us!
-        my $bodyhandle = $entity->bodyhandle;
-        my $body = $bodyhandle->as_string;
-        if (!is_7bit_clean($body)) {
-            # count number of 7-bit chars, and use quoted-printable if more
-            # than half the message is 7-bit clean
-            my $count = ($body =~ tr/\x20-\x7E\x0A\x0D//);
-            if ($count > length($body) / 2) {
-                $head->mime_attr('Content-Transfer-Encoding' => 'quoted-printable');
-            } else {
-                $head->mime_attr('Content-Transfer-Encoding' => 'base64');
-            }
-        }
-
-        # Set the content/type and charset of the part, if not set
-        $head->mime_attr('Content-Type' => 'text/plain')
-            unless defined $head->mime_attr('content-type');
-        $head->mime_attr('Content-Type.charset' => 'UTF-8');
-    }
-
-    $head->mime_attr('MIME-Version' => '1.0');
-    $head->fold(75);
-    return $entity;
-}
-
-sub encode_qp_words {
-    my ($line) = (@_);
-    my @encoded;
-    foreach my $word (split / /, $line) {
-        if (!is_7bit_clean($word)) {
-            push @encoded, '=?UTF-8?Q?_' . encode_qp($word, '') . '?=';
-        } else {
-            push @encoded, $word;
-        }
+        # This is useful for both Sendmail and Qmail, so we put it out here.
+        local $ENV{PATH} = SENDMAIL_PATH;
+        my $mailer = Email::Send->new({ mailer => $method, 
+                                        mailer_args => \@args });
+        my $retval = $mailer->send($email);
+        ThrowCodeError('mail_send_error', { msg => $retval, mail => $email })
+            if !$retval;
     }
-    return join(' ', @encoded);
 }
 
 1;
diff --git a/template/en/default/admin/params/mta.html.tmpl b/template/en/default/admin/params/mta.html.tmpl
index 4089820b5..224d21544 100644
--- a/template/en/default/admin/params/mta.html.tmpl
+++ b/template/en/default/admin/params/mta.html.tmpl
@@ -28,14 +28,13 @@
   mail_delivery_method => "Defines how email is sent, or if it is sent at all.<br>
                            <ul>
                              <li>
-                               'sendmail', 'smtp' and 'qmail' are all MTAs.
+                               'Sendmail', 'SMTP' and 'Qmail' are all MTAs.
                                You need to install a third-party sendmail replacement if
                                you want to use sendmail on Windows.
                              </li>
                              <li>
-                               'testfile' is useful for debugging: all email is stored
-                               in 'data/mailer.testfile' instead of being sent. For more
-                               information, see the Mail::Mailer manual.
+                               'Test' is useful for debugging: all email is stored
+                               in 'data/mailer.testfile' instead of being sent.
                              </li>
                              <li>
                                'none' will completely disable email. $terms.Bugzilla continues
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index f6ccae754..a0e9bd9b9 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -295,6 +295,11 @@
   [% ELSIF error == "ldap_server_not_defined" %]
     The LDAP server for authentication has not been defined.
 
+  [% ELSIF error == "mail_send_error" %]
+    There was an error sending mail from '[% mail.header('From') FILTER html %]'
+    to '[% mail.header('To') FILTER html %]':
+    [% msg FILTER html %]
+
   [% ELSIF error == "missing_bug_id" %]
     No [% terms.bug %] ID was given.