1584383 - Cannot Open or Save PDF Attachments in Emails Forwarded from MS Outlook 2016

Status: RESOLVED FIXED

(MailNews Core :: Attachments, defect)

Description

[Daniel Karwatka]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:69.0) Gecko/20100101 Firefox/69.0

Steps to reproduce:

Update To new version: 68.1.1 (32-bit) From: 68.1.0 (32-bit).
Forward email with pdf attachment from MS Outlook 2016.

Actual results:

Receive Alert dialog box with message:
This attachment appears to be empty.
Please check with the person who sent this.
Often company firewalls or antivirus programs will destroy attachments.

Also, attachment does not show when attempting to forward the email.
No problems when receiving the same email sent from Thunderbird.

Expected results:

Should have allowed the user to open or save the attachment.

Comment 1

[Jorg K (CEST = GMT+2)]

You need to attach the e-mail in question. Save it as .eml file, edit it in an editor to remove identifying information, then attach it to the bug using "Attach New File". Of course the attachment will be publicly visible. Or forward the message directly to me as an attachment.

Based on your description, there could be 100 issues. If the e-mail is OK when sent from TB, we really need to see what Outlook 2016 has sent.

Comment 2

[Daniel Karwatka]

Attachment: FW Fax Draft Received.eml

Comment 3

[Jorg K (CEST = GMT+2)]

The message has a wrong MIME structure. Line 20 should be Content-Type: multipart/mixed; not multipart/related;

If you change it to "mixed", all works.

Comment 4

[Daniel Karwatka]

I do understand the problem is caused by MS Office but this hadn't been a problem in 68.1.0. It just started in 68.1.1. I rechecked on multiple installations.

Comment 5

[Jorg K (CEST = GMT+2)]

OK. Yes, we made some MIME changes between TB 68.1.0 and TB 68.1.1. Maybe they have this side effect. Kai, can you please take a look.

Comment 6

[Jorg K (CEST = GMT+2)]

Aha, I tried in TB 60.9 and the attachment is "size unknown". So that change doesn't come from the MIME changes but some other attachment changes. Alta88?

Comment 7

[alta88]

Attachment: fetch.patch

So this non spec case is actually one of those "libmime never managed to figure out", and because it is in a local folder (non imap url) it will test the fetch() codepath and thus demonstrate the dumbest typo ever.

Mozmill attachment/ tests run fine locally.

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 9097137 [details] [diff] [review]
fetch.patch

Review of attachment 9097137 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for looking into this.

::: mail/base/content/msgHdrView.js
@@ -2155,5 @@
>            // will come from libmime.
>            let reader = response.body.getReader();
>            let { value: chunk } = await reader.read();
>            reader.cancel();
> -          size = chunk && chunk.value ? chunk.value.length : -1;

I can just see how error-prone that shorthand it, so how about leaving the line you changed and change two lines above to
let chunk = await reader.read();
?

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: fetch.patch

Do you mind using the version that a simple mind can understand?

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 9097154 [details] [diff] [review]
fetch.patch

BTW, yes, that fixes the issue with the sample e-mail.

Comment 11

[alta88]

The syntax is {value: |some var like chunk|, done: |some var like readerDone| } and at some point the whole stream can be read and a real size determined, since libmime didn't do it, which needs a small loop until readerDone is true. So I think leaving that syntax makes that obvious, and I would have done it like that except the linter complains if readerDone isn't used. Anyway, the fix is in, you can tweak/land it however you like.

Comment 12

[alta88]

So if you go with your way, I'd use a var like |result| and not chunk.

Comment 13

[Jorg K (CEST = GMT+2)]

Attachment: fetch.patch

Thanks for the comment. I'm cool with result like our M-C friends do:
https://searchfox.org/mozilla-central/search?q=await+reader.read()%3B&case=false&regexp=false&path=

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/57f0f8a7f242
Fix fetch size checking. r=jorgk

Comment 15

[Jorg K (CEST = GMT+2)]

TB 70 beta 3:
https://hg.mozilla.org/releases/comm-beta/rev/798aaad4b69f7ff4efa0dd673b1c5f5731813256

Comment 16

[Jorg K (CEST = GMT+2)]

TB 68.1.2 or TB 68.2:
https://hg.mozilla.org/releases/comm-esr68/rev/eb3420625ac088f281ed413d91cacd0b33bd656e