xml, xslt crash using apply-templates and document

RESOLVED FIXED in mozilla1.8beta4

Status

()

P2
critical
RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: ian, Assigned: peterv)

Tracking

({crash, testcase, verified1.8})

Trunk
mozilla1.8beta4
crash, testcase, verified1.8
Points:
---
Bug Flags:
blocking1.8b5 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: nostackwanted, URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

15 years ago
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

Comment 1

15 years ago
confirming crash 20031120 Win2k, no stacktrace yet though.
Keywords: crash, stackwanted

Comment 2

15 years ago
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....

Comment 5

15 years ago
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

Comment 6

15 years ago
I'm crashing on Win2k with a PIII/900MHz, 384MB RAM (and 576 Virtual memory).
OS: Linux → All
(Reporter)

Comment 7

15 years ago
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

Comment 8

15 years ago
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.

Comment 10

15 years ago
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?

Updated

15 years ago
Keywords: testcase
Whiteboard: TB25925804M → TB17805K

Comment 12

15 years ago
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

Updated

14 years ago
Flags: blocking-aviary1.0mac+
Flags: blocking-aviary1.0PR+
Flags: blocking-aviary1.0+

Updated

14 years ago
Flags: blocking-aviary1.0mac+
Flags: blocking-aviary1.0PR+
Flags: blocking-aviary1.0+

Comment 13

14 years ago
*** Bug 275379 has been marked as a duplicate of this bug. ***

Comment 14

14 years ago
reproduced on trunk/Mac also, although hang, not crash.
-> All
Hardware: PC → All

Comment 15

13 years ago
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
(Assignee)

Comment 16

13 years ago
*** Bug 303711 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.8beta4
(Assignee)

Comment 17

13 years ago
Created attachment 194382 [details] [diff] [review]
v1

This seems to do the trick, have to verify it works in all cases. We could
still suffer from the counter overflowing.

Comment 18

13 years ago
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.
(Assignee)

Comment 19

13 years ago
(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.
(Assignee)

Comment 20

13 years ago
Created attachment 194860 [details] [diff] [review]
v1.1
Attachment #194382 - Attachment is obsolete: true
Attachment #194860 - Flags: review?(axel)

Updated

13 years ago
Attachment #194860 - Flags: review?(axel) → review+
(Assignee)

Updated

13 years ago
Attachment #194860 - Flags: superreview?(jst)
Comment on attachment 194860 [details] [diff] [review]
v1.1

sr=jst
Attachment #194860 - Flags: superreview?(jst) → superreview+
(Assignee)

Updated

13 years ago
Flags: blocking1.8b5?
(Assignee)

Updated

13 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Comment 22

13 years ago
Comment on attachment 194860 [details] [diff] [review]
v1.1

Crash fix in XSLT.
(Assignee)

Comment 23

13 years ago
Comment on attachment 194860 [details] [diff] [review]
v1.1

Crash fix in XSLT.
Attachment #194860 - Flags: approval1.8b5?

Updated

13 years ago
Attachment #194860 - Flags: approval1.8b5? → approval1.8b5+

Comment 24

13 years ago
plussing the blocking nomination since we approved the patch yesterday. But
please get this in ASAP. Thanks.
Flags: blocking1.8b5? → blocking1.8b5+
(Assignee)

Updated

13 years ago
Keywords: fixed1.8

Comment 25

13 years ago
*** Bug 308481 has been marked as a duplicate of this bug. ***

Comment 26

13 years ago
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. ***

Comment 28

10 years ago
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.