431703 - Crash [@ nsXMLContentSink::HandleEndElement] setting innerHTML with SVG, onload

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

Loading the testcase makes Firefox crash with a null mDocument at nsXMLContentSink::HandleEndElement:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xml/document/src/nsXMLContentSink.cpp&rev=1.442&mark=1172#1157

Based on code inspection, I think this is a regression from bug 380417.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Doubt it.  The code dereferenced mDocument before as well, and nsXMLFragmentContentSink simply never calls Init() and hence never sets mDocument as far as I can tell.  So any use of mDocument in the XML sink that doesn't null-check it would crash in innerHTML sets if it gets hit.

This code just needs a null-check, and has all along.

Comment 2

[Jesse Ruderman]

You're right.  Sorry for blaming you incorrectly.

Comment 3

[Robert Longson [:longsonr]]

Attachment: null check

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 318988 [details] [diff] [review]
null check

Makes sense.  I doubt anyone expects this event to fire inside innerHTML sets.

Comment 5

[Robert Longson [:longsonr]]

Comment on attachment 318988 [details] [diff] [review]
null check

Simple null check to prevent null dereference crash.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 318988 [details] [diff] [review]
null check

a1.9=beltzner

Comment 7

[Reed Loden [:reed]]

It's "checkin-needed", and it's a keyword, not a status whiteboard note. :)

Comment 8

[Reed Loden [:reed]]

Checking in content/xml/document/src/nsXMLContentSink.cpp;
/cvsroot/mozilla/content/xml/document/src/nsXMLContentSink.cpp,v  <--  nsXMLContentSink.cpp
new revision: 1.443; previous revision: 1.442
done

Comment 9

[Jeff Walden [:Waldo]]

Can someone crashtestify this?

Comment 10

[Jesse Ruderman]

Added crashtest:
http://hg.mozilla.org/mozilla-central/rev/63120a5301f2