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: