Allow incoming multipart/alternative HTML mail with attachments

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Incoming Email
P3
enhancement
RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: Zach van Schouwen, Assigned: Max Kanat-Alexander)

Tracking

unspecified
Bugzilla 4.4
Dependency tree / graph
Bug Flags:
approval +

Details

(Whiteboard: [es-ita])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14
Build Identifier: 3.0.3

per bug 381912, I'm filing this separately:

When Bugzilla receives HTML mail with attachments, it fails to identify the HTML portion, instead rejecting it as having an "empty body". The code attributes this to a problem with the Email::MIME Stripper component, but my office wrote a pretty decent workaround. I'm attaching it to this bug, in the hopes that it could be reviewed and used (when bug 381912's fix joins the codebase).

Reproducible: Always

Steps to Reproduce:
Send an HTML email to Bugzilla with attachments.
Actual Results:  
It's rejected as empty.

Expected Results:  
The text component should have been extracted and used as the summary.
(Reporter)

Comment 1

10 years ago
Created attachment 323574 [details] [diff] [review]
Patch for HTML mail with attachments

Attaching our in-house patch.
Attachment #323574 - Flags: review?(zmv2102)
(Reporter)

Updated

10 years ago
Depends on: 381912

Comment 2

10 years ago
Comment on attachment 323574 [details] [diff] [review]
Patch for HTML mail with attachments

Don't ask yourself for review. The requestee field must contain the name of the reviewer who is going to review your patch, per http://www.bugzilla.org/docs/reviewer-list.html
Attachment #323574 - Flags: review?(zmv2102) → review?(mkanat)
(Assignee)

Comment 3

10 years ago
Comment on attachment 323574 [details] [diff] [review]
Patch for HTML mail with attachments

I need a patch to exist for bug 381912 first, before I can review this.
Attachment #323574 - Flags: review?(mkanat)
(Assignee)

Updated

10 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Priority: -- → P3
Hardware: PC → All
(Assignee)

Updated

7 years ago
Assignee: incoming.email → mkanat
Summary: Allow incoming HTML mail with attachments → Allow incoming multipart/alternative HTML mail with attachments
Target Milestone: --- → Bugzilla 4.2
(Assignee)

Updated

7 years ago
Target Milestone: Bugzilla 4.2 → Bugzilla 5.0
(Assignee)

Comment 4

7 years ago
Created attachment 534341 [details] [diff] [review]
v1

This removes Email::MIME::Attachment::Stripper from our optional requirements and instead implements similar code directly inside of email_in.pl. This should be much more reliable and do more what we want (and most importantly, be fixable by us).
Attachment #323574 - Attachment is obsolete: true
Attachment #534341 - Flags: review?(glob)
Attachment #534341 - Flags: review?(LpSolit)
Comment on attachment 534341 [details] [diff] [review]
v1

multipart/alternative shouldn't be a BODY_TYPE.

if you have a message with the following structure:
  multipart/alternative
    text/plain body
    text/html body
  image/png attachment

the current code detects only one body:

  Part Content-Type: [multipart/alternative]
  Part Disposition: [inline]
  Part Content-Type: [image/png]
  Part Disposition: [inline; filename="blueball.png"]
  1 body part(s) and 1 attachment part(s).

this may result in incorrect parsing of emails, as it appears to depend on the order of the parts.

if you drop multipart/alternative from the list of BODY_TYPE's, the code falls through to parsing the multipart/alternative's sub-parts, resulting in:

  Part Content-Type: [multipart/alternative]
  Part Disposition: [inline]
  Part Content-Type: [text/plain]
  Part Disposition: [inline]
  Part Content-Type: [text/html]
  Part Disposition: [inline]
  Part Content-Type: [image/png]
  Part Disposition: [inline; filename="blueball.png"]
  2 body part(s) and 1 attachment part(s).

which is what i'd expected to see.
Attachment #534341 - Flags: review?(glob) → review-

Updated

7 years ago
Attachment #534341 - Flags: review?(LpSolit)
(Assignee)

Comment 6

7 years ago
Comment on attachment 534341 [details] [diff] [review]
v1

(In reply to comment #5)
> multipart/alternative shouldn't be a BODY_TYPE.

  No, it should be. Are you perhaps missing the get_text_alternative code that happens below?

> the current code detects only one body:

  That's correct. Then it picks the text part of that body in get_text_alternative.

> [snip]
> which is what i'd expected to see.

  You may just be reading too much into the meaning of the debug output instead of the actual code functionality.

  I don't want to take a text/html inline attachment as a body.
Attachment #534341 - Flags: review- → review?(glob)
Comment on attachment 534341 [details] [diff] [review]
v1

ah, sorry about that; i _was_ reading too much into the debug output.

r=glob
Attachment #534341 - Flags: review?(glob) → review+
(Assignee)

Comment 8

7 years ago
Woohoo! Thanks. :-)
Flags: approval+
(Assignee)

Comment 9

7 years ago
Holding checkin until we branch.
Flags: approval+ → approval?
(Assignee)

Updated

7 years ago
Whiteboard: [es-ita]
(Assignee)

Updated

7 years ago
Blocks: 660691
(Assignee)

Updated

7 years ago
Flags: approval? → approval+
(Assignee)

Comment 10

7 years ago
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified email_in.pl
modified Bugzilla/Install/Requirements.pm
modified template/en/default/global/user-error.html.tmpl                       
Committed revision 7903.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.