<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[212025] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/212025">212025</a></dd>
<dt>Author</dt> <dd>bfulgham@apple.com</dd>
<dt>Date</dt> <dd>2017-02-09 18:13:07 -0800 (Thu, 09 Feb 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>Crash under HTMLFormElement::registerFormElement()
https://bugs.webkit.org/show_bug.cgi?id=167162
Patch by Chris Dumez <cdumez@apple.com> on 2017-02-09
Reviewed by Ryosuke Niwa.
Source/WebCore:
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].
LayoutTests:
Add layout test coverage.
* fast/forms/registerFormElement-crash-expected.txt: Added.
* fast/forms/registerFormElement-crash.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorehtmlFormAssociatedElementcpp">trunk/Source/WebCore/html/FormAssociatedElement.cpp</a></li>
<li><a href="#trunkSourceWebCorehtmlHTMLFormElementcpp">trunk/Source/WebCore/html/HTMLFormElement.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastformsregisterFormElementcrashexpectedtxt">trunk/LayoutTests/fast/forms/registerFormElement-crash-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastformsregisterFormElementcrashhtml">trunk/LayoutTests/fast/forms/registerFormElement-crash.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (212024 => 212025)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2017-02-10 02:11:28 UTC (rev 212024)
+++ trunk/LayoutTests/ChangeLog 2017-02-10 02:13:07 UTC (rev 212025)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2017-02-09 Chris Dumez <cdumez@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.
+
</ins><span class="cx"> 2017-02-09 Antti Koivisto <antti@apple.com>
</span><span class="cx">
</span><span class="cx"> Tear down existing renderers when adding a shadow root.
</span></span></pre></div>
<a id="trunkLayoutTestsfastformsregisterFormElementcrashexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/forms/registerFormElement-crash-expected.txt (0 => 212025)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/forms/registerFormElement-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/registerFormElement-crash-expected.txt 2017-02-10 02:13:07 UTC (rev 212025)
</span><span class="lines">@@ -0,0 +1,2 @@
</span><ins>+CONSOLE MESSAGE: line 17: PASS: did not crash
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastformsregisterFormElementcrashhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/forms/registerFormElement-crash.html (0 => 212025)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/forms/registerFormElement-crash.html (rev 0)
+++ trunk/LayoutTests/fast/forms/registerFormElement-crash.html 2017-02-10 02:13:07 UTC (rev 212025)
</span><span class="lines">@@ -0,0 +1,33 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (212024 => 212025)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-02-10 02:11:28 UTC (rev 212024)
+++ trunk/Source/WebCore/ChangeLog 2017-02-10 02:13:07 UTC (rev 212025)
</span><span class="lines">@@ -1,3 +1,53 @@
</span><ins>+2017-02-09 Chris Dumez <cdumez@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].
+
</ins><span class="cx"> 2017-02-09 Antti Koivisto <antti@apple.com>
</span><span class="cx">
</span><span class="cx"> Tear down existing renderers when adding a shadow root.
</span></span></pre></div>
<a id="trunkSourceWebCorehtmlFormAssociatedElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/html/FormAssociatedElement.cpp (212024 => 212025)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/html/FormAssociatedElement.cpp 2017-02-10 02:11:28 UTC (rev 212024)
+++ trunk/Source/WebCore/html/FormAssociatedElement.cpp 2017-02-10 02:13:07 UTC (rev 212025)
</span><span class="lines">@@ -63,7 +63,7 @@
</span><span class="cx"> void FormAssociatedElement::didMoveToNewDocument(Document&)
</span><span class="cx"> {
</span><span class="cx"> HTMLElement& element = asHTMLElement();
</span><del>- if (element.hasAttributeWithoutSynchronization(formAttr))
</del><ins>+ if (element.hasAttributeWithoutSynchronization(formAttr) && element.inDocument())
</ins><span class="cx"> resetFormAttributeTargetObserver();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -266,6 +266,7 @@
</span><span class="cx">
</span><span class="cx"> void FormAssociatedElement::resetFormAttributeTargetObserver()
</span><span class="cx"> {
</span><ins>+ ASSERT_WITH_SECURITY_IMPLICATION(asHTMLElement().inDocument());
</ins><span class="cx"> m_formAttributeTargetObserver = std::make_unique<FormAttributeTargetObserver>(asHTMLElement().attributeWithoutSynchronization(formAttr), *this);
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorehtmlHTMLFormElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/html/HTMLFormElement.cpp (212024 => 212025)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/html/HTMLFormElement.cpp 2017-02-10 02:11:28 UTC (rev 212024)
+++ trunk/Source/WebCore/html/HTMLFormElement.cpp 2017-02-10 02:13:07 UTC (rev 212025)
</span><span class="lines">@@ -547,8 +547,9 @@
</span><span class="cx">
</span><span class="cx"> // Treats separately the case where this element has the form attribute
</span><span class="cx"> // for performance consideration.
</span><del>- if (associatedHTMLElement.hasAttributeWithoutSynchronization(formAttr)) {
</del><ins>+ if (associatedHTMLElement.hasAttributeWithoutSynchronization(formAttr) && associatedHTMLElement.inDocument()) {
</ins><span class="cx"> unsigned short position = compareDocumentPosition(associatedHTMLElement);
</span><ins>+ ASSERT_WITH_SECURITY_IMPLICATION(!(position & DOCUMENT_POSITION_DISCONNECTED));
</ins><span class="cx"> if (position & DOCUMENT_POSITION_PRECEDING) {
</span><span class="cx"> ++m_associatedElementsBeforeIndex;
</span><span class="cx"> ++m_associatedElementsAfterIndex;
</span></span></pre>
</div>
</div>
</body>
</html>