Closed Bug 81682 Opened 23 years ago Closed 23 years ago

crash when opening multipart html attachment to mail message [@ nsMultiMixedConv::FindToken]

Categories

(MailNews Core :: Composition, defect, P2)

Product:
MailNews Core
Component:
Composition
Platform:
x86
Linux
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.2

People

(Reporter: blizzard, Assigned: dougt)

Details

(Keywords: crash, topcrash, Whiteboard: [PDT+] ETA 06/25)

Crash Data

Build is on May 18, 2001:

When I attach a bugzilla query request to a mail message and then try to open
that message from in mozilla with the attachment the browser crashes.

Steps to reproduce:

Make a bugzilla query.

Open a mail message.

Using the url icon in the url bar, drag it to the attachments area of the
message.  Send the message to yourself.

When you get the message note the attachment.  Try to open the attachment and
you will crash.

Here's the stack trace:

#0  0x405478e1 in __libc_nanosleep () from /lib/i686/libc.so.6
#1  0x40547761 in __sleep (seconds=300)
    at ../sysdeps/unix/sysv/linux/sleep.c:85
#2  0x080507df in ah_crap_handler (signum=11) at nsSigHandlers.cpp:114
#3  0x401cf935 in pthread_sighandler (signo=11, ctx=
      {gs = 7, __gsh = 0, fs = 0, __fsh = 0, es = 43, __esh = 0, ds = 43, __dsh
= 0, edi = 140452696, esi = 144875080, ebp = 3221221632, esp = 3221221560, ebx =
1083051828, edx = 140452696, ecx = 0, eax = 3221221576, trapno = 14, err = 4,
eip = 1082804823, cs = 35, __csh = 0, eflags = 66054, esp_at_signal =
3221221560, ss = 43, __ssh = 0, fpstate = 0xbfffee38, oldmask = 2147483648, cr2
= 0})
    at signals.c:97
#4  <signal handler called>
#5  0x408a4a57 in nsMultiMixedConv::FindToken (this=0x85f2358, 
    aCursor=0x8a29e48 "\n--thisrandomstring\nContent-type:
text/html\n\n<html><head><title>Bugzilla is pondering your query</title>\n<style
type=\"text/css\">\n    .psb { margin-top: 20%; text-align: center;
}\n</style></head><body>"..., 
    aLen=20609) at nsMultiMixedConv.cpp:536
#6  0x408a3b40 in nsMultiMixedConv::OnDataAvailable (this=0x85f2358, 
    request=0x85beee8, context=0x8691454, inStr=0x87d295c, sourceOffset=0, 
    count=20608) at nsMultiMixedConv.cpp:202
#7  0x40e990af in nsDocumentOpenInfo::OnDataAvailable (this=0x88bb160, 
    request=0x85beee8, aCtxt=0x8691454, inStr=0x87d295c, sourceOffset=0, 
    count=20608) at nsURILoader.cpp:237
#8  0x41130f9e in nsMimeBaseEmitter::Complete (this=0x8692370)
    at ../../../../dist/include/nsCOMPtr.h:642
#9  0x40ff8a53 in nsStreamConverter::OnStopRequest (this=0x8691b30, 
    request=0x85beee8, ctxt=0x0, status=0)
    at ../../../dist/include/nsCOMPtr.h:648
#10 0x41b08b68 in nsImapCacheStreamListener::OnStopRequest (this=0x88e1218, 
    request=0x8692018, aCtxt=0x0, aStatus=0)
    at ../../../dist/include/nsCOMPtr.h:649
#11 0x4089d359 in nsStorageTransport::nsReadRequest::Process (this=0x8692010)
    at ../../../dist/include/nsCOMPtr.h:649
#12 0x4089d710 in nsStorageTransport::nsReadRequest::OnDataAvailable (
    this=0x8692010, aRequest=0x8692018, aContext=0x0, aInput=0x8692020, 
    aOffset=16384, aCount=5722) at nsStorageTransport.cpp:613
#13 0x400d72f2 in XPTC_InvokeByIndex (that=0x869201c, methodIndex=5, 
    paramCount=5, params=0x8691688) at xptcinvoke_unixish_x86.cpp:138
#14 0x400c79fb in EventHandler (self=0x88be8d8)
    at ../../../dist/include/nsProxyEvent.h:132
#15 0x400c2d4f in PL_HandleEvent (self=0x88be8d8) at plevent.c:588
#16 0x400c2c5d in PL_ProcessPendingEvents (self=0x80ba148) at plevent.c:518
#17 0x400c3dbf in nsEventQueueImpl::ProcessPendingEvents (this=0x80ba320)
    at nsEventQueue.cpp:374
#18 0x40771a76 in event_processor_callback (data=0x80ba320, source=5, 
    condition=GDK_INPUT_READ) at nsAppShell.cpp:168
#19 0x407717c5 in our_gdk_io_invoke (source=0x40d1aca8, condition=G_IO_IN, 
    data=0x40d1ab58) at nsAppShell.cpp:61
#20 0x4035cfae in g_io_unix_dispatch () from /usr/lib/libglib-1.2.so.0
#21 0x4035e783 in g_main_dispatch () from /usr/lib/libglib-1.2.so.0
#22 0x4035ed49 in g_main_iterate () from /usr/lib/libglib-1.2.so.0
#23 0x4035eefc in g_main_run () from /usr/lib/libglib-1.2.so.0
#24 0x402738f3 in gtk_main () from /usr/lib/libgtk-1.2.so.0
#25 0x40771fa6 in nsAppShell::Run (this=0x80eab50) at nsAppShell.cpp:360
#26 0x40747e3a in nsAppShellService::Run (this=0x80e46d8)
    at ../../../dist/include/nsCOMPtr.h:649
#27 0x0804fa27 in main1 (argc=1, argv=0xbffff82c, nativeApp=0x0)
    at ../../dist/include/nsCOMPtr.h:649
#28 0x080502c3 in main (argc=1, argv=0xbffff82c) at nsAppRunner.cpp:1391
#29 0x404a9177 in __libc_start_main (main=0x8050178 <main>, argc=1, 
    ubp_av=0xbffff82c, init=0x804be74 <_init>, fini=0x805210c <_fini>, 
    rtld_fini=0x4000e184 <_dl_fini>, stack_end=0xbffff81c)
    at ../sysdeps/generic/libc-start.c:129

The bit of offending code seems to be nsMultiMixedConv::FindToken() in
netwerk/streamconv/converters/nsMultiMixedConv.cpp.  The token variable ends up
being NULL and we try to dereference it.
Severity: normal → critical
Keywords: crash
Whiteboard: want for mozilla 0.9.1
looks like a 0.9.1 stopper to me
Target Milestone: --- → mozilla0.9.1
cc'ing ducarroz and varada
Looks like we have several problem here:

1) When I send the buzilla query from 4.x, evrything is fine but when it's sent from 6.x, the attachment looks like:

--thisrandomstring
Content-type: text/html

<html><head><title>Bugzilla is pondering your query</title>
<style type="text/css">
    .psb { margin-top: 20%; text-align: center; }
</style></head><body>
<h1 class="psb">Please stand by ...</h1></body></html>
    
--thisrandomstring
Content-type: text/html
Content-disposition: inline; filename=bugzilla_bug_list.html
Set-Cookie: LASTORDER=bugs.priority%2C%20bugs.bug_id ; path=/; expires=Sun, 30-Jun-2029 00:00:00 GMT
Set-Cookie: BUGLIST=80113:81306:81720:22486:28348:76323:79622:71865:74649

<HTML><HEAD>
<TITLE>Bug List</TITLE>

...

--thisrandomstring

We should not have those pseudo two parts in the attachment data, I don't have any clue how that appends

2) The browser code should not crash when viewing the attachment. If I save the attachment to the disk and then 
open it in the browser, I don't crash!

It looks like 4.x will attach the second part of a multipart response instead of
the entire response.  I've set up bug 81751 to track that issue which is
seperate from this which should also be fixed.
I'll bet that the multi mixed converter is getting confused by the fact that
there are tags in the mail message as well as the attachment.
how is this one coming.   we really need it for 0.9.1...
FYI, I have a fix for bug bug 81751
We are crashing because mToken is not being set to a valid value in 
nsMultiMixedConv::OnStartRequest(..)

We are not receiving a boundary=thisrandomstring in the Content-Type.

We should be getting 
Content-Type: multipart/x-mixed-replace;boundary=thisrandomstring

instead we only get
Content-Type: multipart/x-mixed-replace;

so mToken is not being set to the boundary value.
This part of the source from the mail attachment.

Content-Type: multipart/mixed;
 boundary="------------000903060705000804050204"

This is a multi-part message in MIME format.
--------------000903060705000804050204
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit



--------------000903060705000804050204
Content-Type: multipart/x-mixed-replace;
 name="buglist.cgi"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="buglist.cgi"

--thisrandomstring
Content-type: text/html
Ooooh This is a great find Neeti!
I think I know whats going on here... Darin's recent changes broke multi-line
parsing of headers and that is what is going on here. cc'ing darin since he has
a fix for it already.
looks like a dupe of 81008

*** This bug has been marked as a duplicate of 81008 ***
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → DUPLICATE
Yes, this is similar to bug 81008, but is this actually fed into the code
to be changed by the patch to 81008? I haven't investigated in detail, but I
have the patch in my tree and can still reproduce the crash.
Furthermore, even if the boundary value gets lost we should not crash.
Reopening.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Code in nsHttpTransaction.cpp seems not to be touched by reproducing this crash
(or fails the log right before a crash (I have "sync" enabled)?).

Darin: In nsHttpResponseHead::ParseContentType(..), we get type as 
multipart/x-mixed-replace;boundary=thisrandomstring.

When we check if the content-type has additional fields, since we currently only 
check for charset field only, we are stripping away the boundary value in the 
content type, setting it to multipart/x-mixed-replace;

In this case we also need to check for boundary= .

 
the code that looks for the boundary= should call
GetResponseHeader("content-type") instead of GetContentType().

multipart/x-mixed-replace is a content-type;
multipart/x-mixed-replace;boundary=blahblahblah is not.
QA Contact: tever → benc
related to the parser changes darin's working on for bug 81008
Assignee: neeti → darin
Status: REOPENED → NEW
actually, the channel (data source) in question is not HTTP... it is IMAP.

-> mailnews
Assignee: darin → mscott
Component: Networking → Networking - IMAP
Product: Browser → MailNews
QA Contact: benc → huang
Hey Darin, I'm pretty confused about this bug. Imap isn't involved at all. The
multi-mixed converter is the guy trying to parse out the boundary tag. That's in
networking not in mailnews. The multi mixed converter does indeed call
GetResponseHeader("content-type"):
 
rv = httpChannel->GetResponseHeader("content-type", getter_Copies(delimiter));

when it wants to extract the boundary information from it. I don't think we are
getting the boundary string back here. And that's what leads to the crash. 
ignore my comments. I see, the multi mixed converter asks the imap channel for
the content type and we know nothing about parsing boundary headers so we always
give back just a content type when you call GetContentType. Hmm this is going to
be pretty hard to fix I think.
updating status whiteboard.  We might try to get in 81751 which will prevent the
case that we know causes the crash but not the crash.
Whiteboard: want for mozilla 0.9.1 → want for mozilla 0.9.1,eta = unknown
Adding topcrash keyword and [@ nsMultiMixedConv::FindToken] to summary.  This is
a topcrasher according to the latest Talkback data for the Trunk.  Here is the
latest stack trace:

Incident ID 31068424 
nsMultiMixedConv::FindToken
[d:\builds\seamonkey\mozilla\netwerk\streamconv\converters\nsMultiMixedConv.cpp,
line 598] 
nsMultiMixedConv::OnDataAvailable
[d:\builds\seamonkey\mozilla\netwerk\streamconv\converters\nsMultiMixedConv.cpp,
line 161] 
nsDocumentOpenInfo::OnDataAvailable
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 238] 
nsStreamListenerTee::OnDataAvailable
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerTee.cpp, line 57] 
nsHttpChannel::OnDataAvailable
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp, line
2091] 
nsOnDataAvailableEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerProxy.cpp, line 183] 
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591] 
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c,
line 524] 
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line
1072] 
KERNEL32.DLL + 0x248f7 (0xbff848f7) 
0x00688b5e 
0x00058f64 

And some user comments:
     (31068424) URL: bugzilla.mozilla.org
     (31068424) Comments: searching bugzilla for the term "bookmarks"
     (31063889) Comments: Trying to display pdf document
     (30954884) URL: http://westwaybc.camarades.com/
     (30945611) Comments: I tried to open an attachement in Mozilla mail client.
It had a named bugxyz.cgi
     (30906640) Comments: click on buglist attachment
     (30874370) URL: news://news.mcom.com/netscape.public.mozilla.mail-news
     (30867062) Comments: Trying to open a pdf file
     (30862432) URL: www.google.com
     (30862432) Comments: Clicked on a PDF from a Google link.  Walked away
while Acrobat was loading up and came back a few minutes later and the Dr.
Watson was showing and this feedback agent.
Keywords: topcrash
Summary: crash when opening multipart html attachment to mail message → crash when opening multipart html attachment to mail message [@ nsMultiMixedConv::FindToken]
We're going to try to fix 81751 instead.  Moving off 0.9.1 radar.
Target Milestone: mozilla0.9.1 → mozilla0.9.2
multipart html attachment -> esther
Component: Networking - IMAP → Composition
QA Contact: huang → esther
I want to move this out of 0.9.2 but wanted to get some feedback first.  If we
fix 81751 (which has a fix and therefore should make 0.9.2), then we should
prevent the case that we know causes this crash and therefore make this not a
top crash or even a likely crash through mail.  Given that and the fact that it
looks hard to fix, I don't think this is as high of a priority or severity that
it is without the fix in 87151. Any other opinions?

When I read the 3 user's comment above, 1 crash when opening a mail attachment,
2 while surfing the web. That's not a lot of data to make accurate analysis but
looks like fixing mail will solve only some of the cases.
adding PDT+, though with ducarroz's fix I'm not sure if remaining crashers can
happen through mail anymore.
Whiteboard: want for mozilla 0.9.1,eta = unknown → [PDT+]want for mozilla 0.9.1,eta = unknown
Priority: -- → P2
I'm going to re-assign this crasher over to necko. Here's why:
1) We already have a bug tracking the issue that causes mailnews to get into
this crash. jean-Francois has the fix for that

2) many of the talkback reports with this stack trace are seeing this crash just
by browsing web pages. So clearly there are code paths on the http side wich are
leading to the crash in nsMultiMixedConv::FindToken as well. 

see the comments about crashing while searching bugzilla for examples.
Assignee: mscott → darin
removed the stale PDT+ status whiteboard comment.
Whiteboard: [PDT+]want for mozilla 0.9.1,eta = unknown
*** Bug 85891 has been marked as a duplicate of this bug. ***
Keywords: nsenterprise
Whiteboard: [PDT+]
Gagan, since you added PDT+ back to this today is there a patch available
already?  Please add your eta to the status whiteboard.
Whiteboard: [PDT+] → [PDT+] need eta
investigating... i don't expect it to be too difficult to make the multipart
stream converter more robust.  seems like some well placed null checks should
do the trick.
Whiteboard: [PDT+] need eta → [PDT+] ETA 06/25
-> dougt
Assignee: darin → dougt
Status: NEW → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Well, the crash is gone after changes that I just checked in, but I don't see 
the first segment of the multipart (the once that says "Please Wait" or 
whatever).

I think that this is good enough for 0.9.2 (unless you can find a site that send 
multipart attachments where the first part is important).  I filed another bug 
(87190) about the first part of the multipart not viewable.
Using build 2001-07-02 on windows and 2001-06-29 on mac and linux this is fixed
per orginal scenario.  I don't crash opening a bugzilla query attachment.  Bug
query was attached using the 6.1 build.  Verified
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
Crash Signature: [@ nsMultiMixedConv::FindToken]
You need to log in before you can comment on or make changes to this bug.