Bug 30049 – Manipulating DOM from a script while parsing XHTML can cause a crash

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[chameleon]

Created attachment 40582 [details]
A whole web site which reproduce the crash

I attach a simplified website, programmed by me.
Website uses Javascript and XSLT 1.1.
Complete source code is available in attachment.

The attached website works fine on Firefox/Opera.
When load it with latest Safari, it crashes immediatelly.

So, I cannot provide more info.

[Mark Rowe (bdash)]

Thanks for filing this bug report.  Is there any chance that you can provide your attached reproducible case in a format that would be easier to reproduce?  Test cases that require a web server are naturally more difficult to work with than a simple HTML file.

[chameleon]

Created attachment 40612 [details]
Static website which reproduce the crash

PHP has only the header('.....'); dynamic.
All other content was static.
So I replace the attachment with a 100% static website.

[Alexey Proskuryakov]

Created attachment 40847 [details]
same test in zip format

Confirmed with r48940.

[Alexey Proskuryakov]

Created attachment 40850 [details]
reduced test case (will crash)

[Alexey Proskuryakov]

The issue here is that an element is removed while it's still being parsed. I'm not sure what the right behavior would be here.

[Alexey Proskuryakov]

<rdar://problem/7286002>

[Alexey Proskuryakov]

Created attachment 41883 [details]
proposed fix

[Darin Adler]

Comment on attachment 41883 [details]
proposed fix

> +        (WebCore::XMLTokenizer::setCurrentNode): Push the new node onto stack. If null is passed,
> +        then we're aborting; nuke the whole stack.

It seems strange to give setCurrentNode(0) this special behavior. Perhaps instead we could use a separate functions for this purpose. One could be called pushNode and the other could be called something else.

> +        (WebCore::XMLTokenizer::popCurrentNode): This is now called instead of setCurrentNode when
> +        exiting a node.

I'm not sure why the word "current" is needed in the name of this function.

r=me as is, but please consider getting rid of the two different meanings for setCurrentNode.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/50110>.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.