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

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Tom Bohler]

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.

Comment 1

[Tom Bohler]

Attachment: Attachmant to mail with Outlook Express

Comment 2

[Tom Bohler]

Attachment: Message received by Thunderbird (Containing the file test.pdf)

Comment 3

[Tom Bohler]

Attachment: Forwarded message (Containing an unreadable version of test.pdf)

Comment 4

[Magnus Melin [:mkmelin]]

Similar: bug 296282.

Comment 5

[Mike Cowperthwaite]

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.

Comment 6

[Tom Bohler]

Attachment: Proposed patch - folder mailnews/mime/src/mimeenc.cpp

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 7

[Mike Cowperthwaite]

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.

Comment 8

[Tom Bohler]

Attachment: proposed patch - file mailnews/mime/src/mimeenc.cpp

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)?

Comment 9

[Mike Cowperthwaite]

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.

Comment 10

[David :Bienvenu]

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.

Comment 11

[Scott MacGregor]

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!

Comment 12

[:Gavin Sharp [email: [e-mail]]]

mozilla/mailnews/mime/src/mimeenc.cpp 	1.22

Comment 13

[[e-mail]]

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 : ' ';

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Attachment: additional cleanup patch

This is an untested patch to fix Neil's style nit, and to make the code a bit easier to read.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Attachment: additional cleanup patch, v2

Further suggestion from Neil (this is still untested).

Comment 16

[:Gavin Sharp [email: [e-mail]]]

Attachment: additional cleanup patch, v3

As Neil pointed out, I prefer JavaScript's || operator :)

Comment 17

[[e-mail]]

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.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

Attachment: additional cleanup patch, for checkin

Comment 19

[:Gavin Sharp [email: [e-mail]]]

I've checked in attachment 242237 [details] [diff] [review].
mozilla/mailnews/mime/src/mimeenc.cpp 	1.23

Comment 20

[Mike Cowperthwaite]

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.

Comment 21

[Mike Cowperthwaite]

btw, I can verify that 3a1-1018 does the right thing with the test data now.

Comment 22

[:Gavin Sharp [email: [e-mail]]]

Both patches checked in on the branch.
mozilla/mailnews/mime/src/mimeenc.cpp 	1.20.28.2

Comment 23

[Scott MacGregor]

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.

Comment 25

[Carsten Book [:Tomcat]]

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.

Comment 26

[u389733]

It seems this bug is not fixed, I have been able to reproduce it with Thunderbird version 24.2 on Windows XP.