Crash in HTMLCollection::updateNamedElementCache
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2018 00:30:23 +0000 (00:30 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2018 00:30:23 +0000 (00:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192347

Reviewed by Darin Adler.

Source/WebCore:

The bug was caused by CollectionIndexCache's nodeAt caching the length of 1
when there are no matching elements in the subtree when the index is non-zero.

A related bug was fixed in r182125 but we were not considering the possibility
that the index given to this function might be non-zero even when there were
no matching elements.

Test: fast/dom/options-collection-zero-length-crash.html

* dom/CollectionIndexCache.h:
(WebCore::CollectionIndexCache<Collection, Iterator>::nodeAt):

LayoutTests:

Added a regression test. We can't simply call select.options.item
to catch this crash because the generated bidning code first call length()
to check if the index is within the valid range.

* fast/dom/options-collection-zero-length-crash-expected.txt: Added.
* fast/dom/options-collection-zero-length-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/options-collection-zero-length-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/options-collection-zero-length-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/CollectionIndexCache.h

index b9c902b..6577eb6 100644 (file)
@@ -1,3 +1,17 @@
+2018-12-04  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Crash in HTMLCollection::updateNamedElementCache
+        https://bugs.webkit.org/show_bug.cgi?id=192347
+
+        Reviewed by Darin Adler.
+
+        Added a regression test. We can't simply call select.options.item
+        to catch this crash because the generated bidning code first call length()
+        to check if the index is within the valid range.
+
+        * fast/dom/options-collection-zero-length-crash-expected.txt: Added.
+        * fast/dom/options-collection-zero-length-crash.html: Added.
+
 2018-11-30  Jiewen Tan  <jiewen_tan@apple.com>
 
         Don't report resource timing to parent frame for history items
diff --git a/LayoutTests/fast/dom/options-collection-zero-length-crash-expected.txt b/LayoutTests/fast/dom/options-collection-zero-length-crash-expected.txt
new file mode 100644 (file)
index 0000000..b8fd6bb
--- /dev/null
@@ -0,0 +1,11 @@
+This tests accessing the length after accessing a particular index in HTMLOptionsCollections via HTMLSelectElement's item. WebKit should not crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS select.item(500) is null
+PASS select.options.length is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/options-collection-zero-length-crash.html b/LayoutTests/fast/dom/options-collection-zero-length-crash.html
new file mode 100644 (file)
index 0000000..7542eca
--- /dev/null
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description('This tests accessing the length after accessing a particular index in HTMLOptionsCollections via HTMLSelectElement\'s item. WebKit should not crash.');
+
+const select = document.createElement('select');
+
+// Need to keep HTMLOptionsCollection alive during the call to item() and until the length getter is called.
+const optionsCollection = select.options;
+
+shouldBe('select.item(500)', 'null');
+shouldBe('select.options.length', '0');
+
+</script>
+</body>
+</html>
index b112c6e..f74f8ed 100644 (file)
@@ -1,3 +1,22 @@
+2018-12-04  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Crash in HTMLCollection::updateNamedElementCache
+        https://bugs.webkit.org/show_bug.cgi?id=192347
+
+        Reviewed by Darin Adler.
+
+        The bug was caused by CollectionIndexCache's nodeAt caching the length of 1
+        when there are no matching elements in the subtree when the index is non-zero.
+
+        A related bug was fixed in r182125 but we were not considering the possibility
+        that the index given to this function might be non-zero even when there were
+        no matching elements.
+
+        Test: fast/dom/options-collection-zero-length-crash.html
+
+        * dom/CollectionIndexCache.h:
+        (WebCore::CollectionIndexCache<Collection, Iterator>::nodeAt):
+
 2018-11-30  Jiewen Tan  <jiewen_tan@apple.com>
 
         Don't report resource timing to parent frame for history items
index 6e622e9..2801942 100644 (file)
@@ -203,13 +203,14 @@ inline typename CollectionIndexCache<Collection, Iterator>::NodeType* Collection
 
     m_current = collection.collectionBegin();
     m_currentIndex = 0;
-    if (index && m_current != end) {
+    bool startIsEnd = m_current == end;
+    if (index && !startIsEnd) {
         collection.collectionTraverseForward(m_current, index, m_currentIndex);
         ASSERT(m_current != end || m_currentIndex < index);
     }
     if (m_current == end) {
         // Failed to find the index but at least we now know the size.
-        m_nodeCount = index ? m_currentIndex + 1 : 0;
+        m_nodeCount = startIsEnd ? 0 : m_currentIndex + 1;
         m_nodeCountValid = true;
         return nullptr;
     }