184697 - Crash on XML that uses a namespace that's not declared

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Boris Zbarsky [:bzbarsky]]

The basic problem is that if I use an element like

<foo:bar>

in XML and do not have an xmlns:foo="something", we will crash because the
PR_ASSERT in nsXMLContentSink::HandleEndElement triggers (the one that asserts
eXMLContentSinkState_InDocumentElement == mState).

It does not seem to matter what else is in the document....  Minimal testcase is
an XML document containing the 6 chars:

<a:b/>

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, if I uncomment the PR_ASSERT then PopContent() returns null (asserting as it
does so) and we crash in nsXMLContentSink::CloseElement after asserting that
we've got nothing to close.....  

Comment 3

[Boris Zbarsky [:bzbarsky]]

*** Bug 188026 has been marked as a duplicate of this bug. ***

Comment 4

[Asa Dotzler [:asa]]

Sounds like this has the potential to become a topcrash. Harish, can you take a
look at this for fixing in 1.3beta (about 2 weeks away).

Comment 5

[harishd]

Attachment: Patch v1.0

Added a couple of lines that got yanked when sicking landed the pretty printing
changes.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 111450 [details] [diff] [review]
Patch v1.0

this shouldn't be needed. If no factory is registered we set up a default
factory that always calls NS_NewXMLElement(aResult, aNodeInfo);

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

ooh, i understand what's happening. When the prefix is undeclared nameSpaceID
will be -1 (kNameSpaceID_Unknown) and GetElementFactory will return null.

IMHO a better fix would be to let GetElementFactory return the default factory
for that namespace.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

actually we should abort the parsing if this occurs (I have that filed as a bug
somewhere), but for now we will have to work around it.

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: alternative patch

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

harish: which approach do you think we should go with?

Comment 11

[harishd]

Jonas: I like your patch, however, you should addref aElementFactory before
returning. no?

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: ...with an addref

thanks! of course i thould addref

Comment 13

[harishd]

Comment on attachment 111589 [details] [diff] [review]
...with an addref

r=harishd

Comment 14

[harishd]

--> sicking 

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

checked in. Thanks for catching this and for the reviews