702938 - Thunderbird 8 doesn't show attachment with corrupted filename and name parameter (corrupted rfc2231 encoded filename*="utf-8''test...%82", corrupted name*="utf-8''test...%82", thus bug 705431 started to occur in Tb 8)

Status: REOPENED

(Thunderbird :: Message Reader UI, defect)

Description

[Vins]

Attachment: email with attachement name with utf8 character.eml

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.1 (KHTML, like Gecko) Chrome/14.0.835.202 Safari/535.1

Steps to reproduce:

I've tried to open email with attached file with filename "test with utf8 ł" where "ł" is the utf8 character.


Actual results:

The attachement doesn't show up. It does show e.g. when I try to forward this email or set "show all body parts" option.


Expected results:

The attachement should show up.

Comment 1

[Magnus Melin [:mkmelin]]

Did it show in an earlier version?

Comment 2

[Vins]

No, it hasn't. For example Thunderbird 7 shows this email just fine.

Comment 3

[Magnus Melin [:mkmelin]]

I can confirm the attachment doesn't show in tb8.

Comment 4

[Alice0775 White]

Regression window
Works:
http://hg.mozilla.org/comm-central/rev/3b67c67a2ab0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110809 Thunderbird/8.0a1 ID:20110809030029
Fails:
http://hg.mozilla.org/comm-central/rev/71b93e1bf96c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110810 Thunderbird/8.0a1 ID:20110810030013
Pushlog:
http://hg.mozilla.org/comm-central/pushloghtml?fromchange=3b67c67a2ab0&tochange=71b93e1bf96c

Comment 5

[Jonathan Protzenko [:protz]]

Interesting. jcranmer, you're the MIME expert. Can you tell if that message structure is illegal or not?

Comment 6

[Magnus Melin [:mkmelin]]

Fallout from bug 564423 then, likely. FWIW the attachment name is displayed correctly in tb7.

Comment 7

[rsx11m]

This message seems to be in compliance with RFC 2231 Section 4, thus valid:

>   Specifically, an asterisk at the end of a parameter name acts as an
>   indicator that character set and language information may appear at
>   the beginning of the parameter value. A single quote is used to
>   separate the character set, language, and actual value information in
>   the parameter value string, and an percent sign is used to flag
>   octets encoded in hexadecimal.  For example:
>
>        Content-Type: application/x-stuff;
>         title*=us-ascii'en-us'This%20is%20%2A%2A%2Afun%2A%2A%2A
>
>   Note that it is perfectly permissible to leave either the character
>   set or language field [applies here] blank.

Comment 9

[David :Bienvenu]

Yes, I believe this an issue with the *= syntax. I think it's running afoul of the code in bug 564423 that suppresses display of attachments w/o names.

Comment 10

[David :Bienvenu]

we should fix this asap.

Comment 11

[Jonathan Protzenko [:protz]]

I'll try to take a look at this tomorrow.

Comment 12

[Jonathan Protzenko [:protz]]

As a workaround, you can use the View > Show all body parts option. I think we should fix the underlying issue, namely making sure libmime understand the filenames with that syntax. David, any idea why libmime is unable to figure out the name of the attachment with the *= syntax? Is it not designed to parse that syntax?

Comment 13

[David :Bienvenu]

protz, I'd be surprised if libmime doesn't understand that filename syntax. I'll try to figure out what's going on in libmime.

Comment 14

[David :Bienvenu]

At first glance, it looks like nsMIMEHeaderParamImpl::DoParameterInternal (which is part of necko) is broken - it tries to handle the filename* case, but doesn't.

Comment 15

[David :Bienvenu]

hg blame picked a bad time to be broken - but that file has changed quite a bit recently. They've been trying to tighten up the parsing of those headers, which may have broken us. I'm removing the dependency on bug 564423 for now.

Comment 16

[David :Bienvenu]

I think this is a problem with the message. I don't think there should be double quotes around the filename, from what I can tell from the rfc. I think it should look like this:

 filename*=utf-8''test%20with%20utf8%20%C5%82

cc'ing Julian, who has been doing a lot of work with nsIMimeHeaderImpl.cpp tightening up our rfc 2231 parsing.

Comment 17

[David :Bienvenu]

In particular, I suspect bug 651185 is pertinent.

Comment 18

[Jonathan Protzenko [:protz]]

Thanks for the awesome detective work, David :)

Comment 19

[Julian Reschke]

Indeed, see also bug 703015.

Out of curiosity, do we know which UA has sent this header field?

Comment 20

[David :Bienvenu]

(In reply to Julian Reschke from comment #19)
> Indeed, see also bug 703015.
thx.
> 
> Out of curiosity, do we know which UA has sent this header field?

Julian, see bug 703614 for one source of such messages.

Comment 21

[Sebastian Hagedorn]

FWIW, in the mean time we confirmed that the messages created by IMP (referenced in bug 703614) are broken. There's a fix for that, which we will implement:

http://bugs.horde.org/ticket/9212

Of course that still leaves messages that have been sent using the broken version.

Comment 22

[Julian Reschke]

(In reply to Sebastian Hagedorn from comment #21)
> FWIW, in the mean time we confirmed that the messages created by IMP
> (referenced in bug 703614) are broken. There's a fix for that, which we will
> implement:
> 
> http://bugs.horde.org/ticket/9212
> 
> Of course that still leaves messages that have been sent using the broken
> version.

Thanks for the pointer. And, indeed, removing support for the broken encoding will be harder for the mail client compared to the browser.

Comment 23

[Julian Reschke]

And, see also bug 703015.

Comment 24

[Vins]

(In reply to Sebastian Hagedorn from comment #21)
> FWIW, in the mean time we confirmed that the messages created by IMP
> (referenced in bug 703614) are broken. There's a fix for that, which we will
> implement:
> 
> http://bugs.horde.org/ticket/9212
> 
> Of course that still leaves messages that have been sent using the broken
> version.

Thank you for the solution. Fixing IMP by replacing old MIME.php library with the latest one fixes the problem. And the problem is connected to IMP not Thunderbird.

Comment 25

[Vins]

(In reply to Vins from comment #24)
> Thank you for the solution. Fixing IMP by replacing old MIME.php library
> with the latest one fixes the problem. And the problem is connected to IMP
> not Thunderbird.

And I meant Horde, not IMP :)

Comment 26

[Julian Reschke]

So do we have any evidence of another MUA sending the same type of broken header, or is this really just one client which already has been fixed?

Comment 27

[David :Bienvenu]

(In reply to Julian Reschke from comment #26)
> So do we have any evidence of another MUA sending the same type of broken
> header, or is this really just one client which already has been fixed?

I don't know of any other MUA sending out the same type of broken header. Doesn't the fix for bug 703015 fix this case as well?

Comment 28

[Julian Reschke]

(In reply to David :Bienvenu from comment #27)
> I don't know of any other MUA sending out the same type of broken header.
> Doesn't the fix for bug 703015 fix this case as well?

Yes, but that fix (IMHO) is a temporary workaround because of the OWA breakage. Once OWA is fixed and most customers have it installed, I'd like to put the conformance improvement back in.

Comment 29

[David :Bienvenu]

(In reply to Julian Reschke from comment #28)

> 
> Yes, but that fix (IMHO) is a temporary workaround because of the OWA
> breakage. Once OWA is fixed and most customers have it installed, I'd like
> to put the conformance improvement back in.

There tends to be a very long tail for customers installing fixes to MS software...so it's going to be hard to know when most customers have it installed.

Comment 30

[Sebastian Hagedorn]

I'm surprised this ticket was closed. It's true that the problem was partly caused by a buggy client, but even though that client is fixed now, that doesn't change anything about the countless messages that have been sent using the buggy version. I can understand the desire to fix the parsing of MIME encodings in the Mozilla core, cf. Bug 651185, but I think Firefox and Thunderbird are very different beasts with respect to that. For Firefox it's much easier to declare an implementation broken (even though Microsoft's buggy OWA is apparently too big to go up against) than for Thunderbird. The problem with an e-mail client is that you can't easily fix history. The messages in the archives are what they are.
I think Thunderbird should continue to indicate the presence of those attachments in some manner, even if their MIME encodings are malformed. Perhaps there's a way to mark them as such, i.e. broken. For the average user it now looks as though there is no attachment at all. I think that's not acceptable.

Comment 31

[Vins]

I think Sebastian has a very good point in: https://bugzilla.mozilla.org/show_bug.cgi?id=702938#c30.

I'm reopening the bug.

Comment 32

[Julian Reschke]

(In reply to Sebastian Hagedorn from comment #30)
> I'm surprised this ticket was closed. It's true that the problem was partly
> caused by a buggy client, but even though that client is fixed now, that
> doesn't change anything about the countless messages that have been sent
> using the buggy version. I can understand the desire to fix the parsing of
> MIME encodings in the Mozilla core, cf. Bug 651185, but I think Firefox and
> Thunderbird are very different beasts with respect to that. For Firefox it's
> much easier to declare an implementation broken (even though Microsoft's
> buggy OWA is apparently too big to go up against) than for Thunderbird. The
> problem with an e-mail client is that you can't easily fix history. The
> messages in the archives are what they are.
> I think Thunderbird should continue to indicate the presence of those
> attachments in some manner, even if their MIME encodings are malformed.
> Perhaps there's a way to mark them as such, i.e. broken. For the average
> user it now looks as though there is no attachment at all. I think that's
> not acceptable.

(a) The change was backed out for now. (b) Thunderbird *should* indicate that there's an attachment with missing filename information (that's what happens in the browser). I understand that's not the case? If so, this should be treated as separate bug which probably is simple to fix.

Comment 33

[rsx11m]

For reference, that was bug 703015 and should become effective with TB 9.0, pending the outcome of any follow-up work in bug 651185.

Comment 34

[Julian Reschke]

I don't think "blocks 651185" is correct here. That bug was reopened because the change was backed out.

Comment 35

[rsx11m]

Well, we need some reference that the work there affects TB's interpretation of messages (blocking in the sense of "regression" and marking the causing bug by a dependency, that's usually done in main/news but you are likely more familiar with what's acceptable for Core bugs). Feel free to make any changes as you see fit, but the people involved in bug 651185 should be aware that it affects not just what Firefox does and that there are more complex cases to be considered.

Comment 36

[Julian Reschke]

(In reply to rsx11m from comment #35)
> Well, we need some reference that the work there affects TB's interpretation
> of messages (blocking in the sense of "regression" and marking the causing
> bug by a dependency, that's usually done in main/news but you are likely
> more familiar with what's acceptable for Core bugs). Feel free to make any
> changes as you see fit, but the people involved in bug 651185 should be
> aware that it affects not just what Firefox does and that there are more
> complex cases to be considered.

Trust me, the people are aware :-)

Comment 37

[rsx11m]

Yes, looking at the assignee of those bugs, that's certainly correct. ;-)
I was also thinking about the reviewers and people cc-ed to it.

Thanks for opening bug 705431 on the general issue that simply encountering a header which is considered to be not valid shouldn't make the attachment hide.

Comment 38

[WADA:World Anti-bad-Duping Agency]

utf-8 or not is irrelevant to problem. removing "utf-8 character" from bug summary to avoid misleading and confusion.