Bug 714724: Correctly encode emails as quoted-printable

Bugzilla/BugMail.pm

@@ -19,6 +19,7 @@ use Bugzilla::Bug;
 use Bugzilla::Comment;
 use Bugzilla::Mailer;
 use Bugzilla::Hook;
+use Bugzilla::MIME;
 
 use Date::Parse;
 use Date::Format;
@@ -435,6 +436,7 @@ sub _generate_bugmail {
     my $user = $vars->{to_user};
     my $template = Bugzilla->template_inner($user->setting('lang'));
     my ($msg_text, $msg_html, $msg_header);
+    state $use_utf8 = Bugzilla->params->{'utf8'};
 
     $template->process("email/bugmail-header.txt.tmpl", $vars, \$msg_header)
         || ThrowTemplateError($template->error());
@@ -442,32 +444,35 @@ sub _generate_bugmail {
         || ThrowTemplateError($template->error());
 
     my @parts = (
-        Email::MIME->create(
+        Bugzilla::MIME->create(
             attributes => {
-                content_type => "text/plain",
+                content_type => 'text/plain',
+                charset      => $use_utf8 ? 'UTF-8' : 'iso-8859-1',
+                encoding     => 'quoted-printable',
             },
-            body => $msg_text,
+            body_str => $msg_text,
         )
     );
     if ($user->setting('email_format') eq 'html') {
         $template->process("email/bugmail.html.tmpl", $vars, \$msg_html)
             || ThrowTemplateError($template->error());
-        push @parts, Email::MIME->create(
+        push @parts, Bugzilla::MIME->create(
             attributes => {
-                content_type => "text/html",         
+                content_type => 'text/html',
+                charset      => $use_utf8 ? 'UTF-8' : 'iso-8859-1',
+                encoding     => 'quoted-printable',
             },
-            body => $msg_html,
+            body_str => $msg_html,
         );
     }
 
-    # TT trims the trailing newline, and threadingmarker may be ignored.
-    my $email = new Email::MIME("$msg_header\n");
+    my $email = Bugzilla::MIME->new($msg_header);
     if (scalar(@parts) == 1) {
         $email->content_type_set($parts[0]->content_type);
     } else {
         $email->content_type_set('multipart/alternative');
         # Some mail clients need same encoding for each part, even empty ones.
-        $email->charset_set('UTF-8') if Bugzilla->params->{'utf8'};
+        $email->charset_set('UTF-8') if $use_utf8;
     }
     $email->parts_set(\@parts);
     return $email;

Bugzilla/MIME.pm

+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+package Bugzilla::MIME;
+
+use 5.10.1;
+use strict;
+use warnings;
+
+use parent qw(Email::MIME);
+
+use Encode qw(encode);
+use Encode::MIME::Header;
+
+sub new {
+    my ($class, $msg) = @_;
+    state $use_utf8 = Bugzilla->params->{'utf8'};
+
+    # Template-Toolkit trims trailing newlines, which is problematic when
+    # parsing headers.
+    $msg =~ s/\n*$/\n/;
+
+    # Because the encoding headers are not present in our email templates, we
+    # need to treat them as binary UTF-8 when parsing.
+    my ($in_header, $has_type, $has_encoding, $has_body) = (1);
+    foreach my $line (split(/\n/, $msg)) {
+        if ($line eq '') {
+            $in_header = 0;
+            next;
+        }
+        if (!$in_header) {
+            $has_body = 1;
+            last;
+        }
+        $has_type = 1 if $line =~ /^Content-Type:/i;
+        $has_encoding = 1 if $line =~ /^Content-Transfer-Encoding:/i;
+    }
+    if ($has_body) {
+        if (!$has_type && $use_utf8) {
+            $msg = qq#Content-Type: text/plain; charset="UTF-8"\n# . $msg;
+        }
+        if (!$has_encoding) {
+            $msg = qq#Content-Transfer-Encoding: binary\n# . $msg;
+        }
+    }
+    if ($use_utf8 && utf8::is_utf8($msg)) {
+        utf8::encode($msg);
+    }
+
+    # RFC 2822 requires us to have CRLF for our line endings and
+    # Email::MIME doesn't do this for us. We use \015 (CR) and \012 (LF)
+    # directly because Perl translates "\n" depending on what platform
+    # you're running on. See http://perldoc.perl.org/perlport.html#Newlines
+    $msg =~ s/(?:\015+)?\012/\015\012/msg;
+
+    return $class->SUPER::new($msg);
+}
+
+sub as_string {
+    my $self = shift;
+    state $use_utf8 = Bugzilla->params->{'utf8'};
+
+    # We add this header to uniquely identify all email that we
+    # send as coming from this Bugzilla installation.
+    #
+    # We don't use correct_urlbase, because we want this URL to
+    # *always* be the same for this Bugzilla, in every email,
+    # even if the admin changes the "ssl_redirect" parameter some day.
+    $self->header_set('X-Bugzilla-URL', Bugzilla->params->{'urlbase'});
+
+    # We add this header to mark the mail as "auto-generated" and
+    # thus to hopefully avoid auto replies.
+    $self->header_set('Auto-Submitted', 'auto-generated');
+
+    # MIME-Version must be set otherwise some mailsystems ignore the charset
+    $self->header_set('MIME-Version', '1.0') if !$self->header('MIME-Version');
+
+    # Encode the headers correctly in quoted-printable
+    foreach my $header ($self->header_names) {
+        my @values = $self->header($header);
+        # We don't recode headers that happen multiple times.
+        next if scalar(@values) > 1;
+        if (my $value = $values[0]) {
+            utf8::decode($value) unless $use_utf8 && utf8::is_utf8($value);
+
+            # avoid excessive line wrapping done by Encode.
+            local $Encode::Encoding{'MIME-Q'}->{'bpl'} = 998;
+
+            my $encoded = encode('MIME-Q', $value);
+            $self->header_set($header, $encoded);
+        }
+    }
+
+    # Ensure the character-set and encoding is set correctly on single part
+    # emails.  Multipart emails should have these already set when the parts
+    # are assembled.
+    if (scalar($self->parts) == 1) {
+        $self->charset_set('UTF-8') if $use_utf8;
+        $self->encoding_set('quoted-printable');
+    }
+
+    # Ensure we always return the encoded string
+    my $value = $self->SUPER::as_string();
+    if ($use_utf8 && utf8::is_utf8($value)) {
+        utf8::encode($value);
+    }
+
+    return $value;
+}
+
+1;
+
+__END__
+
+=head1 NAME
+
+Bugzilla::MIME - Wrapper around Email::MIME for unifying MIME related
+workarounds.
+
+=head1 SYNOPSIS
+
+  use Bugzilla::MIME;
+  my $email = Bugzilla::MIME->new($message);
+
+=head1 DESCRIPTION
+
+Bugzilla::MIME subclasses Email::MIME and performs various fixes when parsing
+and generating email.

Bugzilla/Mailer.pm

@@ -17,13 +17,11 @@ use parent qw(Exporter);
 use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Hook;
+use Bugzilla::MIME;
 use Bugzilla::Util;
 
 use Date::Format qw(time2str);
 
-use Encode qw(encode);
-use Encode::MIME::Header;
-use Email::MIME;
 use Email::Sender::Simple qw(sendmail);
 use Email::Sender::Transport::SMTP::Persistent;
 use Bugzilla::Sender::Transport::Sendmail;
@@ -43,18 +41,7 @@ sub MessageToMTA {
 
     my $dbh = Bugzilla->dbh;
 
-    my $email;
-    if (ref $msg) {
-        $email = $msg;
-    }
-    else {
-        # RFC 2822 requires us to have CRLF for our line endings and
-        # Email::MIME doesn't do this for us. We use \015 (CR) and \012 (LF)
-        # directly because Perl translates "\n" depending on what platform
-        # you're running on. See http://perldoc.perl.org/perlport.html#Newlines
-        $msg =~ s/(?:\015+)?\012/\015\012/msg;
-        $email = new Email::MIME($msg);
-    }
+    my $email = ref($msg) ? $msg : Bugzilla::MIME->new($msg);
 
     # If we're called from within a transaction, we don't want to send the
     # email immediately, in case the transaction is rolled back. Instead we
@@ -71,39 +58,6 @@ sub MessageToMTA {
         return;
     }
 
-    # We add this header to uniquely identify all email that we
-    # send as coming from this Bugzilla installation.
-    #
-    # We don't use correct_urlbase, because we want this URL to
-    # *always* be the same for this Bugzilla, in every email,
-    # even if the admin changes the "ssl_redirect" parameter some day.
-    $email->header_set('X-Bugzilla-URL', Bugzilla->params->{'urlbase'});
-
-    # We add this header to mark the mail as "auto-generated" and
-    # thus to hopefully avoid auto replies.
-    $email->header_set('Auto-Submitted', 'auto-generated');
-
-    # MIME-Version must be set otherwise some mailsystems ignore the charset
-    $email->header_set('MIME-Version', '1.0') if !$email->header('MIME-Version');
-
-    # Encode the headers correctly in quoted-printable
-    foreach my $header ($email->header_names) {
-        my @values = $email->header($header);
-        # We don't recode headers that happen multiple times.
-        next if scalar(@values) > 1;
-        if (my $value = $values[0]) {
-            if (Bugzilla->params->{'utf8'} && !utf8::is_utf8($value)) {
-                utf8::decode($value);
-            }
-
-            # avoid excessive line wrapping done by Encode.
-            local $Encode::Encoding{'MIME-Q'}->{'bpl'} = 998;
-
-            my $encoded = encode('MIME-Q', $value);
-            $email->header_set($header, $encoded);
-        }
-    }
-
     my $from = $email->header('From');
 
     my $hostname;
@@ -148,29 +102,6 @@ sub MessageToMTA {
 
     return if $email->header('to') eq '';
 
-    $email->walk_parts(sub {
-        my ($part) = @_;
-        return if $part->parts > 1; # Top-level
-        my $content_type = $part->content_type || '';
-        $content_type =~ /charset=['"](.+)['"]/;
-        # If no charset is defined or is the default us-ascii,
-        # then we encode the email to UTF-8 if Bugzilla has utf8 enabled.
-        # XXX - This is a hack to workaround bug 723944.
-        if (!$1 || $1 eq 'us-ascii') {
-            my $body = $part->body;
-            if (Bugzilla->params->{'utf8'}) {
-                $part->charset_set('UTF-8');
-                # encoding_set works only with bytes, not with utf8 strings.
-                my $raw = $part->body_raw;
-                if (utf8::is_utf8($raw)) {
-                    utf8::encode($raw);
-                    $part->body_set($raw);
-                }
-            }
-            $part->encoding_set('quoted-printable') if !is_7bit_clean($body);
-        }
-    });
-
     if ($method eq "Test") {
         my $filename = bz_locations()->{'datadir'} . '/mailer.testfile';
         open TESTFILE, '>>', $filename;

t/011pod.t

@@ -35,6 +35,7 @@ use constant SUB_WHITELIST => (
     'Bugzilla::JobQueue' => qr/(?:^work_once|work_until_done|subprocess_worker)$/,
     'Bugzilla::Search'   => qr/^SPECIAL_PARSING$/,
     'Bugzilla::Template' => qr/^field_name$/,
+    'Bugzilla::MIME'     => qr/^as_string$/,
 );
 
 # These modules do not need to be documented, generally because they

template/en/default/whine/multipart-mime.txt.tmpl → template/en/default/whine/header.txt.tmpl

@@ -8,10 +8,6 @@
 
 [%# INTERFACE:
   # subject: subject line of message
-  # alternatives: array of hashes containing:
-  #     type: MIME type
-  #     content: verbatim content
-  # boundary: a string that has been generated to be a unique boundary
   # recipient: user object for the intended recipient of the message
   # from: Bugzilla system email address
   #%]
@@ -19,21 +15,5 @@
 From: [% from %]
 To: [% recipient.email %]
 Subject: [[% terms.Bugzilla %]] [% subject %]
-MIME-Version: 1.0
-Content-Type: multipart/alternative; boundary="[% boundary %]"
 X-Bugzilla-Type: whine
 
-
-This is a MIME multipart message.  It is possible that your mail program
-doesn't quite handle these properly.  Some or all of the information in this
-message may be unreadable.
-
-
-[% FOREACH part=alternatives %]
-
---[% boundary %]
-Content-type: [% part.type +%]
-
-[%+ part.content %]
-[%+ END %]
---[% boundary %]--

whine.pl

@@ -346,53 +346,20 @@ while (my $event = get_next_event) {
 #  - subject        Subject line for the message
 #  - recipient      user object for the recipient
 #  - author         user object of the person who created the whine event
-#
-# In addition, mail adds two more fields to $args:
-#  - alternatives   array of hashes defining mime multipart types and contents
-#  - boundary       a MIME boundary generated using the process id and time
-#
 sub mail {
     my $args = shift;
-    my $addressee = $args->{recipient};
     # Don't send mail to someone whose bugmail notification is disabled.
-    return if $addressee->email_disabled;
-
-    my $template = Bugzilla->template_inner($addressee->setting('lang'));
-    my $msg = ''; # it's a temporary variable to hold the template output
-    $args->{'alternatives'} ||= [];
-
-    # put together the different multipart mime segments
+    return if $args->{recipient}->email_disabled;
 
-    $template->process("whine/mail.txt.tmpl", $args, \$msg)
-        or die($template->error());
-    push @{$args->{'alternatives'}},
+    $args->{to_user} = $args->{recipient};
+    MessageToMTA(generate_email(
+        $args,
         {
-            'content' => $msg,
-            'type'    => 'text/plain',
-        };
-    $msg = '';
-
-    $template->process("whine/mail.html.tmpl", $args, \$msg)
-        or die($template->error());
-    push @{$args->{'alternatives'}},
-        {
-            'content' => $msg,
-            'type'    => 'text/html',
-        };
-    $msg = '';
-
-    # now produce a ready-to-mail mime-encoded message
-
-    $args->{'boundary'} = "----------" . $$ . "--" . time() . "-----";
-
-    $template->process("whine/multipart-mime.txt.tmpl", $args, \$msg)
-        or die($template->error());
-
-    MessageToMTA($msg);
-
-    delete $args->{'boundary'};
-    delete $args->{'alternatives'};
-
+            header => 'whine/header.txt.tmpl',
+            text   => 'whine/mail.txt.tmpl',
+            html   => 'whine/mail.html.tmpl',
+        }
+    ));
 }
 
 # run_queries runs all of the queries associated with a schedule ID, adding