Title: [246264] trunk
- Revision
- 246264
- Author
- [email protected]
- Date
- 2019-06-10 09:16:52 -0700 (Mon, 10 Jun 2019)
Log Message
[WKHTTPCookieStore getAllCookies:] may return duplicate cookies
https://bugs.webkit.org/show_bug.cgi?id=198635
<rdar://problem/46010232>
Reviewed by Ryosuke Niwa.
Source/WebCore:
Test: WebKit.WKHTTPCookieStoreWithoutProcessPoolDuplicates
* platform/Cookie.h:
(WebCore::Cookie::isKeyEqual const):
(WTF::HashTraits<WebCore::Cookie>::isEmptyValue):
Source/WebKit:
When there is no process pool, we store cookies set in memory with HashSet m_pendingCookies of WebsiteDataStore.
HashSet does not contain duplicate Cookies that are completely identical, but it may contain Cookies that have
all the other properties identical other than value. This is not correct because Cookies with same name, domain
and path should be treated as the same cookie. When a cookie is set via API, we should either insert the
cookie into m_pendingCookies if the cookie does not exist, or update the cookie value if it already exists.
Note that we still use HashSet with CookieHash for m_pendingCookies because in cookie deletion, we only delete
cookie when there is a complete match. If some cookie from m_pendingCookies has all other properties the same as
the cookie specified in the deletion function, but the value is different, it will not be removed.
* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::addPendingCookie):
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
(areCookiesEqual):
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (246263 => 246264)
--- trunk/Source/WebCore/ChangeLog 2019-06-10 15:59:14 UTC (rev 246263)
+++ trunk/Source/WebCore/ChangeLog 2019-06-10 16:16:52 UTC (rev 246264)
@@ -1,3 +1,17 @@
+2019-06-10 Sihui Liu <[email protected]>
+
+ [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
+ https://bugs.webkit.org/show_bug.cgi?id=198635
+ <rdar://problem/46010232>
+
+ Reviewed by Ryosuke Niwa.
+
+ Test: WebKit.WKHTTPCookieStoreWithoutProcessPoolDuplicates
+
+ * platform/Cookie.h:
+ (WebCore::Cookie::isKeyEqual const):
+ (WTF::HashTraits<WebCore::Cookie>::isEmptyValue):
+
2019-06-09 Rob Buis <[email protected]>
Add wildcard to Access-Control-Allow-Methods and Access-Control-Allow-Headers
Modified: trunk/Source/WebCore/platform/Cookie.h (246263 => 246264)
--- trunk/Source/WebCore/platform/Cookie.h 2019-06-10 15:59:14 UTC (rev 246263)
+++ trunk/Source/WebCore/platform/Cookie.h 2019-06-10 16:16:52 UTC (rev 246264)
@@ -76,6 +76,13 @@
&& commentURL.isNull();
}
+ bool isKeyEqual(const Cookie& otherCookie) const
+ {
+ return name == otherCookie.name
+ && domain == otherCookie.domain
+ && path == otherCookie.path;
+ }
+
String name;
String value;
String domain;
@@ -169,6 +176,9 @@
static WebCore::Cookie emptyValue() { return { }; }
static void constructDeletedValue(WebCore::Cookie& slot) { slot = WebCore::Cookie(WTF::HashTableDeletedValue); }
static bool isDeletedValue(const WebCore::Cookie& slot) { return slot.name.isHashTableDeletedValue(); }
+
+ static const bool hasIsEmptyValueFunction = true;
+ static bool isEmptyValue(const WebCore::Cookie& slot) { return slot.isNull(); }
};
template<> struct EnumTraits<WebCore::Cookie::SameSitePolicy> {
using values = EnumValues<
Modified: trunk/Source/WebKit/ChangeLog (246263 => 246264)
--- trunk/Source/WebKit/ChangeLog 2019-06-10 15:59:14 UTC (rev 246263)
+++ trunk/Source/WebKit/ChangeLog 2019-06-10 16:16:52 UTC (rev 246264)
@@ -1,3 +1,25 @@
+2019-06-10 Sihui Liu <[email protected]>
+
+ [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
+ https://bugs.webkit.org/show_bug.cgi?id=198635
+ <rdar://problem/46010232>
+
+ Reviewed by Ryosuke Niwa.
+
+ When there is no process pool, we store cookies set in memory with HashSet m_pendingCookies of WebsiteDataStore.
+
+ HashSet does not contain duplicate Cookies that are completely identical, but it may contain Cookies that have
+ all the other properties identical other than value. This is not correct because Cookies with same name, domain
+ and path should be treated as the same cookie. When a cookie is set via API, we should either insert the
+ cookie into m_pendingCookies if the cookie does not exist, or update the cookie value if it already exists.
+
+ Note that we still use HashSet with CookieHash for m_pendingCookies because in cookie deletion, we only delete
+ cookie when there is a complete match. If some cookie from m_pendingCookies has all other properties the same as
+ the cookie specified in the deletion function, but the value is different, it will not be removed.
+
+ * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+ (WebKit::WebsiteDataStore::addPendingCookie):
+
2019-06-10 Philippe Normand <[email protected]>
[WPE][Qt] Port to new wpe_fdo_egl_exported_image API
Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (246263 => 246264)
--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2019-06-10 15:59:14 UTC (rev 246263)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2019-06-10 16:16:52 UTC (rev 246264)
@@ -1941,6 +1941,9 @@
void WebsiteDataStore::addPendingCookie(const WebCore::Cookie& cookie)
{
+ m_pendingCookies.removeIf([&cookie](auto& pendingCookie) {
+ return pendingCookie.isKeyEqual(cookie);
+ });
m_pendingCookies.add(cookie);
}
Modified: trunk/Tools/ChangeLog (246263 => 246264)
--- trunk/Tools/ChangeLog 2019-06-10 15:59:14 UTC (rev 246263)
+++ trunk/Tools/ChangeLog 2019-06-10 16:16:52 UTC (rev 246264)
@@ -1,3 +1,15 @@
+2019-06-10 Sihui Liu <[email protected]>
+
+ [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
+ https://bugs.webkit.org/show_bug.cgi?id=198635
+ <rdar://problem/46010232>
+
+ Reviewed by Ryosuke Niwa.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
+ (areCookiesEqual):
+ (TEST):
+
2019-06-10 Adrian Perez de Castro <[email protected]>
[JHBuild] Bump shared-mime-info to version ≥ 1.6
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm (246263 => 246264)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm 2019-06-10 15:59:14 UTC (rev 246263)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm 2019-06-10 16:16:52 UTC (rev 246264)
@@ -627,3 +627,58 @@
[webView loadHTMLString:alertCookieHTML baseURL:[NSURL URLWithString:@"http://127.0.0.1"]];
TestWebKitAPI::Util::run(&finished);
}
+
+static bool areCookiesEqual(NSHTTPCookie *first, NSHTTPCookie *second)
+{
+ return [first.name isEqual:second.name] && [first.domain isEqual:second.domain] && [first.path isEqual:second.path] && [first.value isEqual:second.value];
+}
+
+TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolDuplicates)
+{
+ RetainPtr<WKHTTPCookieStore> httpCookieStore = [WKWebsiteDataStore defaultDataStore].httpCookieStore;
+ RetainPtr<NSHTTPCookie> sessionCookie = [NSHTTPCookie cookieWithProperties:@{
+ NSHTTPCookiePath: @"/",
+ NSHTTPCookieName: @"SessionCookieName",
+ NSHTTPCookieValue: @"CookieValue",
+ NSHTTPCookieDomain: @"127.0.0.1",
+ }];
+
+ auto properties = adoptNS([sessionCookie.get().properties mutableCopy]);
+ properties.get()[NSHTTPCookieDomain] = @"localhost";
+ RetainPtr<NSHTTPCookie> sessionCookieDifferentDomain = [NSHTTPCookie cookieWithProperties:properties.get()];
+ properties.get()[NSHTTPCookieValue] = @"OtherCookieValue";
+ RetainPtr<NSHTTPCookie> sessionCookieDifferentValue = [NSHTTPCookie cookieWithProperties:properties.get()];
+ finished = false;
+
+ [httpCookieStore.get() setCookie:sessionCookie.get() completionHandler:^{
+ finished = true;
+ }];
+ TestWebKitAPI::Util::run(&finished);
+ finished = false;
+
+ [httpCookieStore.get() setCookie:sessionCookieDifferentDomain.get() completionHandler:^{
+ finished = true;
+ }];
+ TestWebKitAPI::Util::run(&finished);
+ finished = false;
+
+ [httpCookieStore.get() setCookie:sessionCookieDifferentValue.get() completionHandler:^{
+ finished = true;
+ }];
+ TestWebKitAPI::Util::run(&finished);
+ finished = false;
+
+ [httpCookieStore.get() getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
+ EXPECT_EQ(2u, cookies.count);
+ bool sessionCookieExists = false, otherSessionCookieExists = false;
+ for (NSHTTPCookie* cookie in cookies) {
+ if (areCookiesEqual(cookie, sessionCookie.get()))
+ sessionCookieExists = true;
+ else if (areCookiesEqual(cookie, sessionCookieDifferentValue.get()))
+ otherSessionCookieExists = true;
+ }
+ EXPECT_TRUE(sessionCookieExists && otherSessionCookieExists);
+ finished = true;
+ }];
+ TestWebKitAPI::Util::run(&finished);
+}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
