Attachment 371643 Details for Bug 198635 – Patch

[patch] Patch

bug-198635-20190607194021.patch (text/plain), 7.56 KB, created by Sihui Liu on 2019-06-07 19:40:21 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-06-07 19:40:21 PDT

Size: 7.56 KB

patch

obsolete

>Subversion Revision: 246225
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 1d11ad9236605671450624636dce5d9431cd08e9..1e79b7d4d0b86da1e002cf270603d5e1748fa148 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-06-07  Sihui Liu  <sihui_liu@apple.com>
>+
>+        [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=198635
>+        <rdar://problem/46010232>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: WebKit.WKHTTPCookieStoreWithoutProcessPoolDuplicates
>+
>+        * platform/Cookie.h:
>+        (WebCore::Cookie::isKeyEqual const):
>+        (WTF::HashTraits<WebCore::Cookie>::isEmptyValue):
>+
> 2019-06-07  Truitt Savell  <tsavell@apple.com>
> 
>         Unreviewed, rolling out r246138.
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4ca0f11eac727295b5d234531f74738f3a08bebc..56e197cada6e4461d9c2b66068d75fc5787f060e 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-06-07  Sihui Liu  <sihui_liu@apple.com>
>+
>+        [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=198635
>+        <rdar://problem/46010232>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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.
>+
>+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
>+        (WebKit::WebsiteDataStore::addPendingCookie):
>+
> 2019-06-05  Alex Christensen  <achristensen@webkit.org>
> 
>         Introduce new SPI for context menus on iOS
>diff --git a/Source/WebCore/platform/Cookie.h b/Source/WebCore/platform/Cookie.h
>index 1e05fcb362724c27fdabd8e2fb509490388c8dbe..a13365cbea39cabfdf10d65cdfdf9d5aa1444d84 100644
>--- a/Source/WebCore/platform/Cookie.h
>+++ b/Source/WebCore/platform/Cookie.h
>@@ -76,6 +76,13 @@ struct Cookie {
>             && 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,10 @@ namespace WTF {
>         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 emptyValueIsZero = false;
>+        static const bool hasIsEmptyValueFunction = true;
>+        static bool isEmptyValue(const WebCore::Cookie& slot) { return slot.isNull(); }
>     };
>     template<> struct EnumTraits<WebCore::Cookie::SameSitePolicy> {
>     using values = EnumValues<
>diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>index 3ba29b31e0f3e89ba154587c1521a8aea7869a4f..39bb2bcfa64206b374cdf65941261025cf907273 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>@@ -1941,6 +1941,13 @@ Vector<WebCore::Cookie> WebsiteDataStore::pendingCookies() const
> 
> void WebsiteDataStore::addPendingCookie(const WebCore::Cookie& cookie)
> {
>+    for (auto& pendingCookie : pendingCookies()) {
>+        if (pendingCookie.isKeyEqual(cookie)) {
>+            m_pendingCookies.remove(pendingCookie);
>+            break;
>+        }
>+    }
>+
>     m_pendingCookies.add(cookie);
> }
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index b47011d7b8a5873a696cffee1d8594a99ecac0e0..fa16392d334158127c4721f879b6a92302bf27c2 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-06-07  Sihui Liu  <sihui_liu@apple.com>
>+
>+        [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=198635
>+        <rdar://problem/46010232>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
>+        (areCookiesEqual):
>+        (TEST):
>+
> 2019-06-07  Daniel Bates  <dabates@apple.com>
> 
>         [lldb-webkit] Pretty-print all kinds of Documents
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>index 0f1e79ee16029dcb1ca8651e48efbe697b04020c..6e3b8534df82cdd4c2f7194b018cb4ad654b9f86 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>@@ -627,3 +627,58 @@ TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolEphemeralSession)
>     [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);
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 198635: 371543 | 371554 | 371606 | 371641 | 371643 | 371749