Title: [294069] branches/safari-613-branch
Revision
294069
Author
[email protected]
Date
2022-05-11 13:40:08 -0700 (Wed, 11 May 2022)

Log Message

Cherry-pick r293736. rdar://80891555

StorageMap::removeItem may fail to remove item from map
https://bugs.webkit.org/show_bug.cgi?id=239982
rdar://80891555

Reviewed by Chris Dumez.

Source/WebCore:

We may have updated m_impl, but we don't update iterator for removal. In this case, item is not removed from
map, but currentSize is updated. The mismatch between currentSize and actual size of the map may lead to
underflow and overflow in currentSize when item is added or removed later.

Test: storage/domstorage/sessionstorage/window-open-remove-item.html

* storage/StorageMap.cpp:
(WebCore::StorageMap::removeItem):

LayoutTests:

* storage/domstorage/sessionstorage/resources/window-open-remove-item.html: Added.
* storage/domstorage/sessionstorage/window-open-remove-item-expected.txt: Added.
* storage/domstorage/sessionstorage/window-open-remove-item.html: Added.

Canonical link: https://commits.webkit.org/[email protected]

Modified Paths

Added Paths

Diff

Modified: branches/safari-613-branch/LayoutTests/ChangeLog (294068 => 294069)


--- branches/safari-613-branch/LayoutTests/ChangeLog	2022-05-11 20:40:03 UTC (rev 294068)
+++ branches/safari-613-branch/LayoutTests/ChangeLog	2022-05-11 20:40:08 UTC (rev 294069)
@@ -1,3 +1,15 @@
+2022-05-03  Sihui Liu  <[email protected]>
+
+        StorageMap::removeItem may fail to remove item from map
+        https://bugs.webkit.org/show_bug.cgi?id=239982
+        rdar://80891555
+
+        Reviewed by Chris Dumez.
+
+        * storage/domstorage/sessionstorage/resources/window-open-remove-item.html: Added.
+        * storage/domstorage/sessionstorage/window-open-remove-item-expected.txt: Added.
+        * storage/domstorage/sessionstorage/window-open-remove-item.html: Added.
+
 2022-04-26  Russell Epstein  <[email protected]>
 
         Cherry-pick r290630. rdar://problem/89442583

Added: branches/safari-613-branch/LayoutTests/storage/domstorage/sessionstorage/resources/window-open-remove-item.html (0 => 294069)


--- branches/safari-613-branch/LayoutTests/storage/domstorage/sessionstorage/resources/window-open-remove-item.html	                        (rev 0)
+++ branches/safari-613-branch/LayoutTests/storage/domstorage/sessionstorage/resources/window-open-remove-item.html	2022-05-11 20:40:08 UTC (rev 294069)
@@ -0,0 +1,15 @@
+<html>
+<body>
+<script>
+
+if (sessionStorage.getItem("key") != "value")
+    localStorage.setItem("result", "fail");
+else {
+    sessionStorage.removeItem("key");
+    sessionStorage.setItem("key", "newValue");
+    localStorage.setItem("result", "pass");
+}
+
+</script>
+</body>
+</html>
\ No newline at end of file

Added: branches/safari-613-branch/LayoutTests/storage/domstorage/sessionstorage/window-open-remove-item-expected.txt (0 => 294069)


--- branches/safari-613-branch/LayoutTests/storage/domstorage/sessionstorage/window-open-remove-item-expected.txt	                        (rev 0)
+++ branches/safari-613-branch/LayoutTests/storage/domstorage/sessionstorage/window-open-remove-item-expected.txt	2022-05-11 20:40:08 UTC (rev 294069)
@@ -0,0 +1,11 @@
+Test verifies that process does not crash when item is updated in another window
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS localStorage.getItem('result') is "pass"
+PASS sessionStorage.getItem('key') is "value"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: branches/safari-613-branch/LayoutTests/storage/domstorage/sessionstorage/window-open-remove-item.html (0 => 294069)


--- branches/safari-613-branch/LayoutTests/storage/domstorage/sessionstorage/window-open-remove-item.html	                        (rev 0)
+++ branches/safari-613-branch/LayoutTests/storage/domstorage/sessionstorage/window-open-remove-item.html	2022-05-11 20:40:08 UTC (rev 294069)
@@ -0,0 +1,24 @@
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description("Test verifies that process does not crash when item is updated in another window");
+jsTestIsAsync = true;
+
+sessionStorage.clear();
+localStorage.clear();
+
+sessionStorage.setItem("key", "value");
+window.open("resources/window-open-remove-item.html");
+
+addEventListener('storage', event => { 
+    shouldBeEqualToString("localStorage.getItem('result')", "pass");
+    shouldBeEqualToString("sessionStorage.getItem('key')", "value");
+    finishJSTest();
+});
+
+</script>
+</body>
+</html>
\ No newline at end of file

Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (294068 => 294069)


--- branches/safari-613-branch/Source/WebCore/ChangeLog	2022-05-11 20:40:03 UTC (rev 294068)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog	2022-05-11 20:40:08 UTC (rev 294069)
@@ -1,3 +1,20 @@
+2022-05-03  Sihui Liu  <[email protected]>
+
+        StorageMap::removeItem may fail to remove item from map
+        https://bugs.webkit.org/show_bug.cgi?id=239982
+        rdar://80891555
+
+        Reviewed by Chris Dumez.
+
+        We may have updated m_impl, but we don't update iterator for removal. In this case, item is not removed from
+        map, but currentSize is updated. The mismatch between currentSize and actual size of the map may lead to
+        underflow and overflow in currentSize when item is added or removed later.
+
+        Test: storage/domstorage/sessionstorage/window-open-remove-item.html
+
+        * storage/StorageMap.cpp:
+        (WebCore::StorageMap::removeItem):
+
 2022-05-04  Alan Coon  <[email protected]>
 
         Apply patch. rdar://problem/75450208

Modified: branches/safari-613-branch/Source/WebCore/storage/StorageMap.cpp (294068 => 294069)


--- branches/safari-613-branch/Source/WebCore/storage/StorageMap.cpp	2022-05-11 20:40:03 UTC (rev 294068)
+++ branches/safari-613-branch/Source/WebCore/storage/StorageMap.cpp	2022-05-11 20:40:08 UTC (rev 294069)
@@ -138,7 +138,7 @@
     if (m_impl->refCount() > 1)
         m_impl = m_impl->copy();
 
-    m_impl->map.remove(iter);
+    m_impl->map.remove(key);
     m_impl->currentSize = newSize;
     invalidateIterator();
 }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to