Title: [257036] trunk
Revision
257036
Author
commit-qu...@webkit.org
Date
2020-02-19 19:36:53 -0800 (Wed, 19 Feb 2020)

Log Message

Fix crash when Node::normalize() triggers mutation event that modifies child order
https://bugs.webkit.org/show_bug.cgi?id=207875
<rdar://58976682>

Patch by Sunny He <sunny...@apple.com> on 2020-02-19
Reviewed by Ryosuke Niwa.

When Node::normalize() merges two text nodes, it calls appendData
before textNodesMerged. If there is a mutator event registered, it
will fire on the call to appendData, potentially changing the child
order and causing a nullptr crash due to incorrect sibling pointers.
Reverse the order of these calls to ensure order gets correctly
updated.

Source/WebCore:

Test: fast/dom/Node/normalize-mutation-event.html

* dom/Node.cpp:
(WebCore::Node::normalize):

LayoutTests:

* fast/dom/Node/normalize-mutation-event-expected.txt: Added.
* fast/dom/Node/normalize-mutation-event.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (257035 => 257036)


--- trunk/LayoutTests/ChangeLog	2020-02-20 03:25:30 UTC (rev 257035)
+++ trunk/LayoutTests/ChangeLog	2020-02-20 03:36:53 UTC (rev 257036)
@@ -1,3 +1,21 @@
+2020-02-19  Sunny He  <sunny...@apple.com>
+
+        Fix crash when Node::normalize() triggers mutation event that modifies child order
+        https://bugs.webkit.org/show_bug.cgi?id=207875
+        <rdar://58976682>
+
+        Reviewed by Ryosuke Niwa.
+
+        When Node::normalize() merges two text nodes, it calls appendData
+        before textNodesMerged. If there is a mutator event registered, it
+        will fire on the call to appendData, potentially changing the child
+        order and causing a nullptr crash due to incorrect sibling pointers.
+        Reverse the order of these calls to ensure order gets correctly
+        updated.
+
+        * fast/dom/Node/normalize-mutation-event-expected.txt: Added.
+        * fast/dom/Node/normalize-mutation-event.html: Added.
+
 2020-02-19  Diego Pino Garcia  <dp...@igalia.com>
 
         [GTK][WPE] Gardening, update baselines of websocket tests

Added: trunk/LayoutTests/fast/dom/Node/normalize-mutation-event-expected.txt (0 => 257036)


--- trunk/LayoutTests/fast/dom/Node/normalize-mutation-event-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Node/normalize-mutation-event-expected.txt	2020-02-20 03:36:53 UTC (rev 257036)
@@ -0,0 +1 @@
+Test that normalize() behaves correctly in the presence of a mutation event that modifies the child order. PASS if test does not crash. abc

Added: trunk/LayoutTests/fast/dom/Node/normalize-mutation-event.html (0 => 257036)


--- trunk/LayoutTests/fast/dom/Node/normalize-mutation-event.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Node/normalize-mutation-event.html	2020-02-20 03:36:53 UTC (rev 257036)
@@ -0,0 +1,18 @@
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    function eventhandler() {
+        document.body.appendChild(document.body.firstChild);
+    }
+    function run() {
+        document.caretRangeFromPoint(0,0);
+        document.body.firstChild.after("abc")
+        document.body.firstChild.addEventListener("DOMSubtreeModified", eventhandler);
+        element.getRootNode().normalize();
+    }
+</script>
+<body _onload_=run()>
+    Test that normalize() behaves correctly in the presence of a mutation event that modifies the child order. PASS if test does not crash.
+    <div id="element"></div>
+</body>

Modified: trunk/Source/WebCore/ChangeLog (257035 => 257036)


--- trunk/Source/WebCore/ChangeLog	2020-02-20 03:25:30 UTC (rev 257035)
+++ trunk/Source/WebCore/ChangeLog	2020-02-20 03:36:53 UTC (rev 257036)
@@ -1,3 +1,23 @@
+2020-02-19  Sunny He  <sunny...@apple.com>
+
+        Fix crash when Node::normalize() triggers mutation event that modifies child order
+        https://bugs.webkit.org/show_bug.cgi?id=207875
+        <rdar://58976682>
+
+        Reviewed by Ryosuke Niwa.
+
+        When Node::normalize() merges two text nodes, it calls appendData
+        before textNodesMerged. If there is a mutator event registered, it
+        will fire on the call to appendData, potentially changing the child
+        order and causing a nullptr crash due to incorrect sibling pointers.
+        Reverse the order of these calls to ensure order gets correctly
+        updated.
+
+        Test: fast/dom/Node/normalize-mutation-event.html
+
+        * dom/Node.cpp:
+        (WebCore::Node::normalize):
+
 2020-02-19  Peng Liu  <peng.l...@apple.com>
 
         Fix check-webkit-style errors related to AVFoundationSPI.h

Modified: trunk/Source/WebCore/dom/Node.cpp (257035 => 257036)


--- trunk/Source/WebCore/dom/Node.cpp	2020-02-20 03:25:30 UTC (rev 257035)
+++ trunk/Source/WebCore/dom/Node.cpp	2020-02-20 03:36:53 UTC (rev 257036)
@@ -670,8 +670,13 @@
 
             // Both non-empty text nodes. Merge them.
             unsigned offset = text->length();
-            text->appendData(nextText->data());
+
+            // Update start/end for any affected Ranges before appendData since modifying contents might trigger mutation events that modify ordering.
             document().textNodesMerged(nextText, offset);
+
+            // FIXME: DOM spec requires contents to be replaced all at once (see https://dom.spec.whatwg.org/#dom-node-normalize).
+            // Appending once per sibling may trigger mutation events too many times.
+            text->appendData(nextText->data());            
             nextText->remove();
         }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to