50134 - Malformed comment crashes Mozilla

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect, P3)

Description

[siebert]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.4.0-test5 i686; en-US; m18) Gecko/20000823
BuildID:    2000082321

                            If you have one or more windows with mozilla open
and type in the url: http://www.rp-online.de the complete program
with all windows stops immediately. Waiting doesn't help; you have to kill it.

Reproducible: Always
Steps to Reproduce:
      go to http://www.rp-online.de

Actual Results:                hangs

Expected Results:                      should have loaded the page!

Comment 1

[cheng]

*** Bug 50135 has been marked as a duplicate of this bug. ***

Comment 2

[R.K.Aa.]

seems to hang here too, same build. Confirming.
99% cpu etc, seems to loop in nsString::GetReadableFragment () from libxpcom.so

Comment 3

[Jeffrey Baker]

Testcase attached.  The malformed comment makes Mozilla spin.  Reassigning to
Parser.

The crash does not occur if you specifiy the Strict doctype.

Comment 4

[Jeffrey Baker]

Attachment: Minimal testcase

Comment 5

[Jeffrey Baker]

Really reassigning this time.

Comment 6

[rickg]

Harishd: Please work with scc to repair this. There's code in nsCommentNode 
where an nsReadableString is being accessed out of range, and it (string) 
asserts. It probably shouldn't. But if scc doesn't want to change that you'll 
have to fix this. It's a very common case.

Comment 7

[harishd]

nsCommentNode is akkana's area, I think. Reassigning to her.  

Comment 8

[Akkana Peck]

I don't understand how nsCommentNode works any better than Harish does, but I'm
willing to take a look if it gets marked nsbeta3+ or if someone can contribute a
stack trace.

Comment 9

[Jeffrey Baker]

Well it doesn't exactly crash.  Do you want me to send it SIGINT and take a
stack trace that way?  I'm likely to miss at least part of the stack by doing
that.

Comment 10

[Akkana Peck]

Even so, it would be very helpful -- that sort of thing is the best we're likely
to have to go on.  Thanks!

Comment 11

[Jeffrey Baker]

We're getting an assertion in a loop.  Witness:

#0  0x403ef554 in write () from /lib/libc.so.6
#1  0x402c75bc in __DTOR_END__ () from /lib/libpthread.so.0
#2  0x403a3b80 in _IO_file_write () from /lib/libc.so.6
#3  0x40314172 in filebuf::sys_write () from /usr/lib/libstdc++-libc6.1-1.so.2
#4  0x403a32f6 in _IO_do_write () from /lib/libc.so.6
#5  0x403a3cfb in _IO_file_xsputn () from /lib/libc.so.6
#6  0x403141d6 in filebuf::xsputn () from /usr/lib/libstdc++-libc6.1-1.so.2
#7  0x403984ce in vfprintf () from /lib/libc.so.6
#8  0x403939a6 in vfprintf () from /lib/libc.so.6
#9  0x4039b9cc in printf () from /lib/libc.so.6
#10 0x401289f2 in nsDebug::Assertion (aStr=0x80633a0 "Infinite loop: can't
advance (backward) a readable iterator beyond the end of a string",
aExpr=0x80631fd "one_hop>0", aFile=0x8063360
"../../dist/include/nsAReadableString.h", aLine=228) at nsDebug.cpp:203
#11 0x8057fc3 in nsReadingIterator<unsigned short>::operator-= (this=0xbfffecf0,
n=1) at ../../../dist/include/nsAReadableString.h:228
#12 0x8058060 in nsReadingIterator<unsigned short>::operator+= (this=0xbfffecf0,
n=-1) at ../../../dist/include/nsAReadableString.h:204
#13 0x401548c2 in basic_nsAReadableString<unsigned short>::CharAt
(this=0xbfffed3c, aIndex=4294967295) at
../../dist/include/nsAReadableString.h:548
#14 0x40c5f725 in StripCommentDelimiters (aCommentString=@0xbfffed3c) at
nsCommentNode.cpp:526
#15 0x40c5f802 in nsCommentNode::SetText (this=0x86962b8, aBuffer=0x8695508,
aLength=6, aNotify=0) at nsCommentNode.cpp:546
#16 0x40c8eaf2 in nsGenericDOMDataNode::AppendData (this=0x86962cc,
aOuterContent=0x86962c0, aData=@0x834acb4) at nsGenericDOMDataNode.cpp:374
#17 0x40c6038d in nsCommentNode::AppendData (this=0x86962b8, aData=@0x834acb4)
at nsCommentNode.cpp:55
#18 0x40abf430 in SinkContext::AddComment (this=0x8911710, aNode=@0x82d5b30) at
nsHTMLContentSink.cpp:1796
#19 0x40ac375a in HTMLContentSink::AddComment (this=0x8910788, aNode=@0x82d5b30)
at nsHTMLContentSink.cpp:3081
#20 0x4147a3c7 in CNavDTD::HandleCommentToken (this=0x88ed9c8, aToken=0x834aca0)
at CNavDTD.cpp:2008
#21 0x41477836 in CNavDTD::HandleToken (this=0x88ed9c8, aToken=0x834aca0,
aParser=0x80c7630) at CNavDTD.cpp:777
#22 0x41476ef3 in CNavDTD::BuildModel (this=0x88ed9c8, aParser=0x80c7630,
aTokenizer=0x86b6198, anObserver=0x0, aSink=0x8910788) at CNavDTD.cpp:499
#23 0x4148d007 in nsParser::BuildModel (this=0x80c7630) at nsParser.cpp:1978
#24 0x4148cd9d in nsParser::ResumeParse (this=0x80c7630, allowIteration=1,
aIsFinalChunk=0) at nsParser.cpp:1859
#25 0x4148db62 in nsParser::OnDataAvailable (this=0x80c7630, channel=0x88f4048,
aContext=0x0, pIStream=0x88f3154, sourceOffset=0, aLength=121) at
nsParser.cpp:2309
#26 0x419fe8c2 in nsDocumentOpenInfo::OnDataAvailable (this=0x88f46d0,
aChannel=0x88f4048, aCtxt=0x0, inStr=0x88f3154, sourceOffset=0, count=121) at
nsURILoader.cpp:251
#27 0x41170a9f in nsHTTPFinalListener::OnDataAvailable (this=0x88f4710,
aChannel=0x88f4048, aContext=0x0, aStream=0x88f3154, aSourceOffset=0,
aCount=121) at nsHTTPResponseListener.cpp:1187
#28 0x4113dfc3 in InterceptStreamListener::OnDataAvailable (this=0x88f3150,
channel=0x88f4048, ctxt=0x0, inStr=0x88f1e00, sourceOffset=0, count=121) at
nsCachedNetData.cpp:1194
#29 0x4116f139 in nsHTTPServerListener::OnDataAvailable (this=0x890d660,
channel=0x890cf1c, context=0x88f4048, i_pStream=0x88f1e00, i_SourceOffset=0,
i_Length=121) at nsHTTPResponseListener.cpp:551
#30 0x41105b8c in nsOnDataAvailableEvent::HandleEvent (this=0x88f0a38) at
nsAsyncStreamListener.cpp:400
#31 0x41104dff in nsStreamListenerEvent::HandlePLEvent (aEvent=0x811f170) at
nsAsyncStreamListener.cpp:97
#32 0x4011d29f in PL_HandleEvent (self=0x811f170) at plevent.c:587
#33 0x4011d141 in PL_ProcessPendingEvents (self=0x80aa4a0) at plevent.c:528
#34 0x4011eec1 in nsEventQueueImpl::ProcessPendingEvents (this=0x80aa468) at
nsEventQueue.cpp:356
#35 0x415fdfac in event_processor_callback (data=0x80aa468, source=8,
condition=GDK_INPUT_READ) at nsAppShell.cpp:158
#36 0x415fdbeb in our_gdk_io_invoke (source=0x8184678, condition=G_IO_IN,
data=0x8184668) at nsAppShell.cpp:58
#37 0x417b920e in g_io_unix_dispatch (source_data=0x8184690,
current_time=0xbffff640, user_data=0x8184668) at giounix.c:135
#38 0x417ba717 in g_main_dispatch (dispatch_time=0xbffff640) at gmain.c:656
#39 0x417bacdb in g_main_iterate (block=1, dispatch=1) at gmain.c:877
#40 0x417bae59 in g_main_run (loop=0x81846d8) at gmain.c:935
#41 0x416e9069 in gtk_main () at gtkmain.c:476
#42 0x415fe695 in nsAppShell::Run (this=0x81bfa28) at nsAppShell.cpp:335
#43 0x413c5e84 in nsAppShellService::Run (this=0x81bdd80) at
nsAppShellService.cpp:378
#44 0x8055374 in main1 (argc=1, argv=0xbffff924, nativeApp=0x0) at
nsAppRunner.cpp:958
#45 0x8055a48 in main (argc=1, argv=0xbffff924) at nsAppRunner.cpp:1139
#46 0x403682e7 in __libc_start_main () from /lib/libc.so.6

Reassign as needed.

Comment 12

[Jeffrey Baker]

Fixing bogus whiteboard. 

Comment 13

[rubydoo123]

removing crash, setting to minus and moving to m19

Comment 14

[Jeffrey Baker]

This is a crash.  Mozilla spins forever.  I do not see how this is different
from dumping core, with the added inconvenience of using all of your CPU time. 
Re-adding keyword.

Comment 15

[rubydoo123]

oops, I thought I read that it wasn't, since it is a crash putting back to 
nsbeta3+

Comment 16

[Akkana Peck]

Thanks for the stack trace!  It pointed right to the problem: the check of
offset before calling CharAt(offset-1) wasn't right.  I changed it to check for
offset > 0, and now I can load the page, no asserts.

Comment 17

[Jan Carpenter]

verified:
2000-09-05-08-M18 : win32
2000-09-05-08-M18 : linux
2000-09-05-08-M18 : mac

Comment 18

[Akkana Peck]

Sorry for the spam!  But apparently all these closed bugs need to have their
target milestones changed since M19 and M20 are going away.  Since they're
allready closed, I'm choosing M18.

Comment 19

[Jesse Ruderman]

Crashtest added as part of http://hg.mozilla.org/mozilla-central/rev/5a6def05ccbc