1096127 - MsgHdrToMimeMessage, when used in conjunction with IMAP parts on demand, returns wildly incorrect results starting with Thunderbird 31

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Jonathan Protzenko [:protz]]

In thunderbird, one can use MsgHdrToMimeMessage to figure out the MIME structure of a message. Furthermore, for a message that's not stored locally, one can use the parts-on-demand option, so that the entire message is not downloaded.

Here's what MsgHdrToMimeMessage has to say, in Thunderbird 24, for a message that contains a simple attachment:

  1 Container (42 bytes): multipart/mixed
    1 Container (15 bytes): multipart/alternative
      1 Body: text/plain (6 bytes)
      2 Body: text/html (9 bytes)
    2 Attachment (27 bytes): DSC_5633.JPG, image/jpeg

The message is not stored locally, and the parts on demand option is used, meaning that we don't download the entire image, and just report its existence (the size is wrong, but that's ok).

Here's what Thunderbird 31 has to say about the exact same message, in the same conditions.


  1 Container (15 bytes): multipart/mixed
    1 Container (15 bytes): multipart/alternative
      1 Body: text/plain (6 bytes)
      2 Body: text/html (9 bytes)
    2 Unknown: image/jpeg (0 bytes)

That is... puzzling, to say the least.

I haven't run a regression, so I don't know when this change of behavior occurred.

The problem could be in several locations (going from lower-level layers to higher abstractions):
- the IMAP code exhibits a different behavior and feeds different data to libmime;
- libmime started behaving differently and outputs something different;
- the jsmimeemitter changed and now fails to understand what libmime feeds it;
- the msghdrtomimemessage and associated Mime* classes from mailnews/db/gloda/modules changed and no longer build the right message structure.

This can be easily witnessed by installing my glodebug extension <http://jonathan.xulforum.org/files/glodebug.xpi>, then adding the toolbar button, and examining such a message.

I don't recall any sweeping changes taking place in gloda-land, so I suspect something must have changed in the two lower-level layers. CCing some people who might have an idea what's going on.

(I'll keep investigating on my side but as of tomorrow time for Thunderbird will be scarce!)

Comment 1

[Jonathan Protzenko [:protz]]

Sorry, I meant to CC both of you, since you are the "people who might have an idea what's going on" I'm talking about :)

Comment 2

[WADA:World Anti-bad-Duping Agency]

Data in Disk Cache, 
Tb 31.2.0 on Win, 
mail in IMAP Offline-Use=Off folder  With mail.imap.mime_parts_on_demand=true
With View/Message Body As/Plain Text, View/Display Attachments Inline=Off
> Content-Type: multipart/mixed; boundary="------------000507020609050701010503"
> 
> --------------000507020609050701010503
> Content-Type: multipart/alternative;
>  boundary="------------070706060101020908080307"
> 
> --------------070706060101020908080307
> Content-Type: text/plain; charset=utf-8; format=flowed
> Content-Transfer-Encoding: 7bit
> 
> <Sufficiently long text lines of this text/plain part>
> 
> --------------070706060101020908080307
> X-Mozilla-IMAP-Part: 1.2
> Content-Type: text/html; charset=utf-8
> Content-Transfer-Encoding: 7bit
> 
> This body part will be downloaded on demand.
> --------------070706060101020908080307--
> --------------000507020609050701010503
> X-Mozilla-IMAP-Part: 2
> Content-Type: image/png; name="grayXXX-32x32.PNG"
> Content-Transfer-Encoding: base64
> Content-Disposition: attachment; filename="grayXXX-32x32.PNG"
> 
> This body part will be downloaded on demand.
> --------------000507020609050701010503--

Report by by your addon.
>	     |       Mime  results       |
> Size of the message: 24106
> Structure of the message:
> Message (24106 bytes): yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
>   1 Container (24106 bytes): multipart/mixed
>     1 Container (24106 bytes): multipart/alternative
>       1 Body: text/plain (24062 bytes)
>       2 Body: text/html (44 bytes)
>     2 Unknown: image/png (0 bytes)
> Number of attachments: 0

If message is pretty small(text/plain = several bytes, text/html = HTML of several bytes text, attachment = base64 of 1KB image), entire mail data is fetched and is held in _CACHE_002_, and "structure of message" is shown as expected by your addon.

"Function used by your addon" or "your addon" considers above data in Disk Cache as "entire message source".
"Why image/png (0 bytes) is reported" is perhaps as follows : 
    "This body part will be downloaded on demand." is invalid base64 data, so decode result is null.

Comment 3

[WADA:World Anti-bad-Duping Agency]

If this bug actually started to occur from Tb 32, it may be caused by JSMIME(But 858337).
However, as seen in Bug 570914, "27 bytes", which is in "2 Attachment (27 bytes): DSC_5633.JPG, image/jpeg" by your addon in Thunderbird 24, looks for me length of base64 encoded "This body part will be downloaded on demand." which is held in Disk Cache.

Comment 4

[WADA:World Anti-bad-Duping Agency]

Sorry for typo. JSMIME(Bug 858337)

Comment 5

[Jonathan Protzenko [:protz]]

WADA, thanks for looking into this.

MsgHdrToMimeMessage reporting a wrong size for the attachment is not an
issue. What is an issue is that:
- the attachment is reported as MimeUnknown, while in Thunderbird 24 is
was reported as MimeAttachment,
- the MimeMessage that was reported in Thunderbird 24 no longer appears
in Thunderbird 31.

This is expected that the addon should show correct results when the
message is small: if the message is small enough, then it's held
entirely in disk cache and imap "parts on demand" is not used.

Thanks,

~ jonathan

Comment 6

[WADA:World Anti-bad-Duping Agency]

(In reply to Jonathan Protzenko [:protz] from comment #5)
> MsgHdrToMimeMessage reporting a wrong size for the attachment is not an issue. 

I'm not talking about "size is wrong". I'm talking about "where size of 27 bytes came from".
   If 44bytes string of "This body part will be downloaded on demand." is decoded as "base64 encoded data",
   it produces "27 bytes data".  (See Bug 570914 comment #27. It was also 27 :-) )
i.e. I think "Text in report by your addon is different, but 'what is looked' is same in Tb 24, Tb 31, Tb 32. 
i.e. I think it's not regression by JSMIME.

> What is an issue is that:
> - the attachment is reported as MimeUnknown, while in Thunderbird 24 is was reported as MimeAttachment, (snip)

As known by bug 629738 comment #8(on 2011-02-08, Tb 3.1, Tb 3.3 era), message data held in _CACHE_00n_ is not changed.
However, Disk Cache directory structure was changed after it.
    ...\Cache\xxxxxx => ...\Cache\#\%%\xxxxxx where # is 0 to F(1 digit hexa decimal) and %% is two digits hexa decimal.
It may affect on "searching for attached file data under multipart/mixed".
And, if decode of base64 is not done by your addon when Unknown case, "size=0 of the Unknown part" may be explained(note: Content-Type of the part is correctly reported by your addon).
It may be difference in "indicator of Attachment" between Tb 24 and Tb 31/32.
If your addon tries to look "indicator of Attachment in Tb 24",  it may fail in Tb 31/32.

Comment 7

[Jonathan Protzenko [:protz]]

Attachment: patch-jsmimeemitter

It turns out that the list of "known flags" in the js mime emitter was out of date with regards to what the MsgHdrToMimeMessage stashes in the URL parameters, meaning that the PART_RE regexp would subsequently not work, hence failing to properly identify the part as being an attachment, resulting in a broken MimeMessage structure.

I've modify an existing test to exercise this codepath, and the one-liner fix merely consists in making that first JS regexp a little bit more robust.

Kent, I believe my commit access may be out of date. Do you mind pushing this to try to make sure nothing breaks? Thanks!

Comment 8

[Kent James (:rkent)]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b9ce06e01761

Comment 9

[Kent James (:rkent)]

(though this may not help since tests seem to be broken completely at the moment).

Comment 10

[Jonathan Protzenko [:protz]]

xpcshell tests are still green; these are the ones I modified, and gloda is well-covered by these tests, so I'd say this is good :)

Comment 11

[WADA:World Anti-bad-Duping Agency]

(In reply to Jonathan Protzenko [:protz] from comment #7)
> patch-jsmimeemitter
> @@ -86,7 +86,7 @@ function startMime()
>      // downloaded on demand.
>      do_check_true(url.contains("/;section="));

As I wrote in bug 629738 comment #4, two pattern is seen in URI for cached data.
> 23e69343imap://[e-mail]:993
>         /fetch%3EUID%3E/AA/AAAA%3E4/;section=2                  1484603 bytes             1   2011-01-31 17:03:03   No expiration time
>
> 23e69343imap://[e-mail]:993
>         /fetch%3EUID%3E/AA/AAAA%3E4?section=2                   1484645 bytes             1   2011-01-31 17:03:06   No expiration time

"1484645 - 1484603 == 42" looks length of "This body part will be downloaded on demand.".

No need to process "?section=2"? ("Search" in URI, not ";section=2" as part of path in URI)

Comment 12

[Jonathan Protzenko [:protz]]

Right, I think checking that the size is 42 or that the URI contains ;section= is equivalent. It's just that the latter is more robust in case the text ever changes.

Comment 13

[Kent James (:rkent)]

Comment on attachment 8581427 [details] [diff] [review]
patch-jsmimeemitter

LGTM.

We'll land in comm-aurora after a nightly cycle.

Comment 14

[Kent James (:rkent)]

https://hg.mozilla.org/comm-central/rev/8819ea35f085

Comment 15

[Jonathan Protzenko [:protz]]

Fantastic, thanks. If you're in commit mode, there's a checkin-needed of mine on bug 1096119.

Comment 16

[Kent James (:rkent)]

Comment on attachment 8581427 [details] [diff] [review]
patch-jsmimeemitter

http://hg.mozilla.org/releases/comm-aurora/rev/9d679df1db12