Closed Bug 81682 Opened 24 years ago Closed 24 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: 24 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: 24 years ago → 24 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.