Attachment 338837 Details for Bug 184938 – Patch

[patch] Patch

bug-184938-20180425174535.patch (text/plain), 7.95 KB, created by Sihui Liu on 2018-04-25 17:45:35 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2018-04-25 17:45:35 PDT

Size: 7.95 KB

patch

obsolete

>Subversion Revision: 231012
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ae2c3bb4251c27c2e60f5d70f94edd9353eb662d..79b9fbce83f7a9b35f97f44a1bcbaed64bea51d2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-04-25  Sihui Liu  <sihui_liu@apple.com>
>+
>+        -[WKHTTPCookieStore deleteCookie:completionHandler:] doesn't delete cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=184938
>+        <rdar://problem/34737395>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When a Cookie object was converted to NSHTTPCookie object, the HTTPOnly property information
>+        was lost so the delete function cannot find the proper cookie to delete.
>+        This patch implements a workaround that compares Cookie object instead of NSHTTPCookie 
>+        object. We might want to add the ability to set HTTPOnly header during conversion if there
>+        is an easy way to do it later.
>+        
>+        New API test: WebKit.WKHTTPCookieStoreHttpOnly
>+
>+        * platform/network/cocoa/CookieCocoa.mm:
>+        (WebCore::Cookie::operator== const):
>+        * platform/network/cocoa/NetworkStorageSessionCocoa.mm:
>+        (WebCore::NetworkStorageSession::deleteCookie):
>+
> 2018-04-25  Brent Fulgham  <bfulgham@apple.com>
> 
>         Don't Block First Party Cookies on Redirects
>diff --git a/Source/WebCore/platform/network/cocoa/CookieCocoa.mm b/Source/WebCore/platform/network/cocoa/CookieCocoa.mm
>index 8df8bb2ec550db92791c25f1d4f2d56206684e37..d94285f68e12389102ee6c554545b8c812e5bdf5 100644
>--- a/Source/WebCore/platform/network/cocoa/CookieCocoa.mm
>+++ b/Source/WebCore/platform/network/cocoa/CookieCocoa.mm
>@@ -132,8 +132,17 @@ bool Cookie::operator==(const Cookie& other) const
>     if (thisNull || otherNull)
>         return thisNull == otherNull;
>     
>-    NSHTTPCookie *nsCookie(*this);
>-    return [nsCookie isEqual:other];
>+    return name == other.name
>+    && value == other.value
>+    && domain == other.domain
>+    && path == other.path
>+    && created == other.created
>+    && expires == other.expires
>+    && httpOnly == other.httpOnly
>+    && secure == other.secure
>+    && session == other.session
>+    && comment == other.comment
>+    && commentURL == other.commentURL;
> }
>     
> unsigned Cookie::hash() const
>diff --git a/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm b/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm
>index b9a95fd459cf5e5853f0e0f89244ab5dec76f808..f499c2b0e140f174172aaf9de6b36b6490961051 100644
>--- a/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm
>+++ b/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm
>@@ -60,8 +60,14 @@ void NetworkStorageSession::setCookies(const Vector<Cookie>& cookies, const URL&
> void NetworkStorageSession::deleteCookie(const Cookie& cookie)
> {
>     ASSERT(hasProcessPrivilege(ProcessPrivilege::CanAccessRawCookies));
>-
>-    [nsCookieStorage() deleteCookie:(NSHTTPCookie *)cookie];
>+    
>+    NSArray *nsCookies = [nsCookieStorage() cookies];
>+    for (NSHTTPCookie *nsCookie in nsCookies) {
>+        if (Cookie(nsCookie) == cookie) {
>+            [nsCookieStorage() deleteCookie:nsCookie];
>+            break;
>+        }
>+    }
> }
> 
> static Vector<Cookie> nsCookiesToCookieVector(NSArray<NSHTTPCookie *> *nsCookies)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index bb5f3ef996e3f11b78a4226c62dc507db5da5b38..657156c81a1d5e003597ebbba14b1c78c7ac6813 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-04-25  Sihui Liu  <sihui_liu@apple.com>
>+
>+        -[WKHTTPCookieStore deleteCookie:completionHandler:] doesn't delete cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=184938
>+        <rdar://problem/34737395>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
>+        (TEST):
>+
> 2018-04-25  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         [WPE] Remove deprecated functions and properties from the API
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>index a8753320411a420c4f84fe600ae5662172c33cbf..2a4b7f74d0078a7a8e4962d7cfd98b8fc68b254b 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>@@ -192,6 +192,90 @@ TEST(WebKit, WKHTTPCookieStore)
>     runTestWithWebsiteDataStore([WKWebsiteDataStore defaultDataStore]);
> }
> 
>+TEST(WebKit, WKHTTPCookieStoreHttpOnly) 
>+{
>+    WKWebsiteDataStore* dataStore = [WKWebsiteDataStore defaultDataStore];
>+
>+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    configuration.get().websiteDataStore = dataStore;
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+
>+    [webView loadHTMLString:@"WebKit Test" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    [webView _test_waitForDidFinishNavigation];
>+
>+    [dataStore removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:[] {
>+        gotFlag = true;
>+    }];
>+
>+    TestWebKitAPI::Util::run(&gotFlag);
>+    gotFlag = false;
>+
>+    id pool = [WKProcessPool _sharedProcessPool];
>+    EXPECT_EQ([pool _pluginProcessCount], static_cast<size_t>(0));
>+
>+    globalCookieStore = dataStore.httpCookieStore;
>+
>+    NSArray<NSHTTPCookie *> *cookies = nil;
>+    [globalCookieStore getAllCookies:[cookiesPtr = &cookies](NSArray<NSHTTPCookie *> *nsCookies) {
>+        *cookiesPtr = [nsCookies retain];
>+        gotFlag = true;
>+    }];
>+
>+    TestWebKitAPI::Util::run(&gotFlag);
>+    gotFlag = false;
>+
>+    ASSERT_EQ(cookies.count, 0u);
>+    [cookies release];
>+
>+    CFMutableDictionaryRef cookieProperties = CFDictionaryCreateMutable(NULL, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
>+    CFDictionarySetValue(cookieProperties, CFSTR("Name"), CFSTR("httpCookie"));
>+    CFDictionarySetValue(cookieProperties, CFSTR("Value"), CFSTR("httpCookieValue"));
>+    CFDictionarySetValue(cookieProperties, CFSTR("Domain"), CFSTR(".www.webkit.org"));
>+    CFDictionarySetValue(cookieProperties, CFSTR("Path"), CFSTR("/httpPath"));
>+    CFDictionarySetValue(cookieProperties, CFSTR("HttpOnly"), kCFBooleanTrue);
>+    RetainPtr<NSHTTPCookie> httpOnlyCookie = [NSHTTPCookie cookieWithProperties:(NSDictionary*) cookieProperties];
>+    CFDictionaryRemoveValue(cookieProperties, CFSTR("HttpOnly"));
>+    RetainPtr<NSHTTPCookie> notHttpOnlyCookie = [NSHTTPCookie cookieWithProperties:(NSDictionary*) cookieProperties];
>+    EXPECT_TRUE(httpOnlyCookie.get().HTTPOnly);
>+    EXPECT_FALSE(notHttpOnlyCookie.get().HTTPOnly);
>+
>+    [globalCookieStore setCookie:notHttpOnlyCookie.get() completionHandler:[]() {
>+        gotFlag = true;
>+    }];
>+
>+    TestWebKitAPI::Util::run(&gotFlag);
>+    gotFlag = false;
>+
>+    [globalCookieStore getAllCookies:[cookiesPtr = &cookies](NSArray<NSHTTPCookie *> *nsCookies) {
>+        *cookiesPtr = [nsCookies retain];
>+        gotFlag = true;
>+    }];
>+
>+    TestWebKitAPI::Util::run(&gotFlag);
>+    gotFlag = false;
>+
>+    ASSERT_EQ(cookies.count, 1u);
>+    [cookies release];
>+
>+    [globalCookieStore deleteCookie:httpOnlyCookie.get() completionHandler:[]() {
>+        gotFlag = true;
>+    }];
>+
>+    TestWebKitAPI::Util::run(&gotFlag);
>+    gotFlag = false;
>+    [globalCookieStore getAllCookies:[cookiesPtr = &cookies](NSArray<NSHTTPCookie *> *nsCookies) {
>+        *cookiesPtr = [nsCookies retain];
>+        gotFlag = true;
>+    }];
>+
>+    TestWebKitAPI::Util::run(&gotFlag);
>+    gotFlag = false;
>+
>+    // Delete httpOnlyCookie should fail because it is different from notHttpOnlyCookie. 
>+    ASSERT_EQ(cookies.count, 1u);
>+    [cookies release];
>+}
>+
> // FIXME: This should be removed once <rdar://problem/35344202> is resolved and bots are updated.
> #if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MAX_ALLOWED <= 101301) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MAX_ALLOWED <= 110102)
> TEST(WebKit, WKHTTPCookieStoreNonPersistent)

Actions: View | Formatted Diff | Diff

Attachments on bug 184938: 338680 | 338837 | 338846 | 338867