Last Comment Bug 460090 - Firefox crashes (segfault) on attempting to view XSL Transform (xml file with linked xsl) [ txMozillaXSLTProcessor::TransformToDoc ]
: Firefox crashes (segfault) on attempting to view XSL Transform (xml file with...
Status: RESOLVED DUPLICATE of bug 485217
[sg:dupe 485217]
: crash
Product: Core
Classification: Components
Component: XSLT (show other bugs)
: 1.9.0 Branch
: All All
: P1 critical (vote)
: ---
Assigned To: Martin
:
: Andrew Overholt [:overholt]
Mentors:
http://launchpadlibrarian.net/1645412...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-15 11:59 PDT by Mike Rooney
Modified: 2011-03-10 23:47 PST (History)
12 users (show)
jonas: wanted1.9.1+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Reduced testcase (326 bytes, application/xml)
2008-10-22 11:56 PDT, Martin
no flags Details
Fix and a couple of crash tests (2.21 KB, patch)
2008-11-29 04:15 PST, Martin
jonas: review+
jonas: superreview+
Details | Diff | Splinter Review
.hg/patches/bug460090 (1.65 KB, patch)
2008-12-11 15:35 PST, Martin
jonas: review+
Details | Diff | Splinter Review

Description Mike Rooney 2008-10-15 11:59:37 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.3) Gecko/2008101315 Ubuntu/8.10 (intrepid) Firefox/3.0.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.3) Gecko/2008101315 Ubuntu/8.10 (intrepid) Firefox/3.0.3

Opened from downstream https://bugs.launchpad.net/ubuntu/+source/firefox-3.0/+bug/253641.

Attempting to view http://launchpadlibrarian.net/16454122/test.xml causes a segfault, confirmed on multiple systems and multiple versions of Firefox. If the XSL is not linked, there is no problem, so it appears to be caused by the XSL.

Reproducible: Always

Steps to Reproduce:
1.Visit http://launchpadlibrarian.net/16454122/test.xml
Actual Results:  
Segfault.

Expected Results:  
No segfault. The XML should be displayed, applying the XSLT, or at least giving an error that it cannot be transformed.
Comment 1 Mardeg 2008-10-15 12:18:16 PDT
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081015 Minefield/3.1b2pre
Confirmed the crash. Please change the product to "Core", component to "XSLT"
Report should appear at http://crash-stats.mozilla.com/report/index/d62fe3c7-9aec-11dd-a839-0013211cbf8a
Comment 2 Thomas K. (:tom) 2008-10-15 12:37:44 PDT
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081014 Firefox/3.0.3pre

http://crash-stats.mozilla.com/report/index/95c38b5e-9af0-11dd-94b2-001cc45a2ce4
Comment 3 Dave Garrett 2008-10-15 13:24:45 PDT
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.3) Gecko/2008092416 Firefox/3.0.3
bp-fa156a2e-9af6-11dd-97b8-001a4bd43e5c
Comment 4 Ria Klaassen (not reading all bugmail) 2008-10-16 00:42:04 PDT
Tested with Windows XP, I see this crash also in older versions like Firefox 1.
Comment 5 Mark Omohundro 2008-10-16 11:16:47 PDT
It could be a problem with this in the xsl file:
 <xsl:for-each select="key('kat', @id)">
or with the related namespace possibly.

Try changing or removing this and see if it runs on FF 3.0.3.
Hope this helps.
Comment 6 Mark Omohundro 2008-10-17 08:34:22 PDT
a minor item(?) worth mentioning is UTF-8 encoding doesn't display the extended characters correctly (in my tests anyway). but ISO-8859-1 does as coded below....  why, does anybody know?  but not only in firefox -- other browsers do this as well -- so maybe some/many of us have the wrong idea of what chars UTF-8 supports??  or is there some other reason.  anybody?  just fyi, for what it is worth. hope it helps someone.

sample xml file that displays chars properly:

<?xml version="1.0" encoding="ISO-8859-1"?>
<sample>
    <data>Bücher</data>
    <data>Verfügbar</data>
</sample>
Comment 7 Martin 2008-10-22 11:56:25 PDT
Created attachment 344331 [details]
Reduced testcase

Looks to be a forwards-compatible processing issue, specifically with a key that uses a function with an unprefixed name that is not in the language.
Comment 8 Martin 2008-11-29 04:15:54 PST
Created attachment 350567 [details] [diff] [review]
Fix and a couple of crash tests

Problem is evalContext is stack-allocated but in the failure case remains referenced in the txExecutionState object, whose destructor later tries to delete it.
Comment 9 Jonas Sicking (:sicking) No longer reading bugmail consistently 2008-12-10 16:09:29 PST
Comment on attachment 350567 [details] [diff] [review]
Fix and a couple of crash tests

Could you write the test by doing

use="count(2)"

instead. That is sure to crash during evaluation rather than earlier.

And thanks for the patch!
Comment 10 Jonas Sicking (:sicking) No longer reading bugmail consistently 2008-12-10 16:10:26 PST
If you attach a patch with that fixed i'll mark it approved for 1.9.1. Preferably a mq patch for easy landing.
Comment 11 Martin 2008-12-11 15:35:36 PST
Created attachment 352626 [details] [diff] [review]
.hg/patches/bug460090

> Could you write the test by doing
>
> use="count(2)"
>
> instead. That is sure to crash during evaluation rather than earlier.

Done, though I don't quite understand why, or how Bug 143291 might affect things.

> If you attach a patch with that fixed i'll mark it approved for 1.9.1.
> Preferably a mq patch for easy landing.

Please feel free to add +a191 to the commit message and push to mozilla-central for me.
Comment 12 Martin 2009-01-11 02:17:00 PST
...or assign the bug to me and I'll bother someone else to commit it.
Comment 13 Martin 2009-02-14 11:42:02 PST
Comment on attachment 352626 [details] [diff] [review]
.hg/patches/bug460090

This bug seems to have fallen through the cracks, not sure what bugzilla incantation is the right one to get it noticed again, so asking for review of changed patch.
Comment 14 Jonas Sicking (:sicking) No longer reading bugmail consistently 2009-02-27 10:01:34 PST
Comment on attachment 352626 [details] [diff] [review]
.hg/patches/bug460090

This patch seems to be missing one of the tests from the previous patch. Was that intentional?
Comment 15 Martin 2009-02-27 13:24:23 PST
> Could you write the test by doing
>
> use="count(2)"
>
> instead. That is sure to crash during evaluation rather than earlier.

I took the singlar 'test' and the lack of mention of the extention function case to mean that you only wanted the forwards-compatible processing version.

By my reading of the spec, the original tests were valid, but I was trying to address your review without understanding what that comment meant.

Please edit whichever patch however you want if you feel the need.
Comment 16 Boris Zbarsky [:bz] (still a bit busy) 2009-03-26 12:35:36 PDT
So now that bug 485217 is fixed, is this fixed too?
Comment 17 Daniel Veditz [:dveditz] 2009-03-26 15:55:15 PDT
ugh, too bad we missed this. Duping forward because we checked in using the other bug.

*** This bug has been marked as a duplicate of bug 485217 ***
Comment 18 Martin 2009-03-26 22:53:08 PDT
There's not much point giving r+ and assigning to me *after* a seperate bug has been filed and fixed for an exploit using this.

Given this result, the time quibbling over tests that will never now be in tree seems a little pointless.
Comment 19 Jonas Sicking (:sicking) No longer reading bugmail consistently 2009-03-27 01:36:11 PDT
Indeed. I'm sorry i forgot about this bug until it was too late. The other discussions about a crash in XSLT made me remember this bug. Wasn't until afterwords that I realized it was the same bug even.

For what it's worth, it wasn't quibbling about the missing testcase. I intended to add that back myself.

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