ASSERT_WITH_SECURITY_IMPLICATION in WebCore::DocumentOrderedMap::get(); update form
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 Sep 2015 22:46:43 +0000 (22:46 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 Sep 2015 22:46:43 +0000 (22:46 +0000)
association after subtree insertion
https://bugs.webkit.org/show_bug.cgi?id=148919
<rdar://problem/21868036>

Patch by Daniel Bates <dabates@apple.com> on 2015-09-07
Reviewed by Andy Estes.

Source/WebCore:

Currently we update the form association of a form control upon insertion into
the document. Instead we should update the form association of a form control
after its containing subtree is inserted into the document to avoid an assertion
failure when the containing subtree has an element whose id is identical to both
the id of some other element in the document and the name of the form referenced
by the inserted form control.

Tests: fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2.html
       fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3.html
       fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4.html
       fast/forms/update-form-owner-in-moved-subtree-assertion-failure.html

* html/FormAssociatedElement.cpp:
(WebCore::FormAssociatedElement::insertedInto): Moved resetFormOwner() from here
to {HTMLFormControlElement, HTMLObjectElement}::finishedInsertingSubtree().
* html/HTMLFormControlElement.cpp:
(WebCore::HTMLFormControlElement::insertedInto): Return InsertionShouldCallFinishedInsertingSubtree
so that HTMLFormControlElement::finishedInsertingSubtree() is called.
(WebCore::HTMLFormControlElement::finishedInsertingSubtree): Added; turn around and
call FormAssociatedElement::resetFormOwner().
* html/HTMLFormControlElement.h:
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::insertedInto): Return InsertionShouldCallFinishedInsertingSubtree so
that HTMLInputElement::finishedInsertingSubtree() is called and move logic to update radio button
group from here...
(WebCore::HTMLInputElement::finishedInsertingSubtree): to here.
* html/HTMLInputElement.h:
* html/HTMLObjectElement.cpp:
(WebCore::HTMLObjectElement::insertedInto): Return InsertionShouldCallFinishedInsertingSubtree so
that HTMLObjectElement::finishedInsertingSubtree() is called.
(WebCore::HTMLObjectElement::finishedInsertingSubtree): Added; turn around and
call FormAssociatedElement::resetFormOwner().
* html/HTMLObjectElement.h:
* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::insertedInto): Modified to return the result of
HTMLFormControlElementWithState::insertedInto(), which may schedule a callback after subtree
insertion.
* html/HTMLTextFormControlElement.cpp:
(WebCore::HTMLTextFormControlElement::insertedInto): Ditto.

LayoutTests:

Add tests to ensure that updating the form association of a form control in a subtree
does not cause an assertion failure.

* fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2-expected.txt: Added.
* fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2.html: Added.
* fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3-expected.txt: Added.
* fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3.html: Added.
* fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4-expected.txt: Added.
* fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4.html: Added.
* fast/forms/update-form-owner-in-moved-subtree-assertion-failure-expected.txt: Added.
* fast/forms/update-form-owner-in-moved-subtree-assertion-failure.html: Added.

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

19 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2.html [new file with mode: 0644]
LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3.html [new file with mode: 0644]
LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4.html [new file with mode: 0644]
LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/FormAssociatedElement.cpp
Source/WebCore/html/HTMLFormControlElement.cpp
Source/WebCore/html/HTMLFormControlElement.h
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/HTMLInputElement.h
Source/WebCore/html/HTMLObjectElement.cpp
Source/WebCore/html/HTMLObjectElement.h
Source/WebCore/html/HTMLSelectElement.cpp
Source/WebCore/html/HTMLTextFormControlElement.cpp

index 10e2706..098fbd2 100644 (file)
@@ -1,3 +1,24 @@
+2015-09-07  Daniel Bates  <dabates@apple.com>
+
+        ASSERT_WITH_SECURITY_IMPLICATION in WebCore::DocumentOrderedMap::get(); update form
+        association after subtree insertion
+        https://bugs.webkit.org/show_bug.cgi?id=148919
+        <rdar://problem/21868036>
+
+        Reviewed by Andy Estes.
+
+        Add tests to ensure that updating the form association of a form control in a subtree
+        does not cause an assertion failure.
+
+        * fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2-expected.txt: Added.
+        * fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2.html: Added.
+        * fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3-expected.txt: Added.
+        * fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3.html: Added.
+        * fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4-expected.txt: Added.
+        * fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4.html: Added.
+        * fast/forms/update-form-owner-in-moved-subtree-assertion-failure-expected.txt: Added.
+        * fast/forms/update-form-owner-in-moved-subtree-assertion-failure.html: Added.
+
 2015-09-07  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 
         [GTK] Unreviewed GTK gardening.
diff --git a/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2-expected.txt b/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2-expected.txt
new file mode 100644 (file)
index 0000000..f1ce779
--- /dev/null
@@ -0,0 +1,2 @@
+
+PASS, this test did not cause an assertion failure.
diff --git a/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2.html b/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2.html
new file mode 100644 (file)
index 0000000..b3a0920
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<div id="container"></div>
+<div id="subtreeToMove">
+    <object form="A"></object>
+    <div id="A"></div>
+</div>
+<div id="A"></div>
+<p>PASS, this test did not cause an assertion failure.</p>
+<script>
+var container = document.getElementById("container");
+var subtreeToMove = document.getElementById("subtreeToMove");
+container.appendChild(subtreeToMove);
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3-expected.txt b/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3-expected.txt
new file mode 100644 (file)
index 0000000..f1ce779
--- /dev/null
@@ -0,0 +1,2 @@
+
+PASS, this test did not cause an assertion failure.
diff --git a/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3.html b/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3.html
new file mode 100644 (file)
index 0000000..a337430
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<div id="container"></div>
+<div id="subtreeToMove">
+    <select form="A"></select>
+    <div id="A"></div>
+</div>
+<div id="A"></div>
+<p>PASS, this test did not cause an assertion failure.</p>
+<script>
+var container = document.getElementById("container");
+var subtreeToMove = document.getElementById("subtreeToMove");
+container.appendChild(subtreeToMove);
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4-expected.txt b/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4-expected.txt
new file mode 100644 (file)
index 0000000..f1ce779
--- /dev/null
@@ -0,0 +1,2 @@
+
+PASS, this test did not cause an assertion failure.
diff --git a/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4.html b/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4.html
new file mode 100644 (file)
index 0000000..f21de2d
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<div id="container"></div>
+<div id="subtreeToMove">
+    <input type="text" form="A">
+    <div id="A"></div>
+</div>
+<div id="A"></div>
+<p>PASS, this test did not cause an assertion failure.</p>
+<script>
+var container = document.getElementById("container");
+var subtreeToMove = document.getElementById("subtreeToMove");
+container.appendChild(subtreeToMove);
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-expected.txt b/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure-expected.txt
new file mode 100644 (file)
index 0000000..e403e56
--- /dev/null
@@ -0,0 +1,2 @@
+  
+PASS, this test did not cause an assertion failure.
diff --git a/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure.html b/LayoutTests/fast/forms/update-form-owner-in-moved-subtree-assertion-failure.html
new file mode 100644 (file)
index 0000000..de903c8
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<div id="container"></div>
+<div id="subtreeToMove">
+    <keygen form="A">
+    <select id="A"></select>
+</div>
+<div id="A"></div>
+<p>PASS, this test did not cause an assertion failure.</p>
+<script>
+var container = document.getElementById("container");
+var subtreeToMove = document.getElementById("subtreeToMove");
+container.appendChild(subtreeToMove);
+</script>
+</body>
+</html>
index 54e1941..4adfbf6 100644 (file)
@@ -1,3 +1,52 @@
+2015-09-07  Daniel Bates  <dabates@apple.com>
+
+        ASSERT_WITH_SECURITY_IMPLICATION in WebCore::DocumentOrderedMap::get(); update form
+        association after subtree insertion
+        https://bugs.webkit.org/show_bug.cgi?id=148919
+        <rdar://problem/21868036>
+
+        Reviewed by Andy Estes.
+
+        Currently we update the form association of a form control upon insertion into
+        the document. Instead we should update the form association of a form control
+        after its containing subtree is inserted into the document to avoid an assertion
+        failure when the containing subtree has an element whose id is identical to both
+        the id of some other element in the document and the name of the form referenced
+        by the inserted form control.
+
+        Tests: fast/forms/update-form-owner-in-moved-subtree-assertion-failure-2.html
+               fast/forms/update-form-owner-in-moved-subtree-assertion-failure-3.html
+               fast/forms/update-form-owner-in-moved-subtree-assertion-failure-4.html
+               fast/forms/update-form-owner-in-moved-subtree-assertion-failure.html
+
+        * html/FormAssociatedElement.cpp:
+        (WebCore::FormAssociatedElement::insertedInto): Moved resetFormOwner() from here
+        to {HTMLFormControlElement, HTMLObjectElement}::finishedInsertingSubtree().
+        * html/HTMLFormControlElement.cpp:
+        (WebCore::HTMLFormControlElement::insertedInto): Return InsertionShouldCallFinishedInsertingSubtree
+        so that HTMLFormControlElement::finishedInsertingSubtree() is called.
+        (WebCore::HTMLFormControlElement::finishedInsertingSubtree): Added; turn around and
+        call FormAssociatedElement::resetFormOwner().
+        * html/HTMLFormControlElement.h:
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::insertedInto): Return InsertionShouldCallFinishedInsertingSubtree so
+        that HTMLInputElement::finishedInsertingSubtree() is called and move logic to update radio button
+        group from here...
+        (WebCore::HTMLInputElement::finishedInsertingSubtree): to here.
+        * html/HTMLInputElement.h:
+        * html/HTMLObjectElement.cpp:
+        (WebCore::HTMLObjectElement::insertedInto): Return InsertionShouldCallFinishedInsertingSubtree so
+        that HTMLObjectElement::finishedInsertingSubtree() is called.
+        (WebCore::HTMLObjectElement::finishedInsertingSubtree): Added; turn around and
+        call FormAssociatedElement::resetFormOwner().
+        * html/HTMLObjectElement.h:
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::insertedInto): Modified to return the result of
+        HTMLFormControlElementWithState::insertedInto(), which may schedule a callback after subtree
+        insertion.
+        * html/HTMLTextFormControlElement.cpp:
+        (WebCore::HTMLTextFormControlElement::insertedInto): Ditto.
+
 2015-09-07  Antti Koivisto  <antti@apple.com>
 
         Remove GlyphPage::mayUseMixedFontsWhenFilling
index f927c6e..474fb65 100644 (file)
@@ -68,7 +68,6 @@ void FormAssociatedElement::didMoveToNewDocument(Document* oldDocument)
 
 void FormAssociatedElement::insertedInto(ContainerNode& insertionPoint)
 {
-    resetFormOwner();
     if (!insertionPoint.inDocument())
         return;
 
index 94d0950..6217fbd 100644 (file)
@@ -260,8 +260,12 @@ Node::InsertionNotificationRequest HTMLFormControlElement::insertedInto(Containe
     setNeedsWillValidateCheck();
     HTMLElement::insertedInto(insertionPoint);
     FormAssociatedElement::insertedInto(insertionPoint);
+    return InsertionShouldCallFinishedInsertingSubtree;
+}
 
-    return InsertionDone;
+void HTMLFormControlElement::finishedInsertingSubtree()
+{
+    resetFormOwner();
 }
 
 void HTMLFormControlElement::removedFrom(ContainerNode& insertionPoint)
index 8669bdc..3e6054e 100644 (file)
@@ -129,6 +129,7 @@ protected:
     virtual void requiredAttributeChanged();
     virtual void didAttachRenderers() override;
     virtual InsertionNotificationRequest insertedInto(ContainerNode&) override;
+    void finishedInsertingSubtree() override;
     virtual void removedFrom(ContainerNode&) override;
     virtual void didMoveToNewDocument(Document* oldDocument) override;
 
index 9a4ddec..3642a9a 100644 (file)
@@ -1473,12 +1473,17 @@ void HTMLInputElement::didChangeForm()
 Node::InsertionNotificationRequest HTMLInputElement::insertedInto(ContainerNode& insertionPoint)
 {
     HTMLTextFormControlElement::insertedInto(insertionPoint);
-    if (insertionPoint.inDocument() && !form())
-        addToRadioButtonGroup();
 #if ENABLE(DATALIST_ELEMENT)
     resetListAttributeTargetObserver();
 #endif
-    return InsertionDone;
+    return InsertionShouldCallFinishedInsertingSubtree;
+}
+
+void HTMLInputElement::finishedInsertingSubtree()
+{
+    HTMLTextFormControlElement::finishedInsertingSubtree();
+    if (inDocument() && !form())
+        addToRadioButtonGroup();
 }
 
 void HTMLInputElement::removedFrom(ContainerNode& insertionPoint)
index 9b27504..b924307 100644 (file)
@@ -335,6 +335,7 @@ private:
     virtual void willChangeForm() override final;
     virtual void didChangeForm() override final;
     virtual InsertionNotificationRequest insertedInto(ContainerNode&) override final;
+    void finishedInsertingSubtree() override final;
     virtual void removedFrom(ContainerNode&) override final;
     virtual void didMoveToNewDocument(Document* oldDocument) override final;
 
index 52384b1..f9d1986 100644 (file)
@@ -337,7 +337,12 @@ Node::InsertionNotificationRequest HTMLObjectElement::insertedInto(ContainerNode
 {
     HTMLPlugInImageElement::insertedInto(insertionPoint);
     FormAssociatedElement::insertedInto(insertionPoint);
-    return InsertionDone;
+    return InsertionShouldCallFinishedInsertingSubtree;
+}
+
+void HTMLObjectElement::finishedInsertingSubtree()
+{
+    resetFormOwner();
 }
 
 void HTMLObjectElement::removedFrom(ContainerNode& insertionPoint)
index 840ecb1..e6e7421 100644 (file)
@@ -63,6 +63,7 @@ private:
     virtual void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStyleProperties&) override;
 
     virtual InsertionNotificationRequest insertedInto(ContainerNode&) override;
+    void finishedInsertingSubtree() override final;
     virtual void removedFrom(ContainerNode&) override;
 
     virtual void didMoveToNewDocument(Document* oldDocument) override;
index d9e37a3..eb73945 100644 (file)
@@ -1592,8 +1592,7 @@ Node::InsertionNotificationRequest HTMLSelectElement::insertedInto(ContainerNode
     // items yet - but for innerHTML and related methods, this method is called
     // after the whole subtree is constructed.
     recalcListItems();
-    HTMLFormControlElementWithState::insertedInto(insertionPoint);
-    return InsertionDone;
+    return HTMLFormControlElementWithState::insertedInto(insertionPoint);
 }
 
 void HTMLSelectElement::accessKeySetSelectedIndex(int index)
index 534a0e1..d665ca3 100644 (file)
@@ -78,12 +78,12 @@ bool HTMLTextFormControlElement::childShouldCreateRenderer(const Node& child) co
 
 Node::InsertionNotificationRequest HTMLTextFormControlElement::insertedInto(ContainerNode& insertionPoint)
 {
-    HTMLFormControlElementWithState::insertedInto(insertionPoint);
+    InsertionNotificationRequest insertionNotificationRequest = HTMLFormControlElementWithState::insertedInto(insertionPoint);
     if (!insertionPoint.inDocument())
-        return InsertionDone;
+        return insertionNotificationRequest;
     String initialValue = value();
     setTextAsOfLastFormControlChangeEvent(initialValue.isNull() ? emptyString() : initialValue);
-    return InsertionDone;
+    return insertionNotificationRequest;
 }
 
 void HTMLTextFormControlElement::dispatchFocusEvent(RefPtr<Element>&& oldFocusedElement, FocusDirection direction)