Closed Bug 286446 Opened 19 years ago Closed 19 years ago

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

Categories

(MailNews Core :: Attachments, defect)

Product:
MailNews Core
Component:
Attachments
Platform:
x86
Windows 2000
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: mcow, Assigned: Bienvenu)

References

Details

Attachments

(5 files)

test case
27.55 KB, message/rfc822
Details
proposed fix
2.08 KB, patch
mscott
: superreview+
Details | Diff | Splinter Review
proposed fix
3.09 KB, patch
mscott
: superreview+
asa
: approval-aviary1.1a2+
Details | Diff | Splinter Review
Text file containing two EMLs before/after the new bug.
2.98 KB, text/plain
Details
proposed fix
813 bytes, patch
mscott
: superreview+
asa
: approval-aviary1.1a2+
Details | Diff | Splinter Review 
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 fix — — Splinter 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: 19 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 fix — — Splinter 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: 19 years ago → 19 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 fix — — Splinter 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

Creator:
Created:
Updated:
Size: