306502 - [FIXr]Hang loading page with mimetype application/javascript

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050830
Firefox/1.0+

Loading https://bugzilla.mozilla.org/attachment.cgi?id=194365 hangs Firefox.

Comment 1

[Jesse Ruderman]

Attachment 194365 [details], which hangs Firefox, is a .js file from bug 256246 comment 2.

Comment 2

[shadytrees]

Attachment: PHP: header('Content-type: application/javascript');

Can reproduce on 1.8 branch. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.8b4) Gecko/20050828 Firefox/1.0+

Appears to be due to a "Content-type: application/javascript" header on the
file, planted by the chosen MIME type for that attachment. Attaching PHP source
as text file with only this line.

Title should be changed to reflect.

Comment 3

[shadytrees]

Demonstration URL. May crash your browser.
http://hao2lian.f2o.org/caged/bug306502.php

Comment 4

[shadytrees]

Bugspam. Hang, not crash.

Comment 5

[shadytrees]

More MIME types that hang for me:
http://hao2lian.f2o.org/caged/bug306502/app-ecmascript.php (application/emcascript)
http://hao2lian.f2o.org/caged/bug306502/text-ecmascript.php (text/ecmascript)

Will not:
http://hao2lian.f2o.org/caged/bug306502/text-javascript.php (text/javascript)
http://hao2lian.f2o.org/caged/bug306502/app-x-javascript.php
(application/x-javascript)

Wild guess would be that all the new JS MIME types are hanging from Bug 62485.

Comment 6

[Brendan Eich [:brendan]]

Any idea of hung main thread stack?

/be

Comment 7

[Bob Clary [:bc] (inactive)]

loop at line 1974 in nsParser.cpp    while((result==NS_OK) &&
(theIterationIsOk)) looks like the offender.

nsParser::ResumeParse(int 0x00000001, int 0x00000001, int 0x00000001) line 1974
nsParser::OnStopRequest(nsParser * const 0x02e51894, nsIRequest * 0x02e4f0d0,
nsISupports * 0x00000000, unsigned int 0x80004005) line 2719 + 21 bytes
nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x02e5f478,
nsIRequest * 0x02e4f0d0, nsISupports * 0x00000000, unsigned int 0x80004005) line 390
nsStreamListenerTee::OnStopRequest(nsStreamListenerTee * const 0x02ede198,
nsIRequest * 0x02e4f0d0, nsISupports * 0x00000000, unsigned int 0x80004005) line 66
nsHttpChannel::OnStopRequest(nsHttpChannel * const 0x02e4f0d8, nsIRequest *
0x02e685a8, nsISupports * 0x00000000, unsigned int 0x80004005) line 4070
nsInputStreamPump::OnStateStop() line 507
nsInputStreamPump::OnInputStreamReady(nsInputStreamPump * const 0x02e685ac,
nsIAsyncInputStream * 0x02e68370) line 343 + 11 bytes
nsInputStreamReadyEvent::EventHandler(PLEvent * 0x02e6869c) line 120
PL_HandleEvent(PLEvent * 0x02e6869c) line 688 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00ae3498) line 623 + 9 bytes
_md_TimerProc(HWND__ * 0x06fe021c, unsigned int 0x00000113, unsigned int
0x00000000, unsigned long 0x06aa203f) line 1013 + 9 bytes
USER32! 77d48734()
USER32! 77d49857()
USER32! 77d49791()
USER32! 77d48a10()
nsAppShell::Run(nsAppShell * const 0x00b61308) line 135
nsAppStartup::Run(nsAppStartup * const 0x00b61268) line 145 + 26 bytes
XRE_main(int 0x00000003, char * * 0x003f6cf8, const nsXREAppData * 0x0042201c
kAppData) line 2322 + 35 bytes
main(int 0x00000003, char * * 0x003f6cf8) line 61 + 18 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 7c816d4f()

Comment 8

[Brendan Eich [:brendan]]

Parser, that's spelled m-r-b-k-a-p ;-).

/be

Comment 9

[Boris Zbarsky [:bzbarsky]]

So there are three things going on here:

1)  The patch for bug 62485 was bogus.  It caused us to try parsing this data
with the nsExpatDriver DTD but with nsHTMLContentSink as the sink.
2)  nsExpatDriver could be better about dealing with a sink that's not an
nsIExpatSink (could store the error in mInternalState).
3)  nsParser::OnStopRequest stomps on an rv that it shouldn't stomp on (but this
only matters for 0-length data streams like this bug's testcase URI).

Either of the first two changes would fix this bug; I propose we make both of
them; patch coming up as soon as I wrap up some other things.

Note that the hang is easily reproducible with the following URI:

  data:application/javascript,hang

which suggests to me that no one ever tested the part of the patch for bug 62485
that touches nsContentDLF.  Or gotten that part reviewed by a content peer, for
that matter... :(

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix those three things

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 194440 [details] [diff] [review]
Fix those three things

r=mrbkap

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 194440 [details] [diff] [review]
Fix those three things

If I were someone nick'ed after a precious metal, I would blame the broken
design that couples nsContentDLF to nsParser magically.

But I'll blame me for not reviewing better last time, and wonder why you are
letting me sr!

Still, from a sr point of view this all looks good.  Comment the coupling to
nsContentDLF (in both places)?

/be

Comment 13

[Boris Zbarsky [:bzbarsky]]

> I would blame the broken design

And you'd be right!

If only parser and layout were a single module, I could just have a single
shared table both could use.  As things stand, I suppose I can hook up a service
to do that or something.... Been meaning to do that for ages; any ideas on doing
it well are much appreciated.

> Comment the coupling to nsContentDLF (in both places)?

Will do.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Fixed trunk and branch