520459 - MsgHdrToMimeMessage: headers for MIME subparts not populated

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Patrick Brunschwig]

Attachment: Sample Message

I really like the new MIME parser available through JS! The only issue I have is that the information in MIME subparts is incomplete and thus not usable.

Here is what I miss: the attached sample message contains a PGP/MIME signed MIME subpart; the full contentType is this:
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="J2SCkAp4GZ/dPZZf"

However, part.content-type only contains "multipart/signed" and part.headers is empty. In order to be able to process the message with Enigmail, I would need to know the full content-type.

I'd propose to keep part.contentType as it is, but instead populate header["content-type"] with the full content-type. Furthermore, it would be super cool if any other headers in the subparts would be polulated too

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Attachment: v1 expose headers in all mimemsg representations, provide full content-type

This is a purely JS change to not throw away the content-type information.  As proposed by Patrick, we leave the 'contentType' attribute as-is, but provide the full value in headers.  We do some slight normalization in that we kill newlines for that header (only).  The unit tests now check for this.

One other related change I made at the same time is that I added an extra argument to MsgHdrToMimeMessage that allows us to stream messages that are not available offline (whether in offline storage or cache).

The bad news is that I doubt this is sufficient on its own for enigmail's use case unless there is already trickery in place to make libmime emit the body of the application/pgp-signature part.  Right now I think the part would have to be explicitly requested for streaming.  This would not be a crazy thing for the mimemsg API to expose (and maybe enigmail already does this?).

This patch does nothing to expose other headers.  That would likely require libmime changes.  A specific use-case would be desired for that change so I can explicitly test for it too.

Comment 2

[Patrick Brunschwig]

(In reply to comment #1)
> The bad news is that I doubt this is sufficient on its own for enigmail's use
> case unless there is already trickery in place to make libmime emit the body of
> the application/pgp-signature part.  Right now I think the part would have to
> be explicitly requested for streaming.  This would not be a crazy thing for the
> mimemsg API to expose (and maybe enigmail already does this?).

Fortunately, we don't need to worry about this :-) Enigmail can already now request specific MIME parts from the backend and stream them properly. The issue I had was that I was only /sometimes/ able to discover if there would be such a mime part (and which one). So if I know the full content-type, I can handle the rest.

> This patch does nothing to expose other headers.  That would likely require
> libmime changes.  A specific use-case would be desired for that change so I can
> explicitly test for it too.

Then let's shift this part to a later TB release. Even though I don't have a specific need right now, I can imagine that this could be useful in general. I'll create a new (enhancement) bug so that we don't forget about it.

Comment 3

[David :Bienvenu]

Comment on attachment 404533 [details] [diff] [review]
v1 expose headers in all mimemsg representations, provide full content-type

I'm not feeling the need for this blank line:

+  this.partName = null;
+  this.headers = {};
+
+  this.parts = [];

I've marked this approved for 3.0, if you still want to land it...

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

pushed with requested change:
http://hg.mozilla.org/comm-central/rev/70ef86cbdecf

Comment 5

[Serge Gautherie (:sgautherie)]

test_mime_emitter.js fails on Windows since this checkin, on SM and TB.

Example:
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey2.0/1254787351.1254794469.18642.gz
WINNT 5.2 comm-1.9.1 unit test on 2009/10/05 17:02:31

... | test failed (with xpcshell return code: 0), see following log:
... | text/plain; charset=ISO-8859-1; format=flowed; name="bob.txt" == text/plain; charset=ISO-8859-1; format=flowed;
... | 2147500036 - See following stack:
}

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

bustage fix pushed:
http://hg.mozilla.org/comm-central/rev/fa1d52607567