1657496 - Bugzilla sets email content-type incorrectly

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, defect)

Description

[Emmanuel Seyman]

I'm just going to quote justdave in bmo #1855962:

Bugzilla's code was broken from the beginning, but a bug in Email::MIME prior to 1.949 was accidentally making our code work. 1.949 fixed that bug, and exposed the error in our code.

What Bugzilla's existing code is trying to do:

• if the email has only one part (which happens in plaintext email), it tries to copy the content type from that part to the email itself
• it does this by grabbing the Content-type header from that part and passing it to content_type_set
• this is broken because it's grabbing the entire Content-type header from that part, and passing it to a function designed explicitly to only set the type itself and not the other attributes.

It works in Email::MIME 1.946 because, when generating the Content-type header, it takes the type and subtype and puts them into the Content-type header unchecked, and then starts adding attributes to it. The attributes we passed as part of the subtype, since they weren't checked, actually made it to the header. In 1.949, Email::MIME started using the build_content_type function from Email::MIME::Header, which actually checks that the subtype is valid.

The code path at https://github.com/bugzilla/bugzilla/blob/5.0/Bugzilla/BugMail.pm#L498 shouldn't be necessary at all. Email::MIME, when generating an email which has only one part, already copies the Content-type header from that one and only
part to the email itself, which you can see at https://metacpan.org/release/Email-MIME/source/lib/Email/MIME.pm#L789 and in Bugzilla's earliest-supported version of Email::MIME at https://metacpan.org/source/RJBS/Email-MIME-1.904/lib/Email/MIME.pm#L658

So the correct fix is basically to remove that line of code (and the if block only needs to act if there are multiple parts)

Comment 1

[Dave Miller [:justdave]]

And as I also said on that bug (on brc actually), I'm approving backporting the fix for this to all supported branches, since it actually breaks email notifications if the site updates its Perl modules.

Comment 2

[Dave Miller [:justdave]]

Attachment: patch against harmony

Comment 3

[Dave Miller [:justdave]]

Attachment: patch against bugzilla/5.2 and bugzilla/5.0

Comment 4

[Dave Miller [:justdave]]

Attachment: patch against bugzilla/4.4

Comment 5

[Emmanuel Seyman]

Comment on attachment 9169528 [details] [diff] [review]
patch against bugzilla/5.2 and bugzilla/5.0

Tested and found to correct the problem. I'll be folding this in the Fedora package in the coming days.

Comment 6

[Emmanuel Seyman]

Comment on attachment 9169529 [details] [diff] [review]
patch against bugzilla/4.4

This is the same patch as for 5.0/5.2 but with different context. I'm comfortable giving a + on it without testing it.

Comment 7

[Emmanuel Seyman]

Comment on attachment 9169527 [details] [diff] [review]
patch against harmony

I'm unable to install the harmony branch of bugzilla and, thus, unable to review this patch.

Comment 8

[Dave Miller [:justdave]]

(bugzilla)
[4.4 c3a6c619b] Bug 1657496: correctly handle MIME type on single-part email. r=eseyman, a=justdave
1 file changed, 4 insertions(+), 3 deletions(-)

[5.0 4f4afbd8d] Bug 1657496: correctly handle MIME type on single-part email. r=eseyman, a=justdave
1 file changed, 4 insertions(+), 4 deletions(-)

[5.2 6765ab047] Bug 1657496: correctly handle MIME type on single-part email. r=eseyman, a=justdave
1 file changed, 4 insertions(+), 4 deletions(-)

(harmony)
[main db9cc834d] Bug 1657496: correctly handle MIME type on single-part email. r=cybershadow, a=justdave
1 file changed, 3 insertions(+), 4 deletions(-)

Comment 9

[Dave Miller [:justdave]]

We should commit this on 5.1 as well since we intend to keep that usable until harmony has feature parity with it.

Comment 10

[Dave Miller [:justdave]]

Attachment: GitHub Pull Request - bugzilla/master (5.1.x)

GitHub PR for 5.1 provided by :alialnu

Comment 11

[Dave Miller [:justdave]]

[master f82dde7] Bug 1657496: correctly handle MIME type on single-part email. r=justdave
1 changed file with 4 additions and 4 deletions.