Title: [136405] trunk
Revision
136405
Author
morr...@google.com
Date
2012-12-03 09:06:36 -0800 (Mon, 03 Dec 2012)

Log Message

Corrupted DOM tree during appendChild/insertBefore
https://bugs.webkit.org/show_bug.cgi?id=103601

Reviewed by Abhishek Arya.

Source/WebCore:

There are some missing protection in appendChild() and insertBefore().
This change added these.

Dromaeo dom-modify shows no speed regression (5445run/s before vs 5351run/s after)

Tests: fast/events/mutation-during-append-child.html
       fast/events/mutation-during-insert-before.html

* dom/ContainerNode.cpp:
(WebCore::checkAcceptChildGuaranteedNodeTypes):
(WebCore):
(WebCore::ContainerNode::insertBefore):
(WebCore::ContainerNode::appendChild):

LayoutTests:

* fast/events/mutation-during-append-child-expected.txt: Added.
* fast/events/mutation-during-append-child.html: Added.
* fast/events/mutation-during-insert-before-expected.txt: Added.
* fast/events/mutation-during-insert-before.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (136404 => 136405)


--- trunk/LayoutTests/ChangeLog	2012-12-03 17:01:57 UTC (rev 136404)
+++ trunk/LayoutTests/ChangeLog	2012-12-03 17:06:36 UTC (rev 136405)
@@ -1,3 +1,15 @@
+2012-12-03  Hajime Morrita  <morr...@google.com>
+
+        Corrupted DOM tree during appendChild/insertBefore
+        https://bugs.webkit.org/show_bug.cgi?id=103601
+
+        Reviewed by Abhishek Arya.
+
+        * fast/events/mutation-during-append-child-expected.txt: Added.
+        * fast/events/mutation-during-append-child.html: Added.
+        * fast/events/mutation-during-insert-before-expected.txt: Added.
+        * fast/events/mutation-during-insert-before.html: Added.
+
 2012-12-03  Jussi Kukkonen  <jussi.kukko...@intel.com>
 
         [GTK][EFL] Gardening for media/video-volume.html

Added: trunk/LayoutTests/fast/events/mutation-during-append-child-expected.txt (0 => 136405)


--- trunk/LayoutTests/fast/events/mutation-during-append-child-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/mutation-during-append-child-expected.txt	2012-12-03 17:06:36 UTC (rev 136405)
@@ -0,0 +1,10 @@
+Ensures that appendChild() throws an exception if mutation even handler does something wrong
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS newparent.appendChild(child); threw exception Error: HierarchyRequestError: DOM Exception 3.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/events/mutation-during-append-child.html (0 => 136405)


--- trunk/LayoutTests/fast/events/mutation-during-append-child.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/mutation-during-append-child.html	2012-12-03 17:06:36 UTC (rev 136405)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<div>
+  <div id="child"></div>
+  <div id="newparent"></div>
+</div>
+<script>
+description("Ensures that appendChild() throws an exception if mutation even handler does something wrong");
+
+var listener = function() {
+    document.removeEventListener("DOMNodeRemoved", listener, false);
+    child.appendChild(newparent);
+};
+document.addEventListener("DOMNodeRemoved", listener, false);
+shouldThrow("newparent.appendChild(child);",  "'Error: HierarchyRequestError: DOM Exception 3'");
+</script>
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/fast/events/mutation-during-insert-before-expected.txt (0 => 136405)


--- trunk/LayoutTests/fast/events/mutation-during-insert-before-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/mutation-during-insert-before-expected.txt	2012-12-03 17:06:36 UTC (rev 136405)
@@ -0,0 +1,10 @@
+Ensures that insertBefore() throws an exception if mutation even handler does something wrong
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS newparent.insertBefore(child, beforeChild); threw exception Error: HierarchyRequestError: DOM Exception 3.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/events/mutation-during-insert-before.html (0 => 136405)


--- trunk/LayoutTests/fast/events/mutation-during-insert-before.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/mutation-during-insert-before.html	2012-12-03 17:06:36 UTC (rev 136405)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<div>
+  <div id="child"></div>
+  <div id="newparent"><span id="beforeChild"></div>
+</div>
+<script>
+description("Ensures that insertBefore() throws an exception if mutation even handler does something wrong");
+
+var listener = function() {
+    document.removeEventListener("DOMNodeRemoved", listener, false);
+    child.appendChild(newparent);
+};
+document.addEventListener("DOMNodeRemoved", listener, false);
+shouldThrow("newparent.insertBefore(child, beforeChild);",  "'Error: HierarchyRequestError: DOM Exception 3'");
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (136404 => 136405)


--- trunk/Source/WebCore/ChangeLog	2012-12-03 17:01:57 UTC (rev 136404)
+++ trunk/Source/WebCore/ChangeLog	2012-12-03 17:06:36 UTC (rev 136405)
@@ -1,3 +1,24 @@
+2012-12-03  Hajime Morrita  <morr...@google.com>
+
+        Corrupted DOM tree during appendChild/insertBefore
+        https://bugs.webkit.org/show_bug.cgi?id=103601
+
+        Reviewed by Abhishek Arya.
+
+        There are some missing protection in appendChild() and insertBefore().
+        This change added these.
+
+        Dromaeo dom-modify shows no speed regression (5445run/s before vs 5351run/s after)
+
+        Tests: fast/events/mutation-during-append-child.html
+               fast/events/mutation-during-insert-before.html
+
+        * dom/ContainerNode.cpp:
+        (WebCore::checkAcceptChildGuaranteedNodeTypes):
+        (WebCore):
+        (WebCore::ContainerNode::insertBefore):
+        (WebCore::ContainerNode::appendChild):
+
 2012-12-03  Jocelyn Turcotte  <jocelyn.turco...@digia.com>
 
         Document::initSecurityContext() fails to call securityOrigin().grantLoadLocalResources()

Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (136404 => 136405)


--- trunk/Source/WebCore/dom/ContainerNode.cpp	2012-12-03 17:01:57 UTC (rev 136404)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp	2012-12-03 17:06:36 UTC (rev 136405)
@@ -177,6 +177,19 @@
     return 0;
 }
 
+static inline bool checkAcceptChildGuaranteedNodeTypes(ContainerNode* newParent, Node* newChild, ExceptionCode& ec)
+{
+    ASSERT(!newParent->isReadOnlyNode());
+    ASSERT(!newParent->isDocumentTypeNode());
+    ASSERT(isChildTypeAllowed(newParent, newChild));
+    if (newChild->contains(newParent)) {
+        ec = HIERARCHY_REQUEST_ERR;
+        return false;
+    }
+
+    return true;
+}
+
 static inline bool checkAddChild(ContainerNode* newParent, Node* newChild, ExceptionCode& ec)
 {
     if (ExceptionCode code = checkAcceptChild(newParent, newChild, 0)) {
@@ -233,6 +246,10 @@
     if (targets.isEmpty())
         return true;
 
+    // We need this extra check because collectChildrenAndRemoveFromOldParent() can fire mutation events.
+    if (!checkAcceptChildGuaranteedNodeTypes(this, newChild.get(), ec))
+        return false;
+
     InspectorInstrumentation::willInsertDOMNode(document(), this);
 
 #if ENABLE(MUTATION_OBSERVERS)
@@ -639,6 +656,10 @@
     if (targets.isEmpty())
         return true;
 
+    // We need this extra check because collectChildrenAndRemoveFromOldParent() can fire mutation events.
+    if (!checkAcceptChildGuaranteedNodeTypes(this, newChild.get(), ec))
+        return false;
+
     InspectorInstrumentation::willInsertDOMNode(document(), this);
 
 #if ENABLE(MUTATION_OBSERVERS)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to