[webkit-changes] [88411] trunk

Wed, 08 Jun 2011 17:34:45 -0700

Title: [88411] trunk
Revision
88411
Author
[email protected]
Date
2011-06-08 17:34:13 -0700 (Wed, 08 Jun 2011)

Log Message

2011-06-08  Adam Barth  <[email protected]>

        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  <[email protected]>

        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):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (88410 => 88411)


--- trunk/LayoutTests/ChangeLog	2011-06-09 00:20:26 UTC (rev 88410)
+++ trunk/LayoutTests/ChangeLog	2011-06-09 00:34:13 UTC (rev 88411)
@@ -1,3 +1,25 @@
+2011-06-08  Adam Barth  <[email protected]>
+
+        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  <[email protected]>
 
         Add PASS expectations to two tests that have been passing on

Added: trunk/LayoutTests/fast/parser/document-write-onload-nesting-expected.txt (0 => 88411)


--- trunk/LayoutTests/fast/parser/document-write-onload-nesting-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/parser/document-write-onload-nesting-expected.txt	2011-06-09 00:34:13 UTC (rev 88411)
@@ -0,0 +1,3 @@
+PASS
+
+

Added: trunk/LayoutTests/fast/parser/document-write-onload-nesting.html (0 => 88411)


--- trunk/LayoutTests/fast/parser/document-write-onload-nesting.html	                        (rev 0)
+++ trunk/LayoutTests/fast/parser/document-write-onload-nesting.html	2011-06-09 00:34:13 UTC (rev 88411)
@@ -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>

Added: trunk/LayoutTests/fast/parser/document-write-onload-ordering-expected.txt (0 => 88411)


--- trunk/LayoutTests/fast/parser/document-write-onload-ordering-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/parser/document-write-onload-ordering-expected.txt	2011-06-09 00:34:13 UTC (rev 88411)
@@ -0,0 +1,9 @@
+ALERT: 0
+ALERT: 1
+ALERT: 2
+ALERT: 3
+ALERT: 4
+ALERT: 5
+PASS
+
+

Added: trunk/LayoutTests/fast/parser/document-write-onload-ordering.html (0 => 88411)


--- trunk/LayoutTests/fast/parser/document-write-onload-ordering.html	                        (rev 0)
+++ trunk/LayoutTests/fast/parser/document-write-onload-ordering.html	2011-06-09 00:34:13 UTC (rev 88411)
@@ -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>

Modified: trunk/Source/WebCore/ChangeLog (88410 => 88411)


--- trunk/Source/WebCore/ChangeLog	2011-06-09 00:20:26 UTC (rev 88410)
+++ trunk/Source/WebCore/ChangeLog	2011-06-09 00:34:13 UTC (rev 88411)
@@ -1,3 +1,23 @@
+2011-06-08  Adam Barth  <[email protected]>
+
+        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  <[email protected]>
 
         Fix Qt build for r88405.

Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp (88410 => 88411)


--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp	2011-06-09 00:20:26 UTC (rev 88410)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp	2011-06-09 00:34:13 UTC (rev 88411)
@@ -274,7 +274,7 @@
         }
 
         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

Modified: trunk/Source/WebCore/html/parser/HTMLToken.h (88410 => 88411)


--- trunk/Source/WebCore/html/parser/HTMLToken.h	2011-06-09 00:20:26 UTC (rev 88410)
+++ trunk/Source/WebCore/html/parser/HTMLToken.h	2011-06-09 00:34:13 UTC (rev 88411)
@@ -73,6 +73,8 @@
         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; }

Modified: trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp (88410 => 88411)


--- trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp	2011-06-09 00:20:26 UTC (rev 88410)
+++ trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp	2011-06-09 00:34:13 UTC (rev 88411)
@@ -435,7 +435,26 @@
 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)

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to