Reparenting during a mutation event inside appendChild could result in a circular...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Mar 2019 22:25:44 +0000 (22:25 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Mar 2019 22:25:44 +0000 (22:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192825

Reviewed by Zalan Bujtas.

Source/WebCore:

The bug was caused by appendChildWithoutPreInsertionValidityCheck, insertBefore and replaceChild
checking the circular dependency against newChild instead of targets even though when newChild
is a document fragment, appendChildWithoutPreInsertionValidityCheck inserts the children of
the document fragment. Fixed the bug by checking the circular dependency against each target child.

Also fixed the bug that checkAcceptChildGuaranteedNodeTypes was not considering shadow inclusive
ancestors or template host elements.

Tests: fast/dom/append-child-with-mutation-event-removal-and-circular-insertion.html
       fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion.html
       fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion.html
       fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion.html
       fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion.html
       fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion.html
       fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion.html
       fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion.html
       fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion.html

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

LayoutTests:

Added regression tests.

* fast/dom/append-child-with-mutation-event-removal-and-circular-insertion-expected.txt: Added.
* fast/dom/append-child-with-mutation-event-removal-and-circular-insertion.html: Added.
* fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt: Added.
* fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion.html: Added.
* fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt: Added.
* fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion.html: Added.
* fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion-expected.txt: Added.
* fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion.html: Added.
* fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt: Added.
* fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion.html: Added.
* fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt: Added.
* fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion.html: Added.
* fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion-expected.txt: Added.
* fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion.html: Added.
* fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt: Added.
* fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion.html: Added.
* fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt: Added.
* fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243175 268f45cc-cd09-0410-ab3c-d52691b4dbfc

21 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-insertion-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-insertion.html [new file with mode: 0644]
LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion.html [new file with mode: 0644]
LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion.html [new file with mode: 0644]
LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion.html [new file with mode: 0644]
LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion.html [new file with mode: 0644]
LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion.html [new file with mode: 0644]
LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion.html [new file with mode: 0644]
LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion.html [new file with mode: 0644]
LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/ContainerNode.cpp

index 939472d..b835e4f 100644 (file)
@@ -1,3 +1,31 @@
+2019-03-19  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reparenting during a mutation event inside appendChild could result in a circular DOM tree
+        https://bugs.webkit.org/show_bug.cgi?id=192825
+
+        Reviewed by Zalan Bujtas.
+
+        Added regression tests.
+
+        * fast/dom/append-child-with-mutation-event-removal-and-circular-insertion-expected.txt: Added.
+        * fast/dom/append-child-with-mutation-event-removal-and-circular-insertion.html: Added.
+        * fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt: Added.
+        * fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion.html: Added.
+        * fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt: Added.
+        * fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion.html: Added.
+        * fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion-expected.txt: Added.
+        * fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion.html: Added.
+        * fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt: Added.
+        * fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion.html: Added.
+        * fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt: Added.
+        * fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion.html: Added.
+        * fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion-expected.txt: Added.
+        * fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion.html: Added.
+        * fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt: Added.
+        * fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion.html: Added.
+        * fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt: Added.
+        * fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion.html: Added.
+
 2019-03-19  Timothy Hatcher  <timothy@apple.com>
 
         REGRESSION (r239904): Update dark mode defines in a few places that got missed.
diff --git a/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-insertion-expected.txt b/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-insertion-expected.txt
new file mode 100644 (file)
index 0000000..9dd24de
--- /dev/null
@@ -0,0 +1,13 @@
+This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.
+WebKit should detect this case and throw HierarchyRequestError.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.appendChild(fragment) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS container.parentNode is child
+PASS child.parentNode is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-insertion.html b/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-insertion.html
new file mode 100644 (file)
index 0000000..055291f
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.<br>
+WebKit should detect this case and throw HierarchyRequestError.`);
+
+fragment = document.createDocumentFragment();
+container = document.createElement('div');
+child = fragment.appendChild(document.createElement('div'));
+fragment.addEventListener('DOMSubtreeModified', function() {
+  child.appendChild(container);
+});
+
+shouldThrowErrorName(`container.appendChild(fragment)`, 'HierarchyRequestError');
+shouldBe('container.parentNode', 'child');
+shouldBe('child.parentNode', 'null');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt b/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt
new file mode 100644 (file)
index 0000000..d52ae3e
--- /dev/null
@@ -0,0 +1,13 @@
+This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.
+WebKit should detect this case and throw HierarchyRequestError.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.appendChild(fragment) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS container.parentNode is childShadowRoot
+PASS child.parentNode is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion.html b/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion.html
new file mode 100644 (file)
index 0000000..13fbb3c
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.<br>
+WebKit should detect this case and throw HierarchyRequestError.`);
+
+fragment = document.createDocumentFragment();
+container = document.createElement('div');
+child = fragment.appendChild(document.createElement('div'));
+childShadowRoot = child.attachShadow({mode: 'closed'});
+fragment.addEventListener('DOMSubtreeModified', function() {
+    childShadowRoot.appendChild(container);
+});
+
+shouldThrowErrorName(`container.appendChild(fragment)`, 'HierarchyRequestError');
+shouldBe('container.parentNode', 'childShadowRoot');
+shouldBe('child.parentNode', 'null');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt b/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt
new file mode 100644 (file)
index 0000000..fa39739
--- /dev/null
@@ -0,0 +1,13 @@
+This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.
+WebKit should detect this case and throw HierarchyRequestError.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.appendChild(fragment) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS container.parentNode is child.content
+PASS child.parentNode is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion.html b/LayoutTests/fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion.html
new file mode 100644 (file)
index 0000000..0601bd9
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.<br>
+WebKit should detect this case and throw HierarchyRequestError.`);
+
+fragment = document.createDocumentFragment();
+container = document.createElement('div');
+child = fragment.appendChild(document.createElement('template'));
+fragment.addEventListener('DOMSubtreeModified', function() {
+    child.content.appendChild(container);
+});
+
+shouldThrowErrorName(`container.appendChild(fragment)`, 'HierarchyRequestError');
+shouldBe('container.parentNode', 'child.content');
+shouldBe('child.parentNode', 'null');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion-expected.txt b/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion-expected.txt
new file mode 100644 (file)
index 0000000..4615c33
--- /dev/null
@@ -0,0 +1,13 @@
+This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.
+WebKit should detect this case and throw HierarchyRequestError.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.insertBefore(fragment, refChild) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS container.parentNode is child
+PASS child.parentNode is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion.html b/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion.html
new file mode 100644 (file)
index 0000000..113358c
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.<br>
+WebKit should detect this case and throw HierarchyRequestError.`);
+
+fragment = document.createDocumentFragment();
+container = document.createElement('div');
+refChild = container.appendChild(document.createElement('section'));
+child = fragment.appendChild(document.createElement('div'));
+fragment.addEventListener('DOMSubtreeModified', function() {
+    child.appendChild(container);
+});
+
+shouldThrowErrorName(`container.insertBefore(fragment, refChild)`, 'HierarchyRequestError');
+shouldBe('container.parentNode', 'child');
+shouldBe('child.parentNode', 'null');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt b/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt
new file mode 100644 (file)
index 0000000..69a21b3
--- /dev/null
@@ -0,0 +1,13 @@
+This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.
+WebKit should detect this case and throw HierarchyRequestError.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.insertBefore(fragment, refChild) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS container.parentNode is childShadowRoot
+PASS child.parentNode is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion.html b/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion.html
new file mode 100644 (file)
index 0000000..7bb44b0
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.<br>
+WebKit should detect this case and throw HierarchyRequestError.`);
+
+fragment = document.createDocumentFragment();
+container = document.createElement('div');
+refChild = container.appendChild(document.createElement('section'));
+child = fragment.appendChild(document.createElement('div'));
+childShadowRoot = child.attachShadow({mode: 'closed'});
+fragment.addEventListener('DOMSubtreeModified', function() {
+    childShadowRoot.appendChild(container);
+});
+
+shouldThrowErrorName(`container.insertBefore(fragment, refChild)`, 'HierarchyRequestError');
+shouldBe('container.parentNode', 'childShadowRoot');
+shouldBe('child.parentNode', 'null');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt b/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt
new file mode 100644 (file)
index 0000000..15853b6
--- /dev/null
@@ -0,0 +1,13 @@
+This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.
+WebKit should detect this case and throw HierarchyRequestError.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.insertBefore(fragment, refChild) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS container.parentNode is child.content
+PASS child.parentNode is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion.html b/LayoutTests/fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion.html
new file mode 100644 (file)
index 0000000..9948b85
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.<br>
+WebKit should detect this case and throw HierarchyRequestError.`);
+
+fragment = document.createDocumentFragment();
+container = document.createElement('div');
+refChild = container.appendChild(document.createElement('section'));
+child = fragment.appendChild(document.createElement('template'));
+fragment.addEventListener('DOMSubtreeModified', function() {
+    child.content.appendChild(container);
+});
+
+shouldThrowErrorName(`container.insertBefore(fragment, refChild)`, 'HierarchyRequestError');
+shouldBe('container.parentNode', 'child.content');
+shouldBe('child.parentNode', 'null');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion-expected.txt b/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion-expected.txt
new file mode 100644 (file)
index 0000000..e1147b6
--- /dev/null
@@ -0,0 +1,13 @@
+This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.
+WebKit should detect this case and throw HierarchyRequestError.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.replaceChild(fragment, refChild) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS container.parentNode is child
+PASS child.parentNode is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion.html b/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion.html
new file mode 100644 (file)
index 0000000..d4a9328
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.<br>
+WebKit should detect this case and throw HierarchyRequestError.`);
+
+fragment = document.createDocumentFragment();
+container = document.createElement('div');
+refChild = container.appendChild(document.createElement('section'));
+child = fragment.appendChild(document.createElement('div'));
+fragment.addEventListener('DOMSubtreeModified', function() {
+    child.appendChild(container);
+});
+
+shouldThrowErrorName(`container.replaceChild(fragment, refChild)`, 'HierarchyRequestError');
+shouldBe('container.parentNode', 'child');
+shouldBe('child.parentNode', 'null');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt b/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt
new file mode 100644 (file)
index 0000000..15ec4c1
--- /dev/null
@@ -0,0 +1,13 @@
+This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.
+WebKit should detect this case and throw HierarchyRequestError.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.replaceChild(fragment, refChild) threw exception HierarchyRequestError: The operation would yield an incorrect node tree..
+PASS container.parentNode is childShadowRoot
+PASS child.parentNode is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion.html b/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion.html
new file mode 100644 (file)
index 0000000..4c024b4
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.<br>
+WebKit should detect this case and throw HierarchyRequestError.`);
+
+fragment = document.createDocumentFragment();
+container = document.createElement('div');
+refChild = container.appendChild(document.createElement('section'));
+child = fragment.appendChild(document.createElement('div'));
+childShadowRoot = child.attachShadow({mode: 'closed'});
+fragment.addEventListener('DOMSubtreeModified', function() {
+    childShadowRoot.appendChild(container);
+});
+
+shouldThrowErrorName(`container.replaceChild(fragment, refChild)`, 'HierarchyRequestError');
+shouldBe('container.parentNode', 'childShadowRoot');
+shouldBe('child.parentNode', 'null');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt b/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt
new file mode 100644 (file)
index 0000000..2d3b32d
--- /dev/null
@@ -0,0 +1,14 @@
+This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.
+WebKit should detect this case and throw HierarchyRequestError.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+FAIL container.replaceChild(fragment, refChild) should throw a HierarchyRequestError. Did not throw.
+PASS container.parentNode is child.content
+FAIL child.parentNode should be null. Was [object HTMLDivElement].
+PASS successfullyParsed is true
+Some tests failed.
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion.html b/LayoutTests/fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion.html
new file mode 100644 (file)
index 0000000..df2ae50
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests re-parenting a child of the document fragment during an insertion so as to create a circular node tree.<br>
+WebKit should detect this case and throw HierarchyRequestError.`);
+
+fragment = document.createDocumentFragment();
+container = document.createElement('div');
+refChild = container.appendChild(document.createElement('section'));
+child = fragment.appendChild(document.createElement('template'));
+fragment.addEventListener('DOMSubtreeModified', function() {
+    child.content.appendChild(container);
+});
+
+shouldThrowErrorName(`container.replaceChild(fragment, refChild)`, 'HierarchyRequestError');
+shouldBe('container.parentNode', 'child.content');
+shouldBe('child.parentNode', 'null');
+
+</script>
+</body>
+</html>
index c592c3b..a46408f 100644 (file)
@@ -1,3 +1,34 @@
+2019-03-19  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reparenting during a mutation event inside appendChild could result in a circular DOM tree
+        https://bugs.webkit.org/show_bug.cgi?id=192825
+
+        Reviewed by Zalan Bujtas.
+
+        The bug was caused by appendChildWithoutPreInsertionValidityCheck, insertBefore and replaceChild
+        checking the circular dependency against newChild instead of targets even though when newChild
+        is a document fragment, appendChildWithoutPreInsertionValidityCheck inserts the children of
+        the document fragment. Fixed the bug by checking the circular dependency against each target child.
+
+        Also fixed the bug that checkAcceptChildGuaranteedNodeTypes was not considering shadow inclusive
+        ancestors or template host elements.
+
+        Tests: fast/dom/append-child-with-mutation-event-removal-and-circular-insertion.html
+               fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion.html
+               fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion.html
+               fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion.html
+               fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion.html
+               fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion.html
+               fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion.html
+               fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion.html
+               fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion.html
+
+        * dom/ContainerNode.cpp:
+        (WebCore::checkAcceptChildGuaranteedNodeTypes):
+        (WebCore::ContainerNode::insertBefore):
+        (WebCore::ContainerNode::replaceChild):
+        (WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck):
+
 2019-03-19  Brent Fulgham  <bfulgham@apple.com>
 
         Add default prompt implementation for the Storage Access API
index ea9a9b8..6297b7c 100644 (file)
@@ -342,7 +342,7 @@ static inline ExceptionOr<void> checkAcceptChildGuaranteedNodeTypes(ContainerNod
 {
     ASSERT(!newParent.isDocumentTypeNode());
     ASSERT(isChildTypeAllowed(newParent, newChild));
-    if (newChild.contains(&newParent))
+    if (containsConsideringHostElements(newChild, newParent))
         return Exception { HierarchyRequestError };
     return { };
 }
@@ -388,9 +388,11 @@ ExceptionOr<void> ContainerNode::insertBefore(Node& newChild, Node* refChild)
         return { };
 
     // We need this extra check because collectChildrenAndRemoveFromOldParent() can fire mutation events.
-    auto checkAcceptResult = checkAcceptChildGuaranteedNodeTypes(*this, newChild);
-    if (checkAcceptResult.hasException())
-        return checkAcceptResult.releaseException();
+    for (auto& child : targets) {
+        auto checkAcceptResult = checkAcceptChildGuaranteedNodeTypes(*this, child);
+        if (checkAcceptResult.hasException())
+            return checkAcceptResult.releaseException();
+    }
 
     InspectorInstrumentation::willInsertDOMNode(document(), *this);
 
@@ -504,9 +506,11 @@ ExceptionOr<void> ContainerNode::replaceChild(Node& newChild, Node& oldChild)
     }
 
     // Do this one more time because collectChildrenAndRemoveFromOldParent() fires a MutationEvent.
-    validityResult = checkPreReplacementValidity(*this, newChild, oldChild);
-    if (validityResult.hasException())
-        return validityResult.releaseException();
+    for (auto& child : targets) {
+        validityResult = checkPreReplacementValidity(*this, child, oldChild);
+        if (validityResult.hasException())
+            return validityResult.releaseException();
+    }
 
     // Remove the node we're replacing.
     Ref<Node> protectOldChild(oldChild);
@@ -520,9 +524,11 @@ ExceptionOr<void> ContainerNode::replaceChild(Node& newChild, Node& oldChild)
             return removeResult.releaseException();
 
         // Does this one more time because removeChild() fires a MutationEvent.
-        validityResult = checkPreReplacementValidity(*this, newChild, oldChild);
-        if (validityResult.hasException())
-            return validityResult.releaseException();
+        for (auto& child : targets) {
+            validityResult = checkPreReplacementValidity(*this, child, oldChild);
+            if (validityResult.hasException())
+                return validityResult.releaseException();
+        }
     }
 
     InspectorInstrumentation::willInsertDOMNode(document(), *this);
@@ -699,9 +705,11 @@ ExceptionOr<void> ContainerNode::appendChildWithoutPreInsertionValidityCheck(Nod
         return { };
 
     // We need this extra check because collectChildrenAndRemoveFromOldParent() can fire mutation events.
-    auto nodeTypeResult = checkAcceptChildGuaranteedNodeTypes(*this, newChild);
-    if (nodeTypeResult.hasException())
-        return nodeTypeResult.releaseException();
+    for (auto& child : targets) {
+        auto nodeTypeResult = checkAcceptChildGuaranteedNodeTypes(*this, child);
+        if (nodeTypeResult.hasException())
+            return nodeTypeResult.releaseException();
+    }
 
     InspectorInstrumentation::willInsertDOMNode(document(), *this);