git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 2dccb0e)
REGRESSION (r179497): Crash inside setAttributeNode
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Feb 2017 02:01:26 +0000 (02:01 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Feb 2017 02:01:26 +0000 (02:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=168161
<rdar://problem/30451581>

Reviewed by Andreas Kling.

Source/WebCore:

The bug was caused by setAttributeNode calling setAttributeInternal with the same element data as the one used
to call removeAttributeInternal despite of the fact removeAttributeInternal could have invoked arbitrary scripts
and mutated element's m_elementData.

Fixed the bug by calling with setAttributeInternal with the result of new invocation of ensureUniqueElementData().

Test: fast/dom/Attr/make-unique-element-data-while-replacing-attr.html

* dom/Element.cpp:
(WebCore::Element::setAttributeNode):

LayoutTests:

Added a regression test.

* fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt: Added.
* fast/dom/Attr/make-unique-element-data-while-replacing-attr.html: Added.

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

LayoutTests/ChangeLog patch | blob | history
LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt [new file with mode: 0644] patch | blob
LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr.html [new file with mode: 0644] patch | blob
Source/WebCore/ChangeLog patch | blob | history
Source/WebCore/dom/Element.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 27d294f..ee95bdf 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2017-02-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION (r179497): Crash inside setAttributeNode
+        https://bugs.webkit.org/show_bug.cgi?id=168161
+        <rdar://problem/30451581>
+
+        Reviewed by Andreas Kling.
+
+        Added a regression test.
+
+        * fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt: Added.
+        * fast/dom/Attr/make-unique-element-data-while-replacing-attr.html: Added.
+
 2017-02-12  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Handle caps lock indicator in event modifiers
 2017-02-12  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Handle caps lock indicator in event modifiers
diff --git a/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt b/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt
new file mode 100644 (file)
index 0000000..3618123
--- /dev/null
+++ b/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt
@@ -0,0 +1,10 @@
+Test making the element data unique while replacing an Attr node.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS element.getAttribute("width") is "b"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr.html b/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr.html
new file mode 100644 (file)
index 0000000..868109b
--- /dev/null
+++ b/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../../resources/js-test.js"></script>
+<script>
+
+description('Test making the element data unique while replacing an Attr node.');
+
+let element = document.createElement('div');
+element.setAttribute('foo', 'bar');
+
+let oldAttr = document.createAttributeNS('http://www.w3.org/XML/1998/namespace', 'width');
+oldAttr.value = 'a';
+element.setAttributeNode(oldAttr);
+
+element.addEventListener('DOMSubtreeModified', () => { element.cloneNode(); }, true);
+
+let newAttr = document.createAttributeNS('http://www.w3.org/1999/xhtml','width');
+newAttr.value = 'b';
+element.setAttributeNode(newAttr);
+
+shouldBeEqualToString('element.getAttribute("width")', 'b');
+
+</script>              
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 5d3b358..5cba6ee 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,24 @@
 2017-02-12  Ryosuke Niwa  <rniwa@webkit.org>
 
 2017-02-12  Ryosuke Niwa  <rniwa@webkit.org>
 
+        REGRESSION (r179497): Crash inside setAttributeNode
+        https://bugs.webkit.org/show_bug.cgi?id=168161
+        <rdar://problem/30451581>
+
+        Reviewed by Andreas Kling.
+
+        The bug was caused by setAttributeNode calling setAttributeInternal with the same element data as the one used
+        to call removeAttributeInternal despite of the fact removeAttributeInternal could have invoked arbitrary scripts
+        and mutated element's m_elementData.
+
+        Fixed the bug by calling with setAttributeInternal with the result of new invocation of ensureUniqueElementData().
+
+        Test: fast/dom/Attr/make-unique-element-data-while-replacing-attr.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::setAttributeNode):
+
+2017-02-12  Ryosuke Niwa  <rniwa@webkit.org>
+
         Rebaseline bindings tests after r212207.
 
         * bindings/scripts/test/JS/JSTestObj.cpp:
         Rebaseline bindings tests after r212207.
 
         * bindings/scripts/test/JS/JSTestObj.cpp:
diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
index 9c3a726..0db2a1e 100644 (file)
--- a/Source/WebCore/dom/Element.cpp
+++ b/Source/WebCore/dom/Element.cpp
@@ -2149,7 +2149,7 @@ ExceptionOr<RefPtr<Attr>> Element::setAttributeNode(Attr& attrNode)
             setAttributeInternal(existingAttributeIndex, attrNode.qualifiedName(), attrNode.value(), NotInSynchronizationOfLazyAttribute);
         else {
             removeAttributeInternal(existingAttributeIndex, NotInSynchronizationOfLazyAttribute);
             setAttributeInternal(existingAttributeIndex, attrNode.qualifiedName(), attrNode.value(), NotInSynchronizationOfLazyAttribute);
         else {
             removeAttributeInternal(existingAttributeIndex, NotInSynchronizationOfLazyAttribute);
-            setAttributeInternal(elementData.findAttributeIndexByName(attrNode.qualifiedName()), attrNode.qualifiedName(), attrNode.value(), NotInSynchronizationOfLazyAttribute);
+            setAttributeInternal(ensureUniqueElementData().findAttributeIndexByName(attrNode.qualifiedName()), attrNode.qualifiedName(), attrNode.value(), NotInSynchronizationOfLazyAttribute);
         }
     }
     if (attrNode.ownerElement() != this) {
         }
     }
     if (attrNode.ownerElement() != this) {
Mirror of the WebKit open source project from svn.webkit.org.