Closed Bug 286446 Opened 21 years ago Closed 21 years ago

Strip attachment (detach/delete) from complex message breaks MIME structure on rewrite

Categories

(MailNews Core :: Attachments, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mcow, Assigned: Bienvenu)

References

Details

Attachments

(5 files)

Error with bug 2920's patch. TB 1.0+0316, Windows 2000. Given a complex message, such as: multipart/related multipart/alternative text/plain text/html application/msword If the Word attachment is stripped, the rewritten message is incorrect. Specifically in this case, the boundary signifying the end of the HTML message is written immediately after the HTML message's headers; after the boundary comes the HTML message boundary, followed by the correctly formatted stripped- attachment part: multipart/related; boundary=XXX --XXX multipart/alternative; boundary=YYY --YYY text/plain OK OK OK OK --YYY text/html --YYY-- <html>NOT OK</html> --XXX text/x-moz-deleted blah blah blah --XXX-- This causes the HTML message body to not be displayed.
Attached file test case
Here's a sample to test with. Steps to reproduce: 1) integrate this message into your mail system 2) View the message; delete and/or detach the Joe.doc attachment 3 [details] [diff] [review]) View the resulting message 4) View the resulting message's source Actual results: 3) No message body displayed 4) Bogus MIME structure observed Expected results: 3) Message body displayed 4) Correct MIME structure Hmmm... I'd meant to add a note that you need to view as HTML, but I checked view as Plain, and that doesn't display either -- because the plain text message body has been entirely eliminated! I hadn't noticed that particular symptom.
Assignee: sspitzer → bienvenu
Attached patch proposed fixSplinter Review
write a crlf before boundary terminator - only affects stripping of attachments code path...
Attachment #179215 - Flags: superreview?(mscott)
Attachment #179215 - Flags: superreview?(mscott) → superreview+
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
David, I hate to be a nitpicker -- but your patch fixed bug 286447, not this bug!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Additional observations: - If the view settings are "plain text", the HTML part can still be read after deletion of the attachment(s). You have to change "view" to one of the HTML modes, however. The plain text part is completely removed, as reported earlier. - If the view settings are already set to HTML, the complete message body can't be read after the attachment deletion.
A more detailed observation: After deletion of attachment/s, the plain text part is *not gone*. It is simply moved to a bad position. After attachment deletion of a multipart/mixed message, the order in the message body is like this: - outer boundary (mixed) - inner boundary (alternative) - inner boundary (alternative) - message text/html - inner boundary (alternative) - message text/plain - outer boundary (mixed) - x-moz-deleted - outer boundary (mixed) The plain text part should however be placed between the first two inner boundary IDs. When manually correcting this with a text editor, the message is shown correctly again. This is fairly reproducible here.
(In reply to comment #5) > the plain text part is *not gone*. It is simply moved to a bad position. This could truely explain why in my case a mail became totally disturbed after I detached the only attached (large) file from an mail. In my case the attachment's name was still represented in the attachment area of the mail but the text was hidden (although it still exists in the source). I wonder if it was intended to still represent the already detached file's name(s).
*** Bug 296272 has been marked as a duplicate of this bug. ***
Attached patch proposed fixSplinter Review
This fixes the attached test case, and a few other problems with complex messages I found. It only affects code that runs when stripping attachments.
Attachment #186104 - Flags: superreview?(mscott)
Attachment #186104 - Flags: superreview?(mscott) → superreview+
Attachment #186104 - Flags: approval-aviary1.1a2?
Attachment #186104 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Status: REOPENED → RESOLVED
Closed: 21 years ago21 years ago
Resolution: --- → FIXED
The MIME structure seems to be correct now after deletion of an attachment, but now the character set sometimes is changed to UTF-8. This happens when the message body contains non-ASCII characters that are not MIME-encoded, but "directly" inserted in the text. MIME encoded characters (like "=E4" for "ä") are not affected by this bug. The MIME part header still shows the original character set (ISO8859-1 in my test cases), but the non-ASCII characters are now coded in UTF-8. (Tested with the Suite 1.8a2, Build 2005-06-14, 10 am.)
Tilmann, can you attach a test case that shows the problem?
Thanks David for your fast response. I added an attachment with two messages saved into EML files - before and after attachment deletion. Additional comment (not related to this coding bug): after deletion of an attachment, the junk status is set to unknown. It should however be the same as prior to the attachment deletion.
Attached patch proposed fixSplinter Review
thx, that test case was very helpful. Fix is to just pass through the lines w/o decoding, when stripping attachments.
Attachment #186612 - Flags: superreview?(mscott)
Attachment #186612 - Flags: superreview?(mscott) → superreview+
Attachment #186612 - Flags: approval-aviary1.1a2?
Attachment #186612 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
charset conversion fix checked in.
I finally got around to downloading a build with this fix: TB 1.0+0618 Altho the test case in attachment 177627 [details] now displays correctly, its internal structure is still not quite right. After detaching the attachment, the resulting message includes two copies of the text/html body, one of which is hidden between MIME boundaries (without headers of its own): multipart/related; boundary=XXX --XXX multipart/alternative; boundary=YYY --YYY text/plain OK OK OK OK --YYY text/html <html>OK OK OK OK</html> --YYY-- <html>COPY, DOESN'T BELONG HERE</html> --XXX text/x-moz-deleted blah blah blah --XXX-- The second patch, for Tilman's test case, works as expected. David, should I reopen this, or file a new bug?
Let's open a new bug for that issue...thx for checking this, Mike.
*** Bug 296936 has been marked as a duplicate of this bug. ***
Has a bug for this remaining issue (double text/html) been filed yet? I haven't been able to find it.
I just opened it: bug 298596.
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: