Last Comment Bug 197052 - crash if modification innerHTML of element in this element [@ js_EmitTree ]
: crash if modification innerHTML of element in this element [@ js_EmitTree ]
: crash, fixed1.8.0.15, hang, testcase, verified1.8.1.12
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: P2 critical (vote)
: mozilla1.4alpha
Assigned To: Olli Pettay [:smaug]
: Andrew Overholt [:overholt]
: 341671 (view as bug list)
Depends on:
Blocks: 400792
  Show dependency treegraph
Reported: 2003-03-12 07:41 PST by Roman Rahman
Modified: 2011-06-09 14:58 PDT (History)
25 users (show)
jst: blocking1.9+
dveditz: blocking1.8.1.12+
dveditz: wanted1.8.1.x+
jruderman: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (375 bytes, text/html)
2003-03-12 16:57 PST, Chris Casciano
no flags Details
mozilla-bin crashlog - OS X (39.44 KB, text/plain)
2003-03-12 17:00 PST, Chris Casciano
no flags Details
modified testcase (370 bytes, text/html)
2006-05-29 02:27 PDT, Olli Pettay [:smaug]
no flags Details
proposed patch - preventing too deep recursion in a simple way (2.37 KB, patch)
2006-05-29 02:42 PDT, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
proposed patch (2.64 KB, patch)
2007-02-10 10:10 PST, Olli Pettay [:smaug]
jst: review+
jst: superreview+
Details | Diff | Splinter Review
for 1.8 (2.78 KB, patch)
2007-12-19 12:18 PST, Olli Pettay [:smaug]
dveditz: approval1.8.1.12+
Details | Diff | Splinter Review

Description Roman Rahman 2003-03-12 07:41:59 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2) Gecko/20020826 MultiZilla/v1.1.22
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2) Gecko/20020826 MultiZilla/v1.1.22

Some problem in processing recursion with _modification_ innerHTML of element in
this element.

Sample 1.
<div id="uniqid">
<script language="JavaScript" type="text/JavaScript">

Sample 2.
<div id="uniqid">
<script language="JavaScript" type="text/JavaScript">

Have Mozilla CRASH in Sample 1, and normal work in Sample 2.

Sample 3.
<div id="uniqid">
<script language="JavaScript" type="text/JavaScript">
document.getElementById('uniqid').innerHTML += "xxx";

I see "xxx xxx" in place "xxx". Problem in re-render HTML, but imho this bugs

Reproducible: Always

Steps to Reproduce:
Comment 1 Olivier Cahagne 2003-03-12 08:06:33 PST
can you attach the testcase and reproduce crash with 1.3 release candidate ?
If so, please post a Talkback ID for this crash
Comment 2 Chris Casciano 2003-03-12 16:57:59 PST
Created attachment 117037 [details]

test case for code snippet 1
Comment 3 Chris Casciano 2003-03-12 17:00:39 PST
Created attachment 117038 [details]
mozilla-bin crashlog - OS X

Crash was in mozilla-bin on OS X... Talkback not envoked... crash at
nsJSContext::EvaluateString (?)
Comment 4 Chris Casciano 2003-03-12 17:04:20 PST
-> All/All (OS X 1.3RC Build)

didn't go looking for dupes, so not confirming yet
Comment 5 Zibi Braniecki [:gandalf][:zibi] 2003-03-12 19:17:24 PST
Confirming with Mozilla 1.3RC 20030311 Windows

Making as NEW. (Could not find and dupe for this) dupeme!

Comment 6 Johnny Stenback (:jst, 2003-03-13 18:33:30 PST
Hmm, I don't immediately see how this could be fixed w/o limiting the nesting
depth of document.write(). Doing that would be easy enough, I'll see what I can do.
Comment 7 Zibi Braniecki [:gandalf][:zibi] 2003-03-14 04:05:18 PST
It's similar behaviour as with loading document into iframe of this document. So
probably limiting the nesting is the only fix.
Comment 8 Johnny Stenback (:jst, 2003-03-19 11:52:49 PST
Mass-reassigning bugs.
Comment 9 Olivier Cahagne 2003-03-24 14:33:19 PST
confirming it also crashes Mozilla on Linux (20030323 CVS) in js_EmitTree:

#0  0x400429dc in js_EmitTree (cx=0x8a5aa48, cg=0xbfe024b0, pn=0x92fadf0)
    at jsemit.c:1956
#1  0x40042281 in EmitPropOp (cx=0x8a5aa48, pn=0x92fae20, op=JSOP_GETPROP,
    cg=0xbfe024b0) at jsemit.c:1747
#2  0x40046524 in js_EmitTree (cx=0x8a5aa48, cg=0xbfe024b0, pn=0x92fae20)
    at jsemit.c:3733
#3  0x4004656b in js_EmitTree (cx=0x8a5aa48, cg=0xbfe024b0, pn=0x8b26f60)
    at jsemit.c:3754
#4  0x4004587a in js_EmitTree (cx=0x8a5aa48, cg=0xbfe024b0, pn=0x8b27110)
    at jsemit.c:3324
#5  0x40076530 in Statements (cx=0x8a5aa48, ts=0x92faa78, tc=0xbfe024b0)
    at jsparse.c:974
#6  0x40075721 in js_CompileTokenStream (cx=0x8a5aa48, chain=0x88541d0,
    ts=0x92faa78, cg=0xbfe024b0) at jsparse.c:452
#7  0x4002c0f5 in CompileTokenStream (cx=0x8a5aa48, obj=0x88541d0,
    ts=0x92faa78, tempMark=0x8a5aac8, eofp=0x0) at jsapi.c:2945
#8  0x4002c317 in JS_CompileUCScriptForPrincipals (cx=0x8a5aa48,
    obj=0x88541d0, principals=0x8b69668, chars=0x908f228, length=76,
"", lineno=1040)
at jsapi.c:3025
#9  0x4002cea3 in JS_EvaluateUCScriptForPrincipals (cx=0x8a5aa48,
    obj=0x88541d0, principals=0x8b69668, chars=0x908f228, length=76,
"", lineno=1040,
rval=0xbfe02628) at jsapi.c:3474
#10 0x418c909b in nsJSContext::EvaluateString (this=0x8a5aa08,
    aScript=@0xbfe02860, aScopeObject=0x88541d0, aPrincipal=0x8b69664,
aLineNo=1040, aVersion=0x40097989 "default", aRetValue=@0xbfe02770,
    aIsUndefined=0xbfe02704) at nsJSEnvironment.cpp:708
#11 0x40f99fc2 in nsScriptLoader::EvaluateScript (this=0x8bc6688,
    aRequest=0x88b2760, aScript=@0xbfe02860) at nsScriptLoader.cpp:656
Comment 10 Johnny Stenback (:jst, 2003-03-24 19:34:26 PST
Brendan, does this look familiar?
Comment 11 Brendan Eich [:brendan] 2003-03-24 20:38:23 PST
Phil, can you try to reduce a testcase into pure-JS, no DOM required?  Thanks,

Comment 12 Phil Schwartau 2003-03-25 14:06:45 PST
Trying to reduce this to pure JS. Anyway, I have reduced 
the original DOM testcase to this:

var s = '\<script>document.write(s);<\/script>';

The stack trace for the original testcase (Comment #2)
and the stack trace for this differ in only trivial ways.

Both crashes occur due to stack overflow, and both feature
this pattern, which repeats over and over:

nsGenericHTMLContainerElement::AppendChildTo() line 3750
HTMLContentSink::ProcessSCRIPTTag() line 5715
HTMLContentSink::AddLeaf() line 3592 + 12 bytes

The line |document.write(s)| adds another <script> element to
the content model containing the instruction |document.write(s)|,
which adds another <script> element containing |document.write(s)|,
and so on ...

If we execute all these intructions we run out of stack. Note,
IE6 does not seem to execute all of it, and loads all variations
of the testcase quickly.
Comment 13 Phil Schwartau 2003-03-25 15:23:50 PST
Spoke with jst about this; it doesn't look to be a JS Engine issue.
The testcase is creating an infinite number of JS compilation units;
that is something the embedding has to guard against, not JS Engine.
Comment 14 Anders Pedersen 2004-03-30 23:45:23 PST
(In reply to comment #2)
> Created an attachment (id=117037)
> testcase
> test case for code snippet 1

Confirming crash using:

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040330
Microsoft Windows 2000 Pro 5.00.2195 SP4

talkbackid: TB8920G
Comment 15 Brendan Eich [:brendan] 2004-03-31 00:22:03 PST
Need some talkback lovin' here.

Comment 16 chris hofmann 2004-03-31 06:27:08 PST
Incident ID: 8920
Stack Signature	js_GetGCThingFlags 36d4dbfd
Email Address
Product ID	MozillaTrunk
Build ID	2004033009
Trigger Time	2004-03-30 23:44:36.0
Platform	Win32
Operating System	Windows NT 5.0 build 2195
Module	js3250.dll + (00019c16)
URL visited
User Comments	- Opening the following testcase - Mozilla
crashes Bug
Since Last Crash	null sec
Total Uptime	null sec
Trigger Reason	Access violation
Source File Name
Trigger Line No.	219
Stack Trace 	
[c:/builds/vc6/release-tinderbox/WINNT_5.1_Clobber/mozilla/js/src/jsgc.c, line 219]
[c:/builds/vc6/release-tinderbox/WINNT_5.1_Clobber/mozilla/js/src/jsgc.c, line 834]
js_GC [c:/builds/vc6/release-tinderbox/WINNT_5.1_Clobber/mozilla/js/src/jsgc.c,
line 1213]
[c:/builds/vc6/release-tinderbox/WINNT_5.1_Clobber/mozilla/js/src/jsgc.c, line 1001]
JS_GC [c:/builds/vc6/release-tinderbox/WINNT_5.1_Clobber/mozilla/js/src/jsapi.c,
line 1682]
[c:/builds/vc6/release-tinderbox/WINNT_5.1_Clobber/mozilla/js/src/jsapi.c, line
line 1799]
line 256]
line 256]
line 603]
line 237]
line 1135]
line 560]
line 98]
line 52]
line 97]
line 351]
line 696]
line 3140]
line 1488]
line 326]
line 2455]
line 2542]
line 436]
line 6020]
line 5948]
line 2281]
line 2025]
line 79]
line 1068]
line 1085]
line 5225]
line 5478]
line 3998]
line 1347]
USER32.dll + 0x2a2d0 (0x77e3a2d0)
USER32.dll + 0x45e5 (0x77e145e5)
USER32.dll + 0xa816 (0x77e1a816)
line 524]
line 1309]
line 1713]
line 1735]
KERNEL32.DLL + 0x287e7 (0x7c5987e7)
Comment 17 Sébastien CRAMATTE 2005-04-21 09:45:03 PDT
I've got an xml file processed by an "<?xsl-stylesheet ?>" PI. 
My stylesheet gerenate strict XHTML 1.1 file and call external Javascript file.
If I create an element on the fly I can't set innerHTML property.

---- code ------
this.tip = document.createElementNS('','div');
var bodyElement = document.getElementsByTagName('body')[0];
this.tip.innerHTML = this.html;

----- error ----------

Erreur : [Exception... "Component returned failure code: 0x80004005
(NS_ERROR_FAILURE) [nsIDOMNSHTMLElement.innerHTML]"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame :: :: Xhint :: line 43" 
data: no]
Fichier Source :
Ligne : 43
Comment 18 timeless 2005-04-21 23:12:53 PDT that's an entirely unrelated problem
Comment 19 Wayne Mery (:wsmwk, NI for questions) 2006-05-26 11:24:57 PDT
hangs FF and SM trunk, but testcase doesn't crash for me
Comment 20 Olli Pettay [:smaug] 2006-05-29 02:27:26 PDT
Created attachment 223678 [details]
modified testcase

Testing whether document.write works after too-deep-recursion
Comment 21 Olli Pettay [:smaug] 2006-05-29 02:42:29 PDT
Created attachment 223680 [details] [diff] [review]
proposed patch - preventing too deep recursion in a simple way

I think 10 is enough for the recursion level, though it can be more.
In these test cases recursion warnings from spidermonkey (or crash(?)) starts 
coming when the level is closer to 90, but some other tests might be using more 
Comment 22 Brendan Eich [:brendan] 2006-05-30 15:33:42 PDT
Could you use the JS_SetThreadStackLimit API and perhaps a new API to get that limit, so you could test (given int dummy local variable) &dummy against it?

Comment 23 Olli Pettay [:smaug] 2006-05-31 03:51:17 PDT
The problem is that JS recursion error comes when a new <script> element is evaluated. So too-deep-stack error doesn't happen yet in HTMLDocument::Write.
But after js recursion error happens, HTML parsing still continues and new 
<script> elements are created and they are evaluated and new errors happen if too deep stack and so on ...
I *think* the simple check for recursion level is enough here. And that is fast.

Comment 24 Olli Pettay [:smaug] 2006-06-15 17:04:54 PDT
*** Bug 341671 has been marked as a duplicate of this bug. ***
Comment 25 Johnny Stenback (:jst, 2006-06-20 17:05:14 PDT
Comment on attachment 223680 [details] [diff] [review]
proposed patch - preventing too deep recursion in a simple way

- At the end of WriteCommon():

+  mTooDeepWriteRecursion = (mWriteLevel != 0 && mTooDeepWriteRecursion);

As written, this makes document.write()'s stop working if any nested document.write() nested too deep. Wouldn't it be less likely to break sites if we'd only block document.write()'s that do nest too deep, but permit following document.write()'s, i.e. simply make document.write() a no-op if mWriteLevel is greater than 10, or does that break in some cases that didn't come to my mind yet?
Comment 26 Olli Pettay [:smaug] 2006-06-21 06:10:22 PDT
Hmm, this seems to work now...
something between 2006-06-04 and 2006-06-05, perhaps Bug 326466
Comment 27 Thomas Bertels 2006-12-12 07:15:43 PST
This is now reproducible with Firefox 2 and 3 alpha 1.
See also (do not click with JavaScript enabled or freeze will occur !)
Comment 28 Johnny Stenback (:jst, 2007-02-08 16:52:55 PST
Smaug, can you check what's going on here, and get a patch together that reflects the last discussion here for 1.9 if needed?
Comment 29 Olli Pettay [:smaug] 2007-02-10 09:57:59 PST
(In reply to comment #25)
> As written, this makes document.write()'s stop working if any nested
> document.write() nested too deep. Wouldn't it be less likely to break sites if
> we'd only block document.write()'s that do nest too deep, but permit following
> document.write()'s, i.e. simply make document.write() a no-op if mWriteLevel is
> greater than 10, or does that break in some cases that didn't come to my mind
> yet?

the problem is that write calls with mWriteLevel < whatever_max_depth may generate new write calls; too-much-recursion is prevented, but there is still endless loop.
Comment 30 Olli Pettay [:smaug] 2007-02-10 10:10:55 PST
Created attachment 254659 [details] [diff] [review]
proposed patch

Comment 31 Olli Pettay [:smaug] 2007-02-10 10:18:14 PST
Btw, the testcase seems to kill Opera 9.x and Konqueror 3.5.x too
Comment 32 Johnny Stenback (:jst, 2007-02-10 14:36:06 PST
Comment on attachment 254659 [details] [diff] [review]
proposed patch

The endless loop would *eventually* be stopped by the JS branch callback (or should at least), but I could imagine it taking a while as 4k branches would need to be executed, so I could see that taking a while.

r+sr=jst for this change, which just kills a document.write() nesting spree dead in the water.
Comment 33 Daniel Veditz [:dveditz] 2007-12-19 12:03:50 PST
Can we get this fixed on the 1.8 branch, too?
Comment 34 Olli Pettay [:smaug] 2007-12-19 12:18:57 PST
Created attachment 293907 [details] [diff] [review]
for 1.8

for 1.8
Comment 35 Daniel Veditz [:dveditz] 2007-12-21 11:47:25 PST
Comment on attachment 293907 [details] [diff] [review]
for 1.8

approved for, a=dveditz for release-drivers
Comment 36 Al Billings [:abillings] 2008-01-18 17:16:01 PST
Verified for branch with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/2008011803 BonEcho/ Verified crash with
Comment 37 Alexander Sack 2008-03-12 08:54:24 PDT
distro patches block
Comment 38 Alexander Sack 2008-03-12 08:55:56 PDT
Comment on attachment 293907 [details] [diff] [review]
for 1.8

distros ship this patch unmodified. caillon, please sign off.
Comment 39 Christopher Aillon (sabbatical, not receiving bugmail) 2008-03-20 13:15:23 PDT
Comment on attachment 293907 [details] [diff] [review]
for 1.8

Comment 40 Christopher Aillon (sabbatical, not receiving bugmail) 2008-03-20 13:26:05 PDT
Fix committed to (a header changed so there was one conflict needed resolving)
Comment 41 Marcia Knous [:marcia - use ni] 2008-05-07 16:05:36 PDT
verified fixed using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050704 Minefield/3.0pre. No crash using testcases in the bug.
Comment 42 Jesse Ruderman 2009-02-11 01:15:37 PST
Crashtest added as part of

Note You need to log in before you can comment on or make changes to this bug.