223920 - mailnews crashes if you try to open an attached email with a non-lowercase mime type

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Samuel Sieb]

As often occurs when forwarding email, an entire email message is attached. 
When a WebTV user forwards such a message it has content-type of
"Message/RFC822".  This *is* valid according to RFC 2045 - "Matching of media
type and subtype is ALWAYS case-insensitive".  However, mailnews will go into an
infinite recursion with the following stack trace (repeated endlessly):

#10 0x425b6c31 in nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*)
    (this=0x8fce0c8, request=0x88005d0, aCtxt=0x0) at nsURILoader.cpp:272
#11 0x43d46bd3 in nsStreamConverter::OnStartRequest(nsIRequest*, nsISupports*)
    (this=0x8fce358, request=0x88005d0, ctxt=0x0) at nsStreamConverter.cpp:997

Comment 1

[Samuel Sieb]

To be more clear, the "this" values keep changing, it's not looping on the same
objects, it creates new ones each time.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Note that the nsIChannel api says
(http://lxr.mozilla.org/seamonkey/source/netwerk/base/public/nsIChannel.idl#122):

  "The value of the contentType attribute is a lowercase string."

That is, the channel is supposed to handle getting it into lowercase.  Does
mailnews actually do that?

Finally, which exact build are you using?  Is it pretty recent?  If so, could
you apply the patch in bug 223694 and attach a log generated with that patch to
this bug?  NSPR_LOG_MODULES=URILoader:5 is what you'll want.  I'd like to keep
the URILoader from hanging even in the face of hostile channels and stream
converters if possible....

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

hm, I thought I fixed some mailnews channels to really return a lowercase string
for the contenttype.

which mozilla version are you using, and what kind of message did you load?
(nntp, imap, local mailbox)?

Comment 4

[Samuel Sieb]

It happens with both imap and local.  I'm using a CVS build that's a couple of
days old.  When I was looking at it in the debugger, the mime type does appear
to get lowercased.  From this part of the stack trace:

#8  0x425b7e61 in nsDocumentOpenInfo::ConvertData(nsIRequest*,
nsIURIContentListener*, nsACString const&, nsACString const&) (this=0x8fce0c8,
    request=0x88005d0, aListener=0x87b6fb8, aSrcContentType=@0x8fce0e4,
    aOutContentType=@0xbff92340) at nsURILoader.cpp:547
#9  0x425b78f5 in nsDocumentOpenInfo::DispatchContent(nsIRequest*, nsISupports*)
(this=0x8fce0c8, request=0x88005d0, aCtxt=0x0) at nsURILoader.cpp:469
#10 0x425b6c31 in nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*)
    (this=0x8fce0c8, request=0x88005d0, aCtxt=0x0) at nsURILoader.cpp:272

in nsDocumentOpenInfo::ConvertData, aSrcContentType is "message/rfc822" and
aOutContentType is "*/*".

I will try that patch later.

Comment 5

[David :Bienvenu]

btw, there's no attached message.

Comment 6

[Samuel Sieb]

Attachment: here's a message

If you have "view attachments inline" set, then it will show up fine and there
will be 2 attachments in the attachment box.  If you open the second
attachment, there is no problem, but if you open the first attachment, it will
crash.

Comment 7

[Samuel Sieb]

Attachment: log of opening the message

I open the second attachment first, then the first one.  The log gets very
repetitive after that. :-)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Based on that log, it looks like the converter never changes the type on the
channel, so the data coming out of the converter still claims to message/rfc822,
so we end up stuffing it through the same converter again....

I suppose I could impose a max nesting depth of eg 10 or so; if more than that
many nsDocumentOpenInfos are chained and we have no listener, we just skip
directly to the helper app service instead of messing around with stream
converters...

But that wouldn't help with the whole "want to view the attachment" issue in
this bug.

Comment 9

[Samuel Sieb]

So what is the difference between the 2 attachments?  Why does having the
mime-type with uppercase characters make a difference?  If I change the
mime-type to be all lowercase, there is no problem.

Comment 10

[Boris Zbarsky [:bzbarsky]]

From the point of view of the URILoader, they look identical...  But the
mailnews converter is hosed.  The URL on the channel is:

imap://samuel@gw:143/fetch%3EUID%3E.INBOX%3E72350?part=1.2&type=Message/RFC822&filename=This%20one%20will%20crash%20mozilla..eml

Note the "&type=Message/RFC822".  Now we fall into the code at
http://lxr.mozilla.org/seamonkey/source/mailnews/mime/src/nsStreamConverter.cpp#486

Those comparisons need to be case-insensitive, I bet.

Comment 11

[David :Bienvenu]

Attachment: proposed fix

Comment 12

[David :Bienvenu]

fix checked in, r/sr=mscott, over aim.

Comment 13

[Samuel Sieb]

With that patch, it doesn't crash, but it also doesn't show the message.  The
window comes up, but there's no text in it.