Title: [238880] trunk
Revision
238880
Author
rn...@webkit.org
Date
2018-12-04 16:30:23 -0800 (Tue, 04 Dec 2018)

Log Message

Crash in HTMLCollection::updateNamedElementCache
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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (238879 => 238880)


--- trunk/LayoutTests/ChangeLog	2018-12-04 23:48:11 UTC (rev 238879)
+++ trunk/LayoutTests/ChangeLog	2018-12-05 00:30:23 UTC (rev 238880)
@@ -1,3 +1,17 @@
+2018-12-04  Ryosuke Niwa  <rn...@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_...@apple.com>
 
         Don't report resource timing to parent frame for history items

Added: trunk/LayoutTests/fast/dom/options-collection-zero-length-crash-expected.txt (0 => 238880)


--- trunk/LayoutTests/fast/dom/options-collection-zero-length-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/options-collection-zero-length-crash-expected.txt	2018-12-05 00:30:23 UTC (rev 238880)
@@ -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
+

Added: trunk/LayoutTests/fast/dom/options-collection-zero-length-crash.html (0 => 238880)


--- trunk/LayoutTests/fast/dom/options-collection-zero-length-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/options-collection-zero-length-crash.html	2018-12-05 00:30:23 UTC (rev 238880)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<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>

Modified: trunk/Source/WebCore/ChangeLog (238879 => 238880)


--- trunk/Source/WebCore/ChangeLog	2018-12-04 23:48:11 UTC (rev 238879)
+++ trunk/Source/WebCore/ChangeLog	2018-12-05 00:30:23 UTC (rev 238880)
@@ -1,3 +1,22 @@
+2018-12-04  Ryosuke Niwa  <rn...@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_...@apple.com>
 
         Don't report resource timing to parent frame for history items

Modified: trunk/Source/WebCore/dom/CollectionIndexCache.h (238879 => 238880)


--- trunk/Source/WebCore/dom/CollectionIndexCache.h	2018-12-04 23:48:11 UTC (rev 238879)
+++ trunk/Source/WebCore/dom/CollectionIndexCache.h	2018-12-05 00:30:23 UTC (rev 238880)
@@ -203,13 +203,14 @@
 
     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;
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to