Bugzilla@Mozilla – Bug 293347
XSLT with document.write crashes [@ nsScrollPortView::~nsScrollPortView]
Last modified: 2010-06-27 22:54:02 PDT
Summon comment box
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; es-ES; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; es-ES; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 Openins this site with my firefox, it crashes automatically. Reproducible: Always
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050503 Firefox/1.0+ I appear to get a hang rather than a crash. ###!!! ASSERTION: mCurrentNode is NULL: 'mCurrentNode', file ../../../../../../src/extensions/transformiix/source/xslt/txMozillaXMLOutput.cpp, line 386 ###!!! ASSERTION: mCurrentNode is NULL: 'mCurrentNode', file ../../../../../../src/extensions/transformiix/source/xslt/txMozillaXMLOutput.cpp, line 465 ###!!! ASSERTION: mCurrentNode is NULL: 'mCurrentNode', file ../../../../../../src/extensions/transformiix/source/xslt/txMozillaXMLOutput.cpp, line 231 ###!!! ASSERTION: initial containing block already created: 'nsnull == mInitialContainingBlock', file ../../../../src/layout/base/nsCSSFrameConstructor.cpp, line 8964 ###!!! ASSERTION: Unexpected child of document element containing block: 'mDocElementContainingBlock->GetFirstChild(nsnull) == nsnull', file ../../../../src/layout/base/nsCSSFrameConstructor.cpp, line 8992 ###!!! ASSERTION: already have a child frame: 'mFrames.IsEmpty()', file ../../../../src/layout/generic/nsHTMLFrame.cpp, line 274 ###!!! ASSERTION: unexpected next reflow command frame: '*iter == mFrames.FirstChild()', file ../../../../src/layout/generic/nsHTMLFrame.cpp, line 481 You might want to review Bug 226425 "xml, xslt crash using apply-templates and document"
Reproduced on two WindowsXP and one Linux machine. I found another URL, that crashes Firefox (recent version 1.0.5): http://de.selfhtml.org/xml/darstellung/anzeige/xsltext.xml This example is quite simpler than the other one: === xmltext.xml === <?xml version="1.0" encoding="ISO-8859-1"?> <?xml-stylesheet type="text/xsl" href="xsltext.xsl" ?> <test> <behauptung>Die Welt ist ein Dorf</behauptung> </test> === xsltext.xsl === <?xml version="1.0" encoding="iso-8859-1"?> <xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"> <xsl:template match="/"> <html> <head> </head> <body> <xsl:apply-templates /> </body> </html> </xsl:template> <xsl:template match="behauptung"> <p><xsl:value-of select="." /></p> <script type="text/javascript"> <xsl:text> <!-- document.write("<p>" + document.lastModified + "</p>"); //--> </xsl:text> </script> </xsl:template> </xsl:stylesheet> After a quick look at both transformation sheets, i recognize that both are using inner Javascript - especially "document.write()" - which might crashes Firefox immediately.
Got another example, I think I know how this works now. I can produce two behaviours with slightly different XSL documents: ----Data.XML---- <?xml version="1.0"?> <?xml-stylesheet type="text/xsl" href="crash.xsl"?> <blank>3</blank> ---------------- #1: This causes FF to simply not load the page, instead it just sits there and attempts to load it forever (does not produce crash) ----crash.xsl---- <xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"> <xsl:template match="/"> <html><script language="javascript">document.write(1)</script></html> </xsl:template> </xsl:stylesheet> --------------- #2: This is the problematic one. This crashes FF 1.0.6 on both Windows and Mac OS X, every time. ----crash.xsl---- <xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"> <xsl:template match="/"> <html><body><script language="javascript">document.write(1)</script></body></html> </xsl:template> </xsl:stylesheet> --------------- The only difference between the two is the second contains a <body> tag around the script; the first does not. Expected results: A single "1" is rendered Actual results: FF crashes. Crash only occurs when document.write is used, not in any other circumstance. Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6 These are the two User-Agents that I have used to test, both with the same reults as described above.
Are any of you seeing these crashes using Firefox 2.0.0.3 or later with a new profile?
Both url's seem to keep loading without finishing, but they don't hang or crash firefox. Tested with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a2pre) Gecko/20090812 Minefield/3.6a2pre ID:20090812045356
I get a crash on Mac when I load http://www.digi21.net/digi3d/internaxml/1/1.inn.xml and then press the back button. ###!!! ASSERTION: root element frame already created: 'nsnull == mRootElementFrame', file /Users/jruderman/central/layout/base/nsCSSFrameConstructor.cpp, line 6559 ###!!! ASSERTION: Shouldn't have a doc element containing block here: '!mDocElementContainingBlock', file /Users/jruderman/central/layout/base/nsCSSFrameConstructor.cpp, line 2448 ###!!! ASSERTION: unexpected second call to SetInitialChildList: 'Not Reached', file /Users/jruderman/central/layout/generic/nsContainerFrame.cpp, line 111 ###!!! ASSERTION: Already have an undisplayed context entry for aContent: '!GetUndisplayedContent(aContent)', file /Users/jruderman/central/layout/base/nsFrameManager.cpp, line 593 ###!!! ASSERTION: unexpected mRootElementFrame: 'processChildren ? !mRootElementFrame : mRootElementFrame == contentFrame', file /Users/jruderman/central/layout/base/nsCSSFrameConstructor.cpp, line 2636
Created attachment 400376 [details] jesse's stack trace
See also bug 202765.
Jonas: please find an appropriate owner for this one
See also bug 202765 comment 39, could "fix" these by disabling document.write in XSLT rather than patching the crash itself.
Created attachment 401110 [details] testcase from URL Preserving original testcase.
Created attachment 401111 [details] testcase stylesheet
Jonas, is this something that you need to look into, or should this go over to layout?
I don't know off the top of my head what's going on here. I suspect the world is different enough from 2005 when this bug was filed that we're now seeing different bugs. I don't actually think making document.write a no-op fixes any actual security problems as you can basically replicate the same behavior by removing all nodes from the document. What we used to have a problem with was layout starting while still displaying the source XML document. Layout (and potentially other) code would then fail when the XSLT code was done running and we tried to swap in the XSLT result instead of the old source XML document. Something about the DocumentViewerImpl::SetDOMDocument not being completely able to swap away a document where layout had already started. Unfortunately I don't really know the code involved off the top of my head. But I suspect the safest fix would be to change DocumentViewerImpl::SetDOMDocument to bail if layout has already started on the current document.
So who should own this bug? Jonas, if you're not the right guy to attempt the fix you suggested in comment 14, can we identify that person?
Jonas? Any progress on the fix you proposed?
Working on another bug, but hopefully should have something end of this week or early next. Simply blocking the call to SetDOMDocument when layout has started should not be too hard.
Created attachment 410705 [details] [review] Patch to fix So the problem is this: If script in the stylesheet calls document.write, the document.write implementation will actually do a whole lot more than just nuke the current document. It will set up a new parser and new content sink and then stream data to these. The new content sink will start layout on the now-empty document. (all of this happens because the result document of an xslt transformation doesn't have a parser, which it shouldn't). This is just scary in and of itself since it means that we have two content sinks messing around with the result document. One that was used to load the original XML file, and one that is used for the document.write. And sure enough, things fail once we're done transforming, since then the XML content sink tries to start layout on the result document. However the document.write content sink has already done so. That is what produces all the layout assertions. Fix is to simply disable document.write for documents created as a result of an XSLT transformation.
Err.. and just imagine that I actually changed the nsIHTMLDocument IID :)
Comment on attachment 410705 [details] [review] Patch to fix r+sr=jst with the IID actually changed! :)
Comment on attachment 410705 [details] [review] Patch to fix Peter, could you have a look at this before I land this on branch as well?
Created attachment 410719 [details] [review] Now with tests
Landed this on trunk. Would still be great to get review from peterv before landing on branch. http://hg.mozilla.org/mozilla-central/rev/a8365a42e185
Checked in to 1.9.2 branch: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/8ecead83349e
Both urls load and work fine now. Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20091109 Minefield/3.7a1pre (.NET CLR 3.5.30729) ID:20091109043812
Comment on attachment 410719 [details] [review] Now with tests >+++ b/content/html/document/src/nsIHTMLDocument.h >-// 5a959364-a2f4-4cac-9a2c-957055dc3569 >+// 56ff0e81-191c-421c-b75c-1727e13091c0 > #define NS_IHTMLDOCUMENT_IID \ Can we change the nsIHTMLDocument interface on branches? Does having it in a .h file make it "internal" enough to not worry about?
Ah, right, I'll have to do a nsIHTMLDocument_1_9_1_BRANCH interface. Want me to write up a patch for that before giving approval?
Yes, please.
Created attachment 411621 [details] [review] 191 branch patch
Created attachment 411622 [details] [review] 191 branch patch This one actually builds
Created attachment 411623 [details] [review] 191 branch patch Ok, really really builds now
Comment on attachment 411623 [details] [review] 191 branch patch Approved for 1.9.1.6, a=dveditz for release-drivers
Checked in to 1.9.1 branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/335158a486fe
Had to land a followup fix for the 1.9.1 branch. HTML elements are in a different namespace there compared to later branches. http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5405e8bb0e82
Jonas: now we need a 1.9.0 version of this fix since we're still supporting that and now the tree has a testcase for this bug. Hopefully not too different from the 1.9.1 version.
Created attachment 412316 [details] [review] 1.9.0 branch patch This *should* work, but I can't test locally. Running a tryserver build right now.
Jonas: How did your tryserver build go?
Created attachment 412752 [details] Another 1.9.0 patch I think this'll work better
Got a verbal a=dveditz Checking in content/html/document/src/nsHTMLDocument.cpp; /cvsroot/mozilla/content/html/document/src/nsHTMLDocument.cpp,v <-- nsHTMLDocument.cpp new revision: 3.796; previous revision: 3.795 done Checking in content/html/document/src/nsHTMLDocument.h; /cvsroot/mozilla/content/html/document/src/nsHTMLDocument.h,v <-- nsHTMLDocument.h new revision: 3.233; previous revision: 3.232 done Checking in content/html/document/src/nsIHTMLDocument.h; /cvsroot/mozilla/content/html/document/src/nsIHTMLDocument.h,v <-- nsIHTMLDocument.h new revision: 3.75; previous revision: 3.74 done Checking in content/xml/document/src/nsXMLContentSink.cpp; /cvsroot/mozilla/content/xml/document/src/nsXMLContentSink.cpp,v <-- nsXMLContentSink.cpp new revision: 1.451; previous revision: 1.450 done Checking in content/xml/document/test/Makefile.in; /cvsroot/mozilla/content/xml/document/test/Makefile.in,v <-- Makefile.in new revision: 1.7; previous revision: 1.6 done RCS file: /cvsroot/mozilla/content/xml/document/test/file_bug293347.xml,v done Checking in content/xml/document/test/file_bug293347.xml; /cvsroot/mozilla/content/xml/document/test/file_bug293347.xml,v <-- file_bug293347.xml initial revision: 1.1 done RCS file: /cvsroot/mozilla/content/xml/document/test/file_bug293347xslt.xml,v done Checking in content/xml/document/test/file_bug293347xslt.xml; /cvsroot/mozilla/content/xml/document/test/file_bug293347xslt.xml,v <-- file_bug293347xslt.xml initial revision: 1.1 done RCS file: /cvsroot/mozilla/content/xml/document/test/test_bug293347.html,v done Checking in content/xml/document/test/test_bug293347.html; /cvsroot/mozilla/content/xml/document/test/test_bug293347.html,v <-- test_bug293347.html initial revision: 1.1
Verified for 1.9.0 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.16pre) Gecko/2009111921 GranParadiso/3.0.16pre (.NET CLR 3.5.30729) using Brandon's testcase (and passing unit test). Verified for 1.9.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.6pre) Gecko/20091119 Shiretoko/3.5.6pre (.NET CLR 3.5.30729) using Brandon's testcase (and mochitest).