Changeset 258521 in webkit

Timestamp:

Mar 16, 2020 3:04:02 PM (4 years ago)

Author:

Chris Dumez

Message:

Crash under WebCookieCache::clearForHost()
​https://bugs.webkit.org/show_bug.cgi?id=209149
<rdar://problem/60453086>

Reviewed by Alex Christensen.

Source/WebKit:

Make sure WebCookieCache::pruneCacheIfNecessary() keeps alive the host String it is passing
to WebCookieCache::clearForHost(). Previously, it was merely deferencing a HashSet iterator
and passing that to clearForHost(). However, clearForHost() would then drop the String from
the HashSet and the host would no longer be valid.

Change covered by new API test.

• WebProcess/WebPage/WebCookieCache.cpp:

(WebKit::WebCookieCache::pruneCacheIfNecessary):

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/CookiePrivateBrowsing.mm:

(TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/WebCookieCache.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/CookiePrivateBrowsing.mm (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-03-16  Chris Dumez  <cdumez@apple.com>
+
+        Crash under WebCookieCache::clearForHost()
+        https://bugs.webkit.org/show_bug.cgi?id=209149
+        <rdar://problem/60453086>
+
+        Reviewed by Alex Christensen.
+
+        Make sure WebCookieCache::pruneCacheIfNecessary() keeps alive the host String it is passing
+        to WebCookieCache::clearForHost(). Previously, it was merely deferencing a HashSet iterator
+        and passing that to clearForHost(). However, clearForHost() would then drop the String from
+        the HashSet and the host would no longer be valid.
+
+        Change covered by new API test.
+
+        * WebProcess/WebPage/WebCookieCache.cpp:
+        (WebKit::WebCookieCache::pruneCacheIfNecessary):
+
 2020-03-16  Per Arne Vollan  <pvollan@apple.com>
 

trunk/Source/WebKit/WebProcess/WebPage/WebCookieCache.cpp

@@ -119 +119 @@
     static const unsigned maxCachedHosts = 5;
 
-    while (m_hostsWithInMemoryStorage.size() >= maxCachedHosts)
-        clearForHost(*m_hostsWithInMemoryStorage.random());
+    while (m_hostsWithInMemoryStorage.size() >= maxCachedHosts) {
+        String hostToRemove = *m_hostsWithInMemoryStorage.random();
+        clearForHost(hostToRemove);
+    }
 }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2020-03-16  Chris Dumez  <cdumez@apple.com>
+
+        Crash under WebCookieCache::clearForHost()
+        https://bugs.webkit.org/show_bug.cgi?id=209149
+        <rdar://problem/60453086>
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/CookiePrivateBrowsing.mm:
+        (TEST):
+
 2020-03-16  Keith Rollin  <krollin@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/CookiePrivateBrowsing.mm

@@ -34 +34 @@
 #import <WebKit/WKWebViewConfiguration.h>
 #import <wtf/RetainPtr.h>
+#import <wtf/text/StringConcatenateNumbers.h>
 #import <wtf/text/WTFString.h>
 
@@ -129 +130 @@
     EXPECT_WK_STREQ("foo=bar", cookieString);
 }
+
+TEST(WebKit, CookieCachePruning)
+{
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    auto view = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+
+    for (unsigned i = 0; i < 100; ++i) {
+        [view synchronouslyLoadHTMLString:@"foo" baseURL:[NSURL URLWithString:makeString("http://foo", i, ".example.com/")]];
+
+        __block bool doneEvaluatingJavaScript = false;
+        [view evaluateJavaScript:@"document.cookie;" completionHandler:^(id _Nullable cookie, NSError * _Nullable error) {
+            EXPECT_NULL(error);
+            EXPECT_TRUE([cookie isKindOfClass:[NSString class]]);
+            EXPECT_WK_STREQ("", (NSString *)cookie);
+            doneEvaluatingJavaScript = true;
+        }];
+        TestWebKitAPI::Util::run(&doneEvaluatingJavaScript);
+    }
+}