747770 - Fails to list attachment such as Content-Type:application/pgp-keys, when no Content-Type:name=, no Content-Disposition:filename=, no Content-Disposition:attachment

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[John]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Build ID: 20120310193829

Steps to reproduce:

save the following in a.eml, and open it in thunderbird:

From: [e-mail]
To: [e-mail]
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="=_0_72114_1335034659_000"; charset="utf-8"

This is a MIME-formatted message.  If you see this text it means that your
mail software cannot handle MIME-formatted messages.

--=_0_72114_1335034659_000
Content-Type: text/plain; format=flowed; delsp=yes; charset="utf-8"
Content-Transfer-Encoding: 7bit


--=_0_72114_1335034659_000
Content-Type: application/pgp-keys; charset="utf-8"
Content-Transfer-Encoding: 7bit

mQGiBEmJN8MRBACWZl9AFgXMN20Dsjuwcp8yp5OCE9pqS/drvErLcXHXUxTEATlC
pMNoS5y4MTWtMF+GwkNLR4JxvR7Y6lsSfR2VgrcG/UnyajqZDf8u9YrQ9UWRGVOQ
EkuwpXhk9K0AoJTG5qR/pLbIcf1yqvgkgpyzFoSMAKC4oCZWFxuUtp8HfnDRXwwW

--=_0_72114_1335034659_000--



Actual results:

thunderbird list no attachment, as if no attachment is in this email.


Expected results:

thunderbird should list an attachment there.

If changing the line

Content-Type: application/pgp-keys; charset="utf-8"

to

Content-Type: application/pgp-keys; charset="utf-8"; name="pubring.gpg"

then thunderbird will be able to list the attachment.

However, RFC2045 does not require the "name=" field in Content-Type.  Ie, thunderbird did not follow RFC2045.

This is especially a problem when working with sqwebmail.  In sqwebmail we can export our pgp key by composing an email in sqwebmail and attaching our own pgp keys, then open the received email in email clients, save the attachment and import to desktop pgp software.  The mail produced by sqwebmail does not contain "name=" field, Outlook can list the attachment, but thunderbird doesn't list attachment so users get confused.

Comment 1

[WADA:World Anti-bad-Duping Agency]

Currently, part shown as attachment at attachmen pane == ( has attachment parameter in Content-Disposition: ) || ( has filename in Content-Disposition: || has name in Content-Type: ), mainly for historical reason.
This produces problem like this bug in following two cases.
- Not-inline-displayable attachment part like application/pgp-keys etc.
- Not-inline-displayable application/octet-stream part
And next case is known on malused application/pkcs7-mime part.
- bug 705059
  even when Content-Disposition: attachment, part is not displayed
  as if attachment, per spec of application/pkcs7-mime.

In RFC, "filename of Content-Disosition:" is merely a file name recommendation by mail sender when attachment is saved as file by mail recipient.
In RFC, "attachment/inline of Content-Disosition:" is sender's request, but "inline of Content-Disosition:" never prohibits "save as file by mail recipient".
I also think "What shoul be shown as attachment at attachment pane" should be sorted out.

Currently available workaround.
- mailnews.display.show_all_body_parts_menu = true
  (via Tools/Options/Advanced/eneral, Config Editor)
  View/Message Body As/All Body Parts
- Save the attachment part
- Change back to mailnews.display.show_all_body_parts_menu = false for daily use

Comment 2

[Ludovic Hirlimann [:Usul]]

John are the workarounds working ?

Wada is this a duplicate ?

Comment 3

[WADA:World Anti-bad-Duping Agency]

(In reply to Ludovic Hirlimann [:Usul] from comment #2)
> Wada is this a duplicate ?

Phenomenon itself is already known in other bugs by intentional test mail. However, IIRC, there is no clear bug report for actual "no filename/no name, no attachment parameter" case. Because "no filename/no name, no attachment parameter" is actually written in a RFC as "recommended" example in this bug's case, RFC violation of Tb is apparent. Duping to other bug is needed? I think duping of part of other bugs to this bug is better.

Comment 4

[John]

(In reply to Ludovic Hirlimann [:Usul] from comment #2)
> John are the workarounds working ?
> 
> Currently available workaround.
- mailnews.display.show_all_body_parts_menu = true
  (via Tools/Options/Advanced/eneral, Config Editor)
  View/Message Body As/All Body Parts
- Save the attachment part
- Change back to mailnews.display.show_all_body_parts_menu = false for daily use

Oh, a much easier workaround that we use is to forward the mail in tb, then the received mail will display the attachment.

Of course, these workarounds assume that we anticipate that email has an attachment.

Comment 5

[WADA:World Anti-bad-Duping Agency]

(In reply to John from comment #4)
> Oh, a much easier workaround that we use is to forward the mail in tb,
> then the received mail will display the attachment.

Thanks for easy/simple workaround operation.
"Forward" looks to access the hidden part normally and looks to set filename/name and Content-Disposition:attachment.
> Content-Type: application/pgp-keys; name="Attached Message Part"
> Content-Disposition: attachment; filename="Attached Message Part"
For ad-hoc/one-time-only "save to local file" purpose, there is no need to send mail because "Save of attachment at Forward window" is possible, although file extension of .tmp should be changed manually after save.

Comment 6

[Jim Porter (:squib)]

Attachment: Fix this

This is easy enough to fix. The new behavior I settled on for when we show an attachment in the attachment pane is as follows:

1) if Content-Disposition is "attachment" -> show
2) else if the attachment can't be displayed inline -> show (this is the new
   condition)
3) else if there's no filename whatsoever -> hide
4) else if the filename was provided by the user -> show
5) else if "Show All Body Parts" is on AND we don't want metadata only -> show
6) else -> hide

Comment 7

[Jim Porter (:squib)]

Attachment: Fix this (properly)

Oops, I forgot to update ProcessBodyAsAttachment as well. Fixed.

Comment 8

[David :Bienvenu]

Comment on attachment 625456 [details] [diff] [review]
Fix this (properly)

looks reasonable, thx for the tests!

Comment 9

[Jim Porter (:squib)]

Checked in: http://hg.mozilla.org/comm-central/rev/f0d80c468f79

Comment 10

[Mark Banner (:standard8)]

Looks like this broke unit tests in gloda:

https://tbpl.mozilla.org/php/getParsedLog.php?id=11936731&tree=Thunderbird-Trunk

Comment 11

[Mark Banner (:standard8)]

Backed out due to the unit test failures:

https://hg.mozilla.org/comm-central/rev/dba50267fd10

Comment 12

[Jim Porter (:squib)]

Attachment: Fix gloda tests

Argh, gloda; I thought I'd checked that, but apparently not. This patch fixes gloda tests locally for me. protz, you wrote the tests for this, and the original "Part 1.2"-hiding code; do my test changes look ok to you?

Try build in progress: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=68c46c12dd86

Comment 13

[Jim Porter (:squib)]

Comment on attachment 626267 [details] [diff] [review]
Fix gloda tests

It looks like the try tests are passing, so flagging this for review.

Comment 14

[Jonathan Protzenko [:protz]]

Comment on attachment 626267 [details] [diff] [review]
Fix gloda tests

Both the test in libmime and the new gloda tests look fine to me! Does

+    name: 'test message with part 1.2 attachment',
+    attachments: [{ body: 'attachment',
+                    filename: '',
+                    format: '' }],

still generate a MimeUnknown container?

Comment 15

[Jim Porter (:squib)]

(In reply to Jonathan Protzenko [:protz] from comment #14)
> Both the test in libmime and the new gloda tests look fine to me! Does
[snip]
> still generate a MimeUnknown container?

Based on the documentation for MimeUnknown, it should.

Comment 16

[David :Bienvenu]

Comment on attachment 626267 [details] [diff] [review]
Fix gloda tests

gloda tests pass with this, thx.

Comment 17

[Ludovic Hirlimann [:Usul]]

(In reply to David :Bienvenu from comment #16)
> Comment on attachment 626267 [details] [diff] [review]
> Fix gloda tests
> 
> gloda tests pass with this, thx.

Jim , did you forget to land this ?

Comment 19

[Jim Porter (:squib)]

(In reply to Ludovic Hirlimann [:Usul] (away until August 5th) from comment #17)
> (In reply to David :Bienvenu from comment #16)
> > Comment on attachment 626267 [details] [diff] [review]
> > Fix gloda tests
> > 
> > gloda tests pass with this, thx.
> 
> Jim , did you forget to land this ?

So I did. I'm going to push to try again and then land this.

Comment 20

[Jim Porter (:squib)]

Try build in progress at https://tbpl.mozilla.org/?tree=Try&rev=60768152bb99

Comment 21

[Jim Porter (:squib)]

Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/c18a0f719011