Title: [212093] branches/safari-603-branch

Diff

Modified: branches/safari-603-branch/LayoutTests/ChangeLog (212092 => 212093)


--- branches/safari-603-branch/LayoutTests/ChangeLog	2017-02-10 08:16:12 UTC (rev 212092)
+++ branches/safari-603-branch/LayoutTests/ChangeLog	2017-02-10 08:16:16 UTC (rev 212093)
@@ -1,5 +1,21 @@
 2017-02-09  Matthew Hanson  <matthew_han...@apple.com>
 
+        Merge r212025. rdar://problem/30076615
+
+    2017-02-09  Chris Dumez  <cdu...@apple.com>
+
+            Crash under HTMLFormElement::registerFormElement()
+            https://bugs.webkit.org/show_bug.cgi?id=167162
+
+            Reviewed by Ryosuke Niwa.
+
+            Add layout test coverage.
+
+            * fast/forms/registerFormElement-crash-expected.txt: Added.
+            * fast/forms/registerFormElement-crash.html: Added.
+
+2017-02-09  Matthew Hanson  <matthew_han...@apple.com>
+
         Merge r212024. rdar://problem/30051227
 
     2017-02-09  Antti Koivisto  <an...@apple.com>

Added: branches/safari-603-branch/LayoutTests/fast/forms/registerFormElement-crash-expected.txt (0 => 212093)


--- branches/safari-603-branch/LayoutTests/fast/forms/registerFormElement-crash-expected.txt	                        (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/forms/registerFormElement-crash-expected.txt	2017-02-10 08:16:16 UTC (rev 212093)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 17: PASS: did not crash
+

Added: branches/safari-603-branch/LayoutTests/fast/forms/registerFormElement-crash.html (0 => 212093)


--- branches/safari-603-branch/LayoutTests/fast/forms/registerFormElement-crash.html	                        (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/forms/registerFormElement-crash.html	2017-02-10 08:16:16 UTC (rev 212093)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function runTest() {
+    var iframe = document.getElementById("iframe");
+    var iframeWindow = window[0];
+    var toInsert = div;
+    var iframeBody = iframeWindow.document.body;
+    iframeBody.before(document.body);
+    iframe.after(toInsert);
+    console.log("PASS: did not crash");
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+</script>
+</head>
+<body _onload_="runTest()">
+    <p>This test passes if it does not crash.</p>
+    <form id="form">
+        <textarea form="form">aaaaaaaa</textarea>
+        <iframe id="iframe"></iframe>
+    </form>
+    <div id="div">
+        <dir><object id=“wtf"></object></dir>
+    <div>
+</body>
+</html>

Modified: branches/safari-603-branch/Source/WebCore/ChangeLog (212092 => 212093)


--- branches/safari-603-branch/Source/WebCore/ChangeLog	2017-02-10 08:16:12 UTC (rev 212092)
+++ branches/safari-603-branch/Source/WebCore/ChangeLog	2017-02-10 08:16:16 UTC (rev 212093)
@@ -1,5 +1,59 @@
 2017-02-09  Matthew Hanson  <matthew_han...@apple.com>
 
+        Merge r212025. rdar://problem/30076615
+
+    2017-02-09  Chris Dumez  <cdu...@apple.com>
+
+            Crash under HTMLFormElement::registerFormElement()
+            https://bugs.webkit.org/show_bug.cgi?id=167162
+
+            Reviewed by Ryosuke Niwa.
+
+            didMoveToNewDocument() was re-registering FormAttributeTargetObserver
+            even if the element's inDocument was not set yet. As a result, it was
+            possible for FormAssociatedElement::resetFormOwner() to be called
+            when the element was in the tree but with its inDocument still being
+            false (because insertedInto() has not been called yet). This could
+            end up calling HTMLFormElement::registerFormElement() even though
+            the element is still recognized as detached. This is an issue because
+            HTMLFormElement::m_associatedElements's order and its corresponding
+            indexes (m_associatedElementsBeforeIndex / m_associatedElementsAfterIndex)
+            rely on the position of the element with regards to the form element
+            (before / inside / after).
+
+            To address the issue, we now only register the FormAttributeTargetObserver
+            in didMoveToNewDocument() if the inDocument flag is set to true. This
+            is similar to what is done at other call sites of
+            resetFormAttributeTargetObserver(). We also ignore the form content
+            attribute in HTMLFormElement::formElementIndex() if the element is
+            not connected.
+
+            As per the HTML specification [1], the form content attribute is only
+            taken if the element is connected (i.e. inDocument flag is true).
+
+            Note that FormAssociatedElement::findAssociatedForm() was already
+            ignoring the form content attribute if the element is disconnected.
+
+            [1] https://html.spec.whatwg.org/#reset-the-form-owner (step 3)
+
+            Test: fast/forms/registerFormElement-crash.html
+
+            * html/FormAssociatedElement.cpp:
+            (WebCore::FormAssociatedElement::didMoveToNewDocument):
+            Only call resetFormAttributeTargetObserver() if inDocument flag is set,
+            similarly to what is done at other call sites.
+
+            (WebCore::FormAssociatedElement::resetFormAttributeTargetObserver):
+            Add an assertion to make sure no one call this method on an element that
+            is not connected.
+
+            * html/HTMLFormElement.cpp:
+            (WebCore::HTMLFormElement::formElementIndex):
+            Ignore the form content attribute if the element is not connected, as
+            per the HTML specification [1].
+
+2017-02-09  Matthew Hanson  <matthew_han...@apple.com>
+
         Merge r212024. rdar://problem/30051227
 
     2017-02-09  Antti Koivisto  <an...@apple.com>

Modified: branches/safari-603-branch/Source/WebCore/html/FormAssociatedElement.cpp (212092 => 212093)


--- branches/safari-603-branch/Source/WebCore/html/FormAssociatedElement.cpp	2017-02-10 08:16:12 UTC (rev 212092)
+++ branches/safari-603-branch/Source/WebCore/html/FormAssociatedElement.cpp	2017-02-10 08:16:16 UTC (rev 212093)
@@ -63,7 +63,7 @@
 void FormAssociatedElement::didMoveToNewDocument(Document&)
 {
     HTMLElement& element = asHTMLElement();
-    if (element.hasAttributeWithoutSynchronization(formAttr))
+    if (element.hasAttributeWithoutSynchronization(formAttr) && element.inDocument())
         resetFormAttributeTargetObserver();
 }
 
@@ -266,6 +266,7 @@
 
 void FormAssociatedElement::resetFormAttributeTargetObserver()
 {
+    ASSERT_WITH_SECURITY_IMPLICATION(asHTMLElement().inDocument());
     m_formAttributeTargetObserver = std::make_unique<FormAttributeTargetObserver>(asHTMLElement().attributeWithoutSynchronization(formAttr), *this);
 }
 

Modified: branches/safari-603-branch/Source/WebCore/html/HTMLFormElement.cpp (212092 => 212093)


--- branches/safari-603-branch/Source/WebCore/html/HTMLFormElement.cpp	2017-02-10 08:16:12 UTC (rev 212092)
+++ branches/safari-603-branch/Source/WebCore/html/HTMLFormElement.cpp	2017-02-10 08:16:16 UTC (rev 212093)
@@ -528,8 +528,9 @@
 
     // Treats separately the case where this element has the form attribute
     // for performance consideration.
-    if (associatedHTMLElement.hasAttributeWithoutSynchronization(formAttr)) {
+    if (associatedHTMLElement.hasAttributeWithoutSynchronization(formAttr) && associatedHTMLElement.inDocument()) {
         unsigned short position = compareDocumentPosition(associatedHTMLElement);
+        ASSERT_WITH_SECURITY_IMPLICATION(!(position & DOCUMENT_POSITION_DISCONNECTED));
         if (position & DOCUMENT_POSITION_PRECEDING) {
             ++m_associatedElementsBeforeIndex;
             ++m_associatedElementsAfterIndex;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to