Closed Bug 226425 Opened 21 years ago Closed 19 years ago

xml, xslt crash using apply-templates and document

Categories

(Core :: XSLT, defect, P2)

Product:
Core
Component:
XSLT
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.8beta4

People

(Reporter: ian, Assigned: peterv)

References

(
URL
)

Details

(Keywords: crash, testcase, verified1.8, Whiteboard: nostackwanted)

Attachments

(1 file, 1 obsolete file)

v1.1
2.66 KB, patch
axel
: review+
jst
: superreview+
asa
: approval1.8b5+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031107 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031107 Mozilla crashes just loading this page. ---bug.xsl--- <?xml version="1.0" encoding="iso-8859-1"?> <xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"> <xsl:output method="html" indent="yes"/> <xsl:template match="/"> <html><body> <xsl:apply-templates select="document('dontmatter')"/> </body></html> </xsl:template> </xsl:stylesheet> ---bug.xml--- <?xml version="1.0" encoding="iso-8859-1"?> <?xml-stylesheet type="text/xsl" href="bug.xsl" ?> <text></text> Reproducible: Always Steps to Reproduce: 1. Load http://www.gentemsn.com/xml/bug.xml 2. 3. Actual Results: Crash and core dump Expected Results: Load the page succesfully
confirming crash 20031120 Win2k, no stacktrace yet though.
Keywords: crash, stackwanted
With a build from this morning, the page loads with the message "Error during XSLT transformation: XSLT Stylesheet (possibly) contains a recursion."
The problem is that the stylesheet is broken and will recurse indefinitly. However the XSLT code is able to detect this and should not recurse to death. What might happen is that it creates a very deep DOM-tree which if we try to lay it out layout will crash in reflow code because it will recurse until you run out of stackspace. I don't know off hand if or when we replace the result-document with an errorpage, which is what tor is seeing.
> will crash in reflow code because it will recurse This is why the HTML parser limits the DOM tree depth... Though I thought we had some sort of depth-limiting in layout too....
this depends on bug 68824, which makes us return a doc for 404s. Which then actually triggers the recursion and the crash, depending on system. We might wanna lower the recursion limit. Ian, could you give the specifications of your machine? That'd be interesting to know, just to see if we're in a low or high limits situation.
Status: UNCONFIRMED → NEW
Depends on: 68824
Ever confirmed: true
I'm crashing on Win2k with a PIII/900MHz, 384MB RAM (and 576 Virtual memory).
OS: Linux → All
Machine: athlon-xp 1700+, 256MB ram, linux 2.4, glibc-2.2.3, gcc-3.3.2, 500MB swap. core file size (blocks, -c) 32000 data seg size (kbytes, -d) unlimited file size (blocks, -f) unlimited max locked memory (kbytes, -l) unlimited max memory size (kbytes, -m) unlimited open files (-n) 1024 pipe size (512 bytes, -p) 8 stack size (kbytes, -s) 8192 cpu time (seconds, -t) unlimited max user processes (-u) 2047 virtual memory (kbytes, -v) unlimited
crash Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6a) Gecko/20031028 Talkback ID: TB25925804M
Whiteboard: TB25925804M
Whatever the stacktrace shows is failing i think it might be a good idea to cap the nestinglevel we create in txMozillaXMLOutput. The reason is that even if layout is able to deal (or that we make sure that layout never sees document that fails) it is not unlikly that we have other routines in content or elsewhere that recursivly walks up or down a DOM-tree. SetDocument is a good example. So for everyones safty it might be a good idea to let the outputhandler either return an error, or simply ignore elements if the nesinglevel becomes too deep. IMHO it's better safe then being able to support insane documents.
I don't think that tree depth is the right measure. I'd like to see other places in the code that impose limits on the input documents before making any decision about limiting XSLT output. Anyone with lxr links?
Keywords: testcase
Whiteboard: TB25925804M → TB17805K
http://lxr.mozilla.org/seamonkey/source/htmlparser/public/nsIHTMLContentSink.h#94 defines MAX_REFLOW_DEPTH, which is what the html parser uses to cut off content. That macro is actually platform dependent, so just choosing a number on our side sounds like a bad idea. But we depend on htmlparser anyway, so we could reuse that. (talkback ids were not found by talkback-public. And we don't really care about which codepath actually out-of-mems, the problem is clear.)
Keywords: stackwanted
Whiteboard: TB17805K → nostackwanted
Flags: blocking-aviary1.0mac+
Flags: blocking-aviary1.0PR+
Flags: blocking-aviary1.0+
Flags: blocking-aviary1.0mac+
Flags: blocking-aviary1.0PR+
Flags: blocking-aviary1.0+
*** Bug 275379 has been marked as a duplicate of this bug. ***
reproduced on trunk/Mac also, although hang, not crash. -> All
Hardware: PC → All
Loading Olivier's page causes a hang in Firefox 1.0.6 and Camino nightly build on OS X. Another test case: http://www.squarza.it/test/test.xml
*** Bug 303711 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.8beta4
Attached patch v1 (obsolete) — — Splinter Review
This seems to do the trick, have to verify it works in all cases. We could still suffer from the counter overflowing.
I still think we should use MAX_REFLOW_DEPTH, which is now in http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/public/nsIHTMLContentSink.h#91 Not sure if we should have our own define and set that to MAX_REFLOW_DEPTH, even. Just throwing in the number doesn't appeal to me.
(In reply to comment #18) > I still think we should use MAX_REFLOW_DEPTH Note that I didn't ask for review yet :-), my local tree has MAX_REFLOW_DEPTH.
Attached patch v1.1 — — Splinter Review
Attachment #194382 - Attachment is obsolete: true
Attachment #194860 - Flags: review?(axel)
Attachment #194860 - Flags: review?(axel) → review+
Attachment #194860 - Flags: superreview?(jst)
Comment on attachment 194860 [details] [diff] [review] v1.1 sr=jst
Attachment #194860 - Flags: superreview?(jst) → superreview+
Flags: blocking1.8b5?
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment on attachment 194860 [details] [diff] [review] v1.1 Crash fix in XSLT.
Comment on attachment 194860 [details] [diff] [review] v1.1 Crash fix in XSLT.
Attachment #194860 - Flags: approval1.8b5?
Attachment #194860 - Flags: approval1.8b5? → approval1.8b5+
plussing the blocking nomination since we approved the patch yesterday. But please get this in ASAP. Thanks.
Flags: blocking1.8b5? → blocking1.8b5+
Keywords: fixed1.8
*** Bug 308481 has been marked as a duplicate of this bug. ***
v.fixed on branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050929 Firefox/1.4
Keywords: fixed1.8 → verified1.8
*** Bug 316619 has been marked as a duplicate of this bug. ***
Crashtest added as part of http://hg.mozilla.org/mozilla-central/rev/afc662d52ab1
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: