45956 - Crash trying to forward some messages as inline

Status: VERIFIED FIXED

(MailNews Core :: MIME, defect, P3)

Description

[Scott MacGregor]

I crash a lot (not consistantly) when I try to forward messages inline. The 
common denominator seems to be I18N files that have charsets associated with 
attachments in the message. 

We crash because mime is trying to come up with a unique file name to save the 
attachment to. It's using strings like:
"nsmail.html; charset=iso-2022-jp" as the file name which it then trys to make 
unique

This causes data corruption in nsFileSpec::MakeUnique. 

Mime needs to do more work with the root of the temp file to make sure it looks 
valid before it calls MakeUnique. We certainly shouldn't be trying to create 
files named "nsmail.html; charset=iso-2022-jp"



It can be something simple like

Comment 1

[Scott MacGregor]

I'm wondering if we would pull the beta off the wire for this. Possibly....

Here's the stack trace:
nsFileSpec::MakeUnique() line 917 + 9 bytes
nsMsgCreateTempFileSpec(char * 0x045b1dc0) line 116
mime_decompose_file_init_fn(void * 0x045339e0, MimeHeaders * 0x045b2b50) line 
1683 + 9 bytes
MimeMultipart_create_child(MimeObject * 0x045b2bb0) line 369 + 29 bytes
MimeMultipart_parse_line(char * 0x03b11be0, int 2, MimeObject * 0x045b2bb0) line 
200 + 12 bytes
convert_and_send_buffer(char * 0x03b11be0, int 2, int 1, int (char *, unsigned 
int, void *)* 0x0165fc40 MimeMultipart_parse_line(char *, int, MimeObject *), 
void * 0x045b2bb0) line 157 + 15 bytes
mime_LineBuffer(const char * 0x0012f390, int 2, char * * 0x045b2bd8, int * 
0x045b2be0, unsigned int * 0x045b2be8, int 1, int (char *, unsigned int, void 
*)* 0x0165fc40 MimeMultipart_parse_line(char *, int, MimeObject *), void * 
0x045b2bb0) line 244 + 29 bytes
MimeObject_parse_buffer(char * 0x0012f390, int 2, MimeObject * 0x045b2bb0) line 
253 + 49 bytes
MimeMultipart_parse_child_line(MimeObject * 0x045b2f30, char * 0x03b11798, int 
62, int 0) line 527 + 18 bytes
MimeMultipart_parse_line(char * 0x03b11798, int 64, MimeObject * 0x045b2f30) 
line 261 + 22 bytes
convert_and_send_buffer(char * 0x03b11798, int 64, int 1, int (char *, unsigned 
int, void *)* 0x0165fc40 MimeMultipart_parse_line(char *, int, MimeObject *), 
void * 0x045b2f30) line 157 + 15 bytes
mime_LineBuffer(const char * 0x039f24c8, int 64, char * * 0x045b2f58, int * 
0x045b2f60, unsigned int * 0x045b2f68, int 1, int (char *, unsigned int, void 
*)* 0x0165fc40 MimeMultipart_parse_line(char *, int, MimeObject *), void * 
0x045b2f30) line 244 + 29 bytes
MimeObject_parse_buffer(char * 0x039f24c8, int 64, MimeObject * 0x045b2f30) line 
253 + 49 bytes
MimeMessage_parse_line(char * 0x039f24c8, int 64, MimeObject * 0x0453c5b0) line 
210 + 20 bytes
convert_and_send_buffer(char * 0x039f24c8, int 64, int 1, int (char *, unsigned 
int, void *)* 0x01664a50 MimeMessage_parse_line(char *, int, MimeObject *), void 
* 0x0453c5b0) line 157 + 15 bytes
mime_LineBuffer(const char * 0x03b1d2e3, int 6845, char * * 0x0453c5d8, int * 
0x0453c5e0, unsigned int * 0x0453c5e8, int 1, int (char *, unsigned int, void 
*)* 0x01664a50 MimeMessage_parse_line(char *, int, MimeObject *), void * 
0x0453c5b0) line 244 + 29 bytes
MimeObject_parse_buffer(char * 0x03b1cda0, int 8192, MimeObject * 0x0453c5b0) 
line 253 + 49 bytes
mime_parse_stream_write(_nsMIMESession * 0x0453dc60, const char * 0x03b1cda0, 
int 8192) line 339 + 26 bytes
nsStreamConverter::OnDataAvailable(nsStreamConverter * const 0x0450dd50, 
nsIChannel * 0x045476d4, nsISupports * 0x045365a0, nsIInputStream * 0x04549a9c, 
unsigned int 166499, unsigned int 8192) line 864 + 24 bytes
nsMailboxProtocol::ReadMessageResponse(nsIInputStream * 0x04549a9c, unsigned int 
166499, unsigned int 8192) line 377
nsMailboxProtocol::ProcessProtocolState(nsIURI * 0x045365a4, nsIInputStream * 
0x04549a9c, unsigned int 166499, unsigned int 8192) line 466 + 20 bytes
nsMsgProtocol::OnDataAvailable(nsMsgProtocol * const 0x045476d0, nsIChannel * 
0x0454d3b0, nsISupports * 0x045365a0, nsIInputStream * 0x04549a9c, unsigned int 
166499, unsigned int 8192) line 192 + 32 bytes
nsFileChannel::OnDataAvailable(nsFileChannel * const 0x0454d3b8, nsIChannel * 
0x0454c640, nsISupports * 0x045365a0, nsIInputStream * 0x04549a9c, unsigned int 
166499, unsigned int 8192) line 658 + 49 bytes
nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const 0x0454cbd0) 
line 401 + 47 bytes
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x0454f4e0) line 97 + 12 bytes
PL_HandleEvent(PLEvent * 0x0454f4e0) line 587 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x01508340) line 528 + 9 bytes
_

Comment 2

[Katsuhiko Momoi]

Scott, is this something that happens because you fixed
Bug 39736, i.e. is this something we need to look at tomorrow's
build to see? Please also fill in which one of the Smoketest
message you've used to enduce this problem.

Xianglan and Gary, please check this out as much as possible tomorrow
and find out how frequent this is. 

I tend to agree with mscott that this would be a bad thing to ship 
PR2 with. We have heard from many Japanese users in the past that 
"forward as inline" is something they use a lot.

Comment 3

[Scott MacGregor]

Kat: no this is something I was seeing before my changes for that bug. I think
it's always been doing it. 

Comment 4

[Jay Patel [:jay]]

Putting on [nsbeta2+] radar for beta2 fix.  The + is for stopping the crash.

Comment 5

[Scott MacGregor]

accepting to get off ftang's radar of bugs people haven't looked at yet =). I
have looked at it. Just didn't mark it as accepted. 

Comment 6

[Scott MacGregor]

Attachment: proposed fix

Comment 7

[Scott MacGregor]

I've attached the fix for review. Rich, can you take a look at this and review
it for me?

mimedrfts used to be determing the extension for the attachment by looking at
the content type and trying to do something strange by extracing the word that
came after the "/" in the content type field. This worked in the case of
"text/html". You'd end up with a file called nsmail.html. We would then take
this file name and call nsFileSpec.MakeUnique on it.

However, for I18N attachments, the content type field also contains a charset.
So we'd end up with a file name like: nsmail.html; ISO-2022-JP and then we'd
call nsFileSpecMakeUnique on this whacky file name. This led to a crash in
MakeUnqique.

I didn't think it was safe to be guessing at the file extension by extracing
whatever comes after the '/' in the content type field. This trick would really
only work for a couple of content types (text/html, image/gif). It fails for
things like message/rfc822, text/plain, application/msword and most other
content types.

I think the right thing to do is to ask the mime service to give us the file
extension that is associated with our content type. This is what I've done in
this patch. If we know about the content type, we use the file extension we have
associated with the content type.

So now we always create files like nsmail.html or nsmail.eml. No more files
containing the charset information as part of the file extension.

This code only effects open draft and forward as inline.

to verify, I'd suggest finding a message you can reliably reproduce the crash
with (make sure it has one or more attachments with content types associated
with the attachments). Try forwarding the message as inline multiple times. You
should be able to reproduce the crash that way.

Then try a build with this fix (when I check it in)

Comment 8

[Scott MacGregor]

Fix checked in. 

Comment 9

[Katsuhiko Momoi]

** Checked with 8/2/2000 Win32 build **

I tried various messages with attachments for sending out with
"forward as inline attachment". After trying out about a dozen messages,
I cannot re-create a crash. As I understand it, mscott used some
of the same messages. 
These results point to this fix being effective.

I'm going to mark the fix verified as fixed.