Closed Bug 343102 Opened 15 years ago Closed 15 years ago

Thunderbird cannot forward messages correctly in the inline format when the message contains attachments encoded in quoted-printable format

Categories

(MailNews Core :: MIME, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: Tom.Bohler, Assigned: Tom.Bohler)

References

Details

(Keywords: fixed1.8.1.2, verified1.8.1.3)

Attachments

(5 files, 4 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Thunderbird version 1.5.0.4 (20060516);

Thunderbird version 1.5.0.4 under Windows XP Service Pack2 cannot forward messages correctly in the inline format when the message contains attachments encoded in quoted-printable format. Thunderbird re-encodes the attachment from quoted-printable to base64 and after this re-encoding, the attachment is unreadable.

Reproducible: Always

Steps to Reproduce:
1)	Send a message containing the attached file ”test.pdf” with the mail client “Outlook Express” to a person using Thunderbird. It is an uncompressed PDF document created with Adobe Acrobat distiller that Outlook Express, version 6.0 will send encoded in quoted-printable format and not in base64.
2)	Verify that the attachment is encoded correctly, because if it is encoded in base64, than thunderbird forwards it correctly:
Content-Type: application/pdf;
	name="test.pdf"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename="test.pdf"
3)	Verify that Thunderbird can open / save correctly the attachments by opening them in Adobe Acrobat reader. Thunderbird 1.5.0 wasn’t even able to open or save the attachment.
4)	Forward this message with Thunderbird version 1.5.0.4. Thunderbird must be configured to forward the message as INLINE and not as an attachment.
5)	The forwarded message contains the attachment in BASE64 coding. The file is corrupt and can’t be opened correctly with Adobe Reader.

Actual Results:  
The file attachments are corrupt.

Expected Results:  
File-Attachments should contain their initial data.
Similar: bug 296282.
Reproduced with TB 3a1-0625, Win2K.

It appears to me the problem is that bytes encoded with =00 are being decoded 
as =20.  This was fixed for the save-attachment-to-disk case in bug 317009, but apparently is still fubar for forward-inline.

Workarounds: save the attachment to disk, then send it as your own attachment; or forward the message as an attachment rather than inline.

This may be a dupe of something else -- certainly, bug 296282 is similar -- 
but I can't find a bug with the specific symptom of broken attachments of 
forwarded-inline messages.
Assignee: mscott → nobody
Severity: normal → major
Status: UNCONFIRMED → NEW
Component: Message Compose Window → MailNews: MIME
Ever confirmed: true
OS: Windows XP → Windows 2000
Product: Thunderbird → Core
QA Contact: message-compose → mime
Version: unspecified → Trunk
The file is a correct version of the file "mimeenc.cpp", Thunderbird version 1.5.0.7. I have tested it against bug id 317009 and bug 243199.

Idee behind the correction:

Treat data->objectToDecode is null as if it were different from "nsMimeOutput::nsMimeMessageBodyDisplay".
Comment on attachment 239647 [details]
Proposed patch - folder mailnews/mime/src/mimeenc.cpp

Thanks for this effort.  However, this isn't actually a patch; a patch needs to be in the form a of a diff -- preferably a 'cvs diff' which makes it easy to apply.
Attachment #239647 - Attachment is patch: false
The variable data->objectToDecode is not initialized when forwarding a message in inline-mode. Thunderbird decodes then the null character as "SPACE" which is wrong. This happens when forwarding, replying to a message received or open from local disc (.eml file stored on a folder of the disk and opened via a bubble-click or via the menu File -> Open saved message).
I have tested it against bug 317009 and bug 243199.

Idea behind the correction:

Treat data->objectToDecode is null as if it were different from
"nsMimeOutput::nsMimeMessageBodyDisplay".

Concerning bug 243199: I don't know if it is correct to replace the null character with a space. The null-character should be diplayed as "no character" at all. I don't know how this can be coded, but this encoding should happen in such a way, that the chain of characters doesn't get truncated when a null character appears. It could happen, that even after my patch, some issues with "null"-characters would remain.

I need a working patch for the release of Thunderbird 1.5.0.7. I don't have a C++ compile for the windows-environment. I can't use CYGWIN. Is there a possibility to compile the concerning library without having a license for Visual C++. I believe the express version of Visual C++ does not work with the release of Thunderbird 1.5.0.7.
Is there a possibility to integrate the patch in the next release (1.5.0.8)?
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
Comment on attachment 239997 [details] [diff] [review]
proposed patch - file mailnews/mime/src/mimeenc.cpp

Thanks again.  I'm asking David to the review this.
Attachment #239997 - Flags: review?(bienvenu)
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment on attachment 239997 [details] [diff] [review]
proposed patch - file mailnews/mime/src/mimeenc.cpp

this looks good to me, thx! 

I doubt this will get into 1.5.0.8 - we can get it into 2.0. 

I don't know if vc express builds 1.5.0.x or not, but I think you need cygwin no matter what.

Do you need this for your own use? You might be able to find someone who can build it for you, based on the latest 1.5.0.x source code.
Attachment #239997 - Flags: review?(bienvenu) → review+
Attachment #239997 - Flags: superreview?(mscott)
Comment on attachment 239997 [details] [diff] [review]
proposed patch - file mailnews/mime/src/mimeenc.cpp

cool. Thanks for the patch, and a libmime one too boot!
Attachment #239997 - Flags: superreview?(mscott) → superreview+
OS: Windows 2000 → All
Hardware: PC → All
Whiteboard: [checkin needed]
Assignee: nobody → Tom.Bohler
Status: REOPENED → NEW
mozilla/mailnews/mime/src/mimeenc.cpp 	1.22
Status: NEW → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 239997 [details] [diff] [review]
proposed patch - file mailnews/mime/src/mimeenc.cpp

>+		  *out++ =       c
>+                               ||( 
>+                                    ( 
>+                                       data->objectToDecode && 
>+                                       data->objectToDecode->options->format_out != nsMimeOutput::nsMimeMessageBodyDisplay
>+                                     
>+                                    ) || (! (data->objectToDecode ))
>+                                 ) ? (char) c : ' ';
Try to avoid writing foo && bar || !foo, this is the same as !foo || bar, i.e.
out++ = c || !data->objectToDecode || data->objectToDecode->options->format_out != nsMimeOutput::nsMimeMessageBodyDisplay ? (char)c : ' ';
Attachment #239647 - Attachment is obsolete: true
Attached patch additional cleanup patch (obsolete) — Splinter Review
This is an untested patch to fix Neil's style nit, and to make the code a bit easier to read.
Attachment #242197 - Flags: superreview?(neil)
Attachment #242197 - Flags: review?(mscott)
Attachment #242197 - Attachment description: patch → additional cleanup patch
Attached patch additional cleanup patch, v2 (obsolete) — Splinter Review
Further suggestion from Neil (this is still untested).
Attachment #242197 - Attachment is obsolete: true
Attachment #242204 - Flags: superreview?(neil)
Attachment #242204 - Flags: review?(mscott)
Attachment #242197 - Flags: superreview?(neil)
Attachment #242197 - Flags: review?(mscott)
Attached patch additional cleanup patch, v3 (obsolete) — Splinter Review
As Neil pointed out, I prefer JavaScript's || operator :)
Attachment #242221 - Flags: superreview?(neil)
Attachment #242221 - Flags: review?(mscott)
Attachment #242204 - Attachment is obsolete: true
Attachment #242204 - Flags: superreview?(neil)
Attachment #242204 - Flags: review?(mscott)
Comment on attachment 242221 [details] [diff] [review]
additional cleanup patch, v3

>+	/* Treat null bytes as spaces when format_out is
>+	 nsMimeOutput::nsMimeMessageBodyDisplay (see bug 243199 comment 7) */
>+	PRBool treatNullAsSpace = data->objectToDecode &&
>+	                          data->objectToDecode->options->format_out == nsMimeOutput::nsMimeMessageBodyDisplay;
>+
>   while (length > 0 || i != 0)
Nit: incorrect indentations.

>+			*out++ = c ? c : ((treatNullAsSpace) ? ' ' : (char) c);
Nit: suggest ? (char) c : for consistency.
Attachment #242221 - Flags: superreview?(neil) → superreview+
Attachment #242221 - Flags: review?(mscott) → review+
Whiteboard: [checkin needed]
I've checked in attachment 242237 [details] [diff] [review].
mozilla/mailnews/mime/src/mimeenc.cpp 	1.23
Whiteboard: [checkin needed]
Comment on attachment 239997 [details] [diff] [review]
proposed patch - file mailnews/mime/src/mimeenc.cpp

(In reply to comment #10)
> I doubt this will get into 1.5.0.8 - we can get it into 2.0. 

OK, requesting.
Attachment #239997 - Flags: approval-thunderbird2?
Attachment #242237 - Flags: approval-thunderbird2?
btw, I can verify that 3a1-1018 does the right thing with the test data now.
Attachment #242237 - Flags: approval-thunderbird2? → approval-thunderbird2+
Whiteboard: [checkin needed (1.8 branch)]
Both patches checked in on the branch.
mozilla/mailnews/mime/src/mimeenc.cpp 	1.20.28.2
Keywords: fixed1.8.1.2
Whiteboard: [checkin needed (1.8 branch)]
Comment on attachment 239997 [details] [diff] [review]
proposed patch - file mailnews/mime/src/mimeenc.cpp

looks like this is fixed on the branch via a later patch in this bug.
Attachment #239997 - Flags: approval-thunderbird2? → approval-thunderbird2-
Duplicate of this bug: 367365
verified fixed 1.8.1.3 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.3) Gecko/20070326 Thunderbird/2.0.0.0 Mnenhy/0.7.5.0 ID:2007032620 and the steps to reproduce from this bug.
Keywords: verified1.8.1.3
Product: Core → MailNews Core
It seems this bug is not fixed, I have been able to reproduce it with Thunderbird version 24.2 on Windows XP.
You need to log in before you can comment on or make changes to this bug.