The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla1.9alpha1

Status

MailNews Core
MIME
--
major
RESOLVED FIXED
11 years ago
3 years ago

People

(Reporter: Tom Bohler, Assigned: Tom Bohler)

Tracking

({fixed1.8.1.2, verified1.8.1.3})

Trunk
mozilla1.9alpha1
fixed1.8.1.2, verified1.8.1.3

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 4 obsolete attachments)

(Assignee)

Description

11 years ago
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.
(Assignee)

Comment 1

11 years ago
Created attachment 227532 [details]
Attachmant to mail with Outlook Express
(Assignee)

Comment 2

11 years ago
Created attachment 227533 [details]
Message received by Thunderbird (Containing the file test.pdf)
(Assignee)

Comment 3

11 years ago
Created attachment 227534 [details]
Forwarded message (Containing an unreadable version of test.pdf)

Comment 4

11 years ago
Similar: bug 296282.

Comment 5

11 years ago
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
(Assignee)

Comment 6

11 years ago
Created attachment 239647 [details]
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

11 years ago
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
(Assignee)

Comment 8

11 years ago
Created attachment 239997 [details] [diff] [review]
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)?
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → WORKSFORME

Comment 9

11 years ago
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)

Updated

11 years ago
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---

Comment 10

11 years ago
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+

Updated

11 years ago
Attachment #239997 - Flags: superreview?(mscott)

Comment 11

11 years ago
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+

Updated

11 years ago
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
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha

Comment 13

11 years ago
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
Created attachment 242197 [details] [diff] [review]
additional cleanup patch

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
Created attachment 242204 [details] [diff] [review]
additional cleanup patch, v2

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)
Created attachment 242221 [details] [diff] [review]
additional cleanup patch, v3

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 17

11 years ago
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+

Updated

11 years ago
Attachment #242221 - Flags: review?(mscott) → review+
Created attachment 242237 [details] [diff] [review]
additional cleanup patch, for checkin
Attachment #242221 - Attachment is obsolete: true
Whiteboard: [checkin needed]
I've checked in attachment 242237 [details] [diff] [review].
mozilla/mailnews/mime/src/mimeenc.cpp 	1.23
Whiteboard: [checkin needed]

Comment 20

11 years ago
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?

Updated

11 years ago
Attachment #242237 - Flags: approval-thunderbird2?

Comment 21

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

Updated

10 years ago
Attachment #242237 - Flags: approval-thunderbird2? → approval-thunderbird2+

Updated

10 years ago
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 23

10 years ago
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-

Updated

10 years ago
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

Comment 26

3 years ago
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.