Bug 30049 - Manipulating DOM from a script while parsing XHTML can cause a crash
Summary: Manipulating DOM from a script while parsing XHTML can cause a crash
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Major
Assignee: Alexey Proskuryakov
Keywords: InRadar
Depends on:
Reported: 2009-10-03 13:54 PDT by chameleon
Modified: 2019-02-06 09:04 PST (History)
2 users (show)

See Also:

A whole web site which reproduce the crash (5.77 KB, application/octet-stream)
2009-10-03 13:54 PDT, chameleon
no flags Details
Static website which reproduce the crash (5.72 KB, application/x-7z-compressed)
2009-10-04 22:25 PDT, chameleon
no flags Details
same test in zip format (7.67 KB, application/zip)
2009-10-07 22:16 PDT, Alexey Proskuryakov
no flags Details
reduced test case (will crash) (248 bytes, application/xhtml+xml)
2009-10-07 22:31 PDT, Alexey Proskuryakov
no flags Details
proposed fix (11.30 KB, patch)
2009-10-26 12:49 PDT, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chameleon 2009-10-03 13:54:11 PDT
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.
Comment 1 Mark Rowe (bdash) 2009-10-03 22:36:43 PDT
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.
Comment 2 chameleon 2009-10-04 22:25:05 PDT
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.
Comment 3 Alexey Proskuryakov 2009-10-07 22:16:08 PDT
Created attachment 40847 [details]
same test in zip format

Confirmed with r48940.
Comment 4 Alexey Proskuryakov 2009-10-07 22:31:08 PDT
Created attachment 40850 [details]
reduced test case (will crash)
Comment 5 Alexey Proskuryakov 2009-10-07 22:35:50 PDT
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.
Comment 6 Alexey Proskuryakov 2009-10-07 22:36:45 PDT
Comment 7 Alexey Proskuryakov 2009-10-26 12:49:22 PDT
Created attachment 41883 [details]
proposed fix
Comment 8 Darin Adler 2009-10-26 15:26:44 PDT
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.
Comment 9 Alexey Proskuryakov 2009-10-26 16:13:53 PDT
Committed <http://trac.webkit.org/changeset/50110>.
Comment 10 Lucas Forschler 2019-02-06 09:04:09 PST
Mass moving XML DOM bugs to the "DOM" Component.