2011-06-08 Adam Barth <abarth@webkit.org>

        Reviewed by Eric Seidel.

        Use after free in WebCore::ContainerNode::parserAddChild
        https://bugs.webkit.org/show_bug.cgi?id=62160

        Test that we don't trigger asserts when re-entering the parser from
        tree construction.

        * fast/parser/document-write-onload-nesting-expected.txt: Added.
        * fast/parser/document-write-onload-nesting.html: Added.
        * fast/parser/document-write-onload-ordering-expected.txt: Added.
        * fast/parser/document-write-onload-ordering.html: Added.
            - The exact ordering of the script execution here differs a bit
              between browsers.  For example, Firefox executes the scripts in a
              slightly different order because Firefox runs the parser on a
              separate thread (and therefore cannot be re-entered from tree
              construction). If/when we move the parser off the main thread,
              we're likely to change the ordering here a bit, which should be
              ok.
2011-06-08  Adam Barth  <abarth@webkit.org>

        Reviewed by Eric Seidel.

        constructTreeFromToken can re-enter parser, causing ASSERTs
        https://bugs.webkit.org/show_bug.cgi?id=62160

        This patch clears the HTMLToken before constructing the tree from the
        token, putting the HTMLDocumentParser in a good state to be re-entered.

        Tests: fast/parser/document-write-onload-nesting.html
               fast/parser/document-write-onload-ordering.html

        * html/parser/HTMLDocumentParser.cpp:
        (WebCore::HTMLDocumentParser::pumpTokenizer):
        * html/parser/HTMLToken.h:
        (WebCore::HTMLToken::isUninitialized):
        * html/parser/HTMLTreeBuilder.cpp:
        (WebCore::HTMLTreeBuilder::constructTreeFromToken):

git-svn-id: svn://svn.chromium.org/blink/trunk@88411 bbb929c8-8fbe-4397-9dbb-9b2b20218538

9 files changed, 86 insertions, 1 deletions

diff --git a/third_party/WebKit/LayoutTests/ChangeLog b/third_party/WebKit/LayoutTests/ChangeLog
index e323f31..49bfa07 100644
--- a/third_party/WebKit/LayoutTests/ChangeLog
+++ b/third_party/WebKit/LayoutTests/ChangeLog
@@ -1,3 +1,25 @@
+2011-06-08  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Use after free in WebCore::ContainerNode::parserAddChild
+        https://bugs.webkit.org/show_bug.cgi?id=62160
+
+        Test that we don't trigger asserts when re-entering the parser from
+        tree construction.
+
+        * fast/parser/document-write-onload-nesting-expected.txt: Added.
+        * fast/parser/document-write-onload-nesting.html: Added.
+        * fast/parser/document-write-onload-ordering-expected.txt: Added.
+        * fast/parser/document-write-onload-ordering.html: Added.
+            - The exact ordering of the script execution here differs a bit
+              between browsers.  For example, Firefox executes the scripts in a
+              slightly different order because Firefox runs the parser on a
+              separate thread (and therefore cannot be re-entered from tree
+              construction). If/when we move the parser off the main thread,
+              we're likely to change the ordering here a bit, which should be
+              ok.
+
 2011-06-08  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add PASS expectations to two tests that have been passing on
diff --git a/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-nesting-expected.txt b/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-nesting-expected.txt
new file mode 100644
index 0000000..9976b12
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-nesting-expected.txt
@@ -0,0 +1,3 @@
+PASS
+
+
diff --git a/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-nesting.html b/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-nesting.html
new file mode 100644
index 0000000..eaf069d
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-nesting.html
@@ -0,0 +1,5 @@
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+<iframe onload="document.write('<p>PASS<iframe onload=&quot;document.write(\'<p>\')&quot;></iframe>');"></iframe>
diff --git a/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-ordering-expected.txt b/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-ordering-expected.txt
new file mode 100644
index 0000000..301c1cb
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-ordering-expected.txt
@@ -0,0 +1,9 @@
+ALERT: 0
+ALERT: 1
+ALERT: 2
+ALERT: 3
+ALERT: 4
+ALERT: 5
+PASS
+
+
diff --git a/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-ordering.html b/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-ordering.html
new file mode 100644
index 0000000..e71ba73
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/parser/document-write-onload-ordering.html
@@ -0,0 +1,5 @@
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+<iframe onload="alert(0);document.write('<p>PASS<iframe onload=&quot;alert(1);document.write(\'<p><iframe onload=alert(3)></iframe>\');alert(4);&quot;></iframe><iframe onload=alert(2)></iframe>');alert(5);document.close();"></iframe>
diff --git a/third_party/WebKit/Source/WebCore/ChangeLog b/third_party/WebKit/Source/WebCore/ChangeLog
index 31dd587..a69f967 100644
--- a/third_party/WebKit/Source/WebCore/ChangeLog
+++ b/third_party/WebKit/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2011-06-08  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        constructTreeFromToken can re-enter parser, causing ASSERTs
+        https://bugs.webkit.org/show_bug.cgi?id=62160
+
+        This patch clears the HTMLToken before constructing the tree from the
+        token, putting the HTMLDocumentParser in a good state to be re-entered.
+
+        Tests: fast/parser/document-write-onload-nesting.html
+               fast/parser/document-write-onload-ordering.html
+
+        * html/parser/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::pumpTokenizer):
+        * html/parser/HTMLToken.h:
+        (WebCore::HTMLToken::isUninitialized):
+        * html/parser/HTMLTreeBuilder.cpp:
+        (WebCore::HTMLTreeBuilder::constructTreeFromToken):
+
 2011-06-08  Kent Tamura  <tkent@chromium.org>
 
         Fix Qt build for r88405.
diff --git a/third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp b/third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp
index f855ce2..5ff7e08 100644
--- a/third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp
+++ b/third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp
@@ -274,7 +274,7 @@ void HTMLDocumentParser::pumpTokenizer(SynchronousMode mode)
         }
 
         m_treeBuilder->constructTreeFromToken(m_token);
-        m_token.clear();
+        ASSERT(m_token.isUninitialized());
     }
 
     // Ensure we haven't been totally deref'ed after pumping. Any caller of this
diff --git a/third_party/WebKit/Source/WebCore/html/parser/HTMLToken.h b/third_party/WebKit/Source/WebCore/html/parser/HTMLToken.h
index 49ec312..59f7ed4 100644
--- a/third_party/WebKit/Source/WebCore/html/parser/HTMLToken.h
+++ b/third_party/WebKit/Source/WebCore/html/parser/HTMLToken.h
@@ -73,6 +73,8 @@ public:
         m_data.clear();
     }
 
+    bool isUninitialized() { return m_type == Uninitialized; }
+
     int startIndex() const { return m_range.m_start; }
     int endIndex() const { return m_range.m_end; }
 
diff --git a/third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp b/third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp
index f83026b..afbe2cc 100644
--- a/third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp
+++ b/third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp
@@ -435,7 +435,26 @@ PassRefPtr<Element> HTMLTreeBuilder::takeScriptToProcess(TextPosition1& scriptSt
 void HTMLTreeBuilder::constructTreeFromToken(HTMLToken& rawToken)
 {
     AtomicHTMLToken token(rawToken);
+
+    // We clear the rawToken in case constructTreeFromAtomicToken
+    // synchronously re-enters the parser. We don't clear the token immedately
+    // for Character tokens because the AtomicHTMLToken avoids copying the
+    // characters by keeping a pointer to the underlying buffer in the
+    // HTMLToken. Fortuantely, Character tokens can't cause use to re-enter
+    // the parser.
+    //
+    // FIXME: Top clearing the rawToken once we start running the parser off
+    // the main thread or once we stop allowing synchronous JavaScript
+    // execution from parseMappedAttribute.
+    if (rawToken.type() != HTMLToken::Character)
+        rawToken.clear();
+
     constructTreeFromAtomicToken(token);
+
+    if (!rawToken.isUninitialized()) {
+        ASSERT(rawToken.type() == HTMLToken::Character);
+        rawToken.clear();
+    }
 }
 
 void HTMLTreeBuilder::constructTreeFromAtomicToken(AtomicHTMLToken& token)