486206 - Quoted-printable bugmail has a =0D at the end of every line

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, defect)

Description

[Phil Ringnalda (:philor)]

Starting today on b.m.o, and reproducible on http://landfill.bugzilla.org/bugzilla-3.2-branch/ but not http://landfill.bugzilla.org/bugzilla-tip/, any bugmail which needs to have the body quoted-printable encoded has every line ending with a =0D, which at least in Thunderbird trunk on Windows makes it look like it's double-spaced.

STR:
1. Change your landfill prefs to get mail when you make changes, and change your name to Frédéric.
2. CC yourself on any bug, in both tip and 3.2
3. Read the resulting bugspam in Thunderbird/Win, or view-source (Gmail's website won't show it, since they do whatever they please with message bodies, but their "original" view will at least show the =0D)

No, I'm not sure why Tb/Win shows those mails as double-spaced and Tb/Mac doesn't, but in any case I doubt that 3.2-only behavior is intended. From the timing I'd bet on bug 467920, but I lack a test install to be sure.

Comment 1

[Phil Ringnalda (:philor)]

Attachment: OK email from tip

Comment 2

[Phil Ringnalda (:philor)]

Attachment: Overencoded email from 3.2

Comment 3

[Max Kanat-Alexander]

Well, that patch went into both 3.2 and tip, and all it's doing at the moment is making sure that email lines correctly end with CRLF per RFC2822 (which they actually didn't before). I'll take a look at the attached emails, though. This could be a problem with the particular versions of Perl modules installed on bmo or on landfill, too.

Comment 4

[Max Kanat-Alexander]

Comment on attachment 370297 [details]
Overencoded email from 3.2

This looks like the translated source, not the raw source. Any way to get the raw source of emails that you're getting? Of course, I can probably generate them myself, too, but it'd be nice to have them from your end and also mine.

Comment 5

[Max Kanat-Alexander]

We've actually released 3.2.3. We need a blocking3.2.4 flag.

Comment 6

[Phil Ringnalda (:philor)]

Comment on attachment 370297 [details]
Overencoded email from 3.2

Curse you, Thunderbird, and whoever thought that decoding qp while saving an attachment was a good idea!

Comment 7

[Phil Ringnalda (:philor)]

Attachment: Overencoded (but not then decoded) email from 3.2

Comment 8

[Max Kanat-Alexander]

Comment on attachment 370326 [details]
Overencoded (but not then decoded) email from 3.2

Okay, yeah, this is what I see Bugzilla actually sending.

Comment 9

[Max Kanat-Alexander]

So the authoritative answer on this seems to be:

http://www.freesoft.org/CIE/RFC/1521/6.htm (See Rule #4)

Which indicates that lines should ALWAYS end in CRLF if you're encoding quoted-printable text. In actual fact we are doing that legally correctly. The fact that our encoder has chosen to encode one of them as =0D and one of them as a literal LF is strange, but in actual fact the MUA should be handling that correctly, if I am reading the spec correctly.

In other words, we may be the only people generating emails in this format, and that may be exposing a bug in Thunderbird's QP decoder.

Comment 10

[Max Kanat-Alexander]

Oh wait. Actually, Rule #1:

      Rule #1: (General 8-bit representation) Any octet, except those
      indicating a line break according to the newline convention of the
      canonical (standard) form of the data being encoded, may be
      represented by an "=" followed by a two digit hexadecimal
      representation of the octet's value.

So in fact it is illegal for our QP encoder to be encoding those CRs as =0D.

Comment 11

[Max Kanat-Alexander]

Okay, so the problem is https://rt.cpan.org/Ticket/Display.html?id=44709 , which I just filed. I've looked into working around this in Bugzilla by bypassing Email::MIME::Modifier and just using MIME::QuotedPrint directly, but we would have to hack around dangerously with the internals of our Email::MIME object if I did that--something that I think would be a very bad idea.

(For those curious, we'd have to be manually setting $email->{body} and $email->{body_raw} and bypassing $email->body_set, and who knows what consequences that could have.)

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Maybe the fix for this bug will fix Bug 365713 too.

Comment 13

[Max Kanat-Alexander]

Attachment: v1

Okay, this patch bypasses Email::MIME::Encodings and does the encoding directly, still resulting in valid emails. However, I'd rather see the bug fixed in Email::MIME::Encodings, so I'm going to wait to see if that happens before we actually check in this patch.

Comment 14

[Max Kanat-Alexander]

I also posted a patch for Email::MIME::Encodings, so hopefully things will just get fixed upstream.

Comment 15

[Max Kanat-Alexander]

I'll block 3.2.4 on this. Either we'll require a newer version of Email::MIME::Encodings, or the above patch will go into Bugzilla.

Comment 16

[Max Kanat-Alexander]

(In reply to comment #12)
> Maybe the fix for this bug will fix Bug 365713 too.

  Oh hey, yeah...in fact, I believe that the bugs are identical! :-) Though this one started affecting every email as of 3.2.3.

Comment 18

[:glob ✱]

Comment on attachment 370336 [details] [diff] [review]
v1

r=glob

Comment 19

[Fredrik Jonson]

I applied the patch in attachment 370336 [details] [diff] [review] on a local 3.2.3 installation, and can confirm that the issue (superfluous =0D) is resolved by that patch.

Comment 21

[:glob ✱]

max has chatted with the maintainer of the Email:: modules and this will be fixed upstream (just needs some test written).

Comment 22

[Max Kanat-Alexander]

Attachment: v2 - Up Email::MIME::Encodings requirement

rjbs is going to release a new version of Email::MIME::Encodings later tonight, 1.313, which fixes this bug. (I tested it on landfill by checking out his unreleased code from Git.) So this just bumps up the version number requirement.

Comment 23

[Max Kanat-Alexander]

Attachment: v3

This also fixes the release notes.

Comment 24

[Frédéric Buclin]

Comment on attachment 373253 [details] [diff] [review]
v3

Looks good, but this shouldn't land before Email::MIME::Encodings 1.313 is available on CPAN as nobody can upgrade in that case. r=LpSolit

Comment 25

[Frédéric Buclin]

Oh, it's already available on CPAN, cool.

Comment 26

[Max Kanat-Alexander]

tip:

Checking in Bugzilla/Install/Requirements.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm,v  <--  Requirements.pm
new revision: 1.62; previous revision: 1.61
done
Checking in template/en/default/pages/release-notes.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/release-notes.html.tmpl,v  <--  release-notes.html.tmpl
new revision: 1.35; previous revision: 1.34
done


3.4:

Checking in Bugzilla/Install/Requirements.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm,v  <--  Requirements.pm
new revision: 1.60.2.1; previous revision: 1.60
done
cvs Checking in template/en/default/pages/release-notes.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/release-notes.html.tmpl,v  <--  release-notes.html.tmpl
new revision: 1.33.2.2; previous revision: 1.33.2.1
done


3.2:

Checking in Bugzilla/Install/Requirements.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm,v  <--  Requirements.pm
new revision: 1.47.2.7; previous revision: 1.47.2.6
done
Checking in template/en/default/pages/release-notes.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/release-notes.html.tmpl,v  <--  release-notes.html.tmpl
new revision: 1.17.2.18; previous revision: 1.17.2.17
done