⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 243175 in webkit


Ignore:
Timestamp:
Mar 19, 2019, 3:25:44 PM (7 years ago)
Author:
rniwa@webkit.org
Message:

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.

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.
Location:
trunk
Files:
18 added
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r243169 r243175  
     12019-03-19  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reparenting during a mutation event inside appendChild could result in a circular DOM tree
     4        https://bugs.webkit.org/show_bug.cgi?id=192825
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        Added regression tests.
     9
     10        * fast/dom/append-child-with-mutation-event-removal-and-circular-insertion-expected.txt: Added.
     11        * fast/dom/append-child-with-mutation-event-removal-and-circular-insertion.html: Added.
     12        * fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt: Added.
     13        * fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion.html: Added.
     14        * fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt: Added.
     15        * fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion.html: Added.
     16        * fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion-expected.txt: Added.
     17        * fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion.html: Added.
     18        * fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt: Added.
     19        * fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion.html: Added.
     20        * fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt: Added.
     21        * fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion.html: Added.
     22        * fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion-expected.txt: Added.
     23        * fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion.html: Added.
     24        * fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion-expected.txt: Added.
     25        * fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion.html: Added.
     26        * fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion-expected.txt: Added.
     27        * fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion.html: Added.
     28
    1292019-03-19  Timothy Hatcher  <timothy@apple.com>
    230

  • trunk/Source/WebCore/ChangeLog

    r243173 r243175  
     12019-03-19  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reparenting during a mutation event inside appendChild could result in a circular DOM tree
     4        https://bugs.webkit.org/show_bug.cgi?id=192825
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        The bug was caused by appendChildWithoutPreInsertionValidityCheck, insertBefore and replaceChild
     9        checking the circular dependency against newChild instead of targets even though when newChild
     10        is a document fragment, appendChildWithoutPreInsertionValidityCheck inserts the children of
     11        the document fragment. Fixed the bug by checking the circular dependency against each target child.
     12
     13        Also fixed the bug that checkAcceptChildGuaranteedNodeTypes was not considering shadow inclusive
     14        ancestors or template host elements.
     15
     16        Tests: fast/dom/append-child-with-mutation-event-removal-and-circular-insertion.html
     17               fast/dom/append-child-with-mutation-event-removal-and-circular-shadow-insertion.html
     18               fast/dom/append-child-with-mutation-event-removal-and-circular-template-insertion.html
     19               fast/dom/insert-child-with-mutation-event-removal-and-circular-insertion.html
     20               fast/dom/insert-child-with-mutation-event-removal-and-circular-shadow-insertion.html
     21               fast/dom/insert-child-with-mutation-event-removal-and-circular-template-insertion.html
     22               fast/dom/replace-child-with-mutation-event-removal-and-circular-insertion.html
     23               fast/dom/replace-child-with-mutation-event-removal-and-circular-shadow-insertion.html
     24               fast/dom/replace-child-with-mutation-event-removal-and-circular-template-insertion.html
     25
     26        * dom/ContainerNode.cpp:
     27        (WebCore::checkAcceptChildGuaranteedNodeTypes):
     28        (WebCore::ContainerNode::insertBefore):
     29        (WebCore::ContainerNode::replaceChild):
     30        (WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck):
     31
    1322019-03-19  Brent Fulgham  <bfulgham@apple.com>
    233

  • trunk/Source/WebCore/dom/ContainerNode.cpp

    r241932 r243175  
    343343    ASSERT(!newParent.isDocumentTypeNode());
    344344    ASSERT(isChildTypeAllowed(newParent, newChild));
    345     if (newChild.contains(&newParent))
     345    if (containsConsideringHostElements(newChild, newParent))
    346346        return Exception { HierarchyRequestError };
    347347    return { };
    … …  
    389389
    390390    // We need this extra check because collectChildrenAndRemoveFromOldParent() can fire mutation events.
    391     auto checkAcceptResult = checkAcceptChildGuaranteedNodeTypes(*this, newChild);
    392     if (checkAcceptResult.hasException())
    393         return checkAcceptResult.releaseException();
     391    for (auto& child : targets) {
     392        auto checkAcceptResult = checkAcceptChildGuaranteedNodeTypes(*this, child);
     393        if (checkAcceptResult.hasException())
     394            return checkAcceptResult.releaseException();
     395    }
    394396
    395397    InspectorInstrumentation::willInsertDOMNode(document(), *this);
    … …  
    505507
    506508    // Do this one more time because collectChildrenAndRemoveFromOldParent() fires a MutationEvent.
    507     validityResult = checkPreReplacementValidity(*this, newChild, oldChild);
    508     if (validityResult.hasException())
    509         return validityResult.releaseException();
     509    for (auto& child : targets) {
     510        validityResult = checkPreReplacementValidity(*this, child, oldChild);
     511        if (validityResult.hasException())
     512            return validityResult.releaseException();
     513    }
    510514
    511515    // Remove the node we're replacing.
    … …  
    521525
    522526        // Does this one more time because removeChild() fires a MutationEvent.
    523         validityResult = checkPreReplacementValidity(*this, newChild, oldChild);
    524         if (validityResult.hasException())
    525             return validityResult.releaseException();
     527        for (auto& child : targets) {
     528            validityResult = checkPreReplacementValidity(*this, child, oldChild);
     529            if (validityResult.hasException())
     530                return validityResult.releaseException();
     531        }
    526532    }
    527533
    … …  
    700706
    701707    // We need this extra check because collectChildrenAndRemoveFromOldParent() can fire mutation events.
    702     auto nodeTypeResult = checkAcceptChildGuaranteedNodeTypes(*this, newChild);
    703     if (nodeTypeResult.hasException())
    704         return nodeTypeResult.releaseException();
     708    for (auto& child : targets) {
     709        auto nodeTypeResult = checkAcceptChildGuaranteedNodeTypes(*this, child);
     710        if (nodeTypeResult.hasException())
     711            return nodeTypeResult.releaseException();
     712    }
    705713
    706714    InspectorInstrumentation::willInsertDOMNode(document(), *this);
Note: See TracChangeset for help on using the changeset viewer.