458685 - encode mime functions in nsIMimeConverter fail when scripted

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Kent James (:rkent)]

While testing UTF8 with bug 419356, I needed to mime-encode a modified subject. In the process, I noticed that the encode function EncodeMimePartIIStr_UTF8 was failing with an assertion when unicode values were included in the header string.

STR:

1. Code a function that uses nsIMimeConverter.EncodeMimePartIIStr_UTF8
2. Include a non-ASCII character in the header parameter, for example cyrillic.

Expected results: no assertion
Actual results: ASSERTION in xpcconvert.cpp "U+0080/U+0100 - U+FFFF data lost" and failure to properly convert the string.

Comment 1

[Kent James (:rkent)]

Attachment: Convert string to AUTF8

This fix works for me in bug 419356. I've only modified the routine EncodeMimePartIIStr_UTF8 and not EncodeMimePartIIStr which presumably has the same issue. The idl strongly encourages use of the first rather than the second function, though the second function has current .js users that presumably would show the problem. EncodeMimePartIIStr_UTF8 has no current .js users in the code base, so this bug is designed to assist extensions only.

Comment 2

[Kent James (:rkent)]

Attachment: Added unit test

This patch provides a working mime encoder for js callers, but does not solve all of the issues one might want. But it should have quite minimal impact.

I was a little unclear from the documentation about whether I could use the nsDependentCString on a modified variable, and I hate doing the ToNewCString copy, but that seems to be the practice, right?

Comment 3

[[e-mail]]

Comment on attachment 342110 [details] [diff] [review]
Added unit test

nsDependentCString is fine, as in this case we know the string is not null.

Some space nits:

>-  string encodeMimePartIIStr_UTF8(in string  header, 
>+  string encodeMimePartIIStr_UTF8(in AUTF8String  header, 
>                                   in boolean structured, 
>                                   in string  mailCharset, 
>                                   in long    fieldnamelen,
These no longer line up, and you might as well remove the trailing whitespace while you're there ;-)

> nsresult
>-nsMimeConverter::EncodeMimePartIIStr_UTF8(const char    *header, 
>+nsMimeConverter::EncodeMimePartIIStr_UTF8(const nsACString & header, 
>                                           PRBool        structured, 
>                                           const char    *mailCharset, 
>                                           PRInt32       fieldnamelen,
>                                           PRInt32       encodedWordSize, 
>                                           char          **encodedString)
Same again ;-)

>-  char *retString = MIME_EncodeMimePartIIStr(header, structured, mailCharset, fieldnamelen, encodedWordSize);
>+  char *cHeader = ToNewCString(header);
>+  char *retString = MIME_EncodeMimePartIIStr(cHeader, structured, mailCharset, fieldnamelen, encodedWordSize);
Use MIME_EncodeMimePartIIStr(PromiseFlatString(header).get(), ...);
r+sr=me with that fixed.

>+ 
One last extraneous space :-)

Comment 4

[Kent James (:rkent)]

Attachment: Fixed nits, use PromiseFlatCString, patch to checkin [Checkin: Comment 5]

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 342258 [details] [diff] [review]
Fixed nits, use PromiseFlatCString, patch to checkin
[Checkin: Comment 5]

http://hg.mozilla.org/comm-central/rev/8f1f7628d700