Bug 15456 - XML parser modifies the document when using foo.innerHtml = "some string"
Summary: XML parser modifies the document when using foo.innerHtml = "some string"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 523.x (Safari 3)
Hardware: Other OS X 10.4
: P1 Major
Assignee: Nobody
URL:
Keywords:
: 15455 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-10-10 14:07 PDT by Lars Knoll
Modified: 2007-10-21 22:25 PDT (History)
2 users (show)

See Also:


Attachments
Protects some of the calls into the document (2.85 KB, patch)
2007-10-10 14:10 PDT, Lars Knoll
no flags Details | Formatted Diff | Diff
Test case for QXML (1.51 KB, application/xhtml+xml)
2007-10-11 01:22 PDT, Eric Seidel (no email)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Knoll 2007-10-10 14:07:43 PDT
The XMLTokenizer.cpp has a constructor that takes a document fragment and parses XML into this fragment (which is used at least for handling innerHtml, maybe other places as well).

While parsing this fragment, the parser calls lots of methods on the document, amongst others finishedParsing(), which can lead to memory corruption when innerHtml is used form within the onload handler.
Comment 1 Lars Knoll 2007-10-10 14:10:56 PDT
Created attachment 16617 [details]
Protects some of the calls into the document

This patch fixes the memory corruption seen with LayoutTests/fast/innerHTML/innerHTML-script-tag-crash.xhtml. It does however not clean up all issues, and is untested for builds that use the libxml based parser.

The main issues remaining are things related to XSLT support (see XMLTokenizer.cpp around line 1210).
Comment 2 Eric Seidel (no email) 2007-10-10 14:13:42 PDT
This is a reproducible memory smasher as demonstrated by the valgrind output:
http://paste.lisp.org/display/48987
Comment 3 Alexey Proskuryakov 2007-10-11 00:54:07 PDT
*** Bug 15455 has been marked as a duplicate of this bug. ***
Comment 4 Eric Seidel (no email) 2007-10-11 01:22:37 PDT
Created attachment 16624 [details]
Test case for QXML

After extensive investigation, I have decided that this bug does not affect the libxml based parser.  These changes are not harmful to the libxml based parser, but they are not necessary.  libxml doesn't set any of the touched methods as handlers during the parseXMLDocumentFragment codepath.
Comment 5 Mark Rowe (bdash) 2007-10-11 10:14:29 PDT
Has this been landed?
Comment 6 David Kilzer (:ddkilzer) 2007-10-12 05:23:35 PDT
(In reply to comment #5)
> Has this been landed?

Apparently it landed in r26356.

http://trac.webkit.org/projects/webkit/changeset/26356

I don't see any indication of a review in the ChangeLog, however.  Perhaps the review was given via IRC?

Comment 7 Eric Seidel (no email) 2007-10-21 22:25:25 PDT
This can be closed.