Attachment 339042 Details for Bug 185052 – Patch

[patch] Patch

bug-185052-20180427171759.patch (text/plain), 10.39 KB, created by Sihui Liu on 2018-04-27 17:17:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2018-04-27 17:17:59 PDT

Size: 10.39 KB

patch

obsolete

>Subversion Revision: 231100
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c98d01b98a327c6fbc0bbd78238d424caf935f45..9610675be57b77534c142c677f46ec2a3fe99be0 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-04-27  Sihui Liu  <sihui_liu@apple.com>
>+
>+        [Cocoa] Set HTTPOnly flag when converting Cookie to NSHTTPCookie
>+        https://bugs.webkit.org/show_bug.cgi?id=185052
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Set HTTPOnly for NSHTTPCookie when it's converted from Cookie, so the WebKit APIs could 
>+        create NSHTTPCookie with correct HTTPOnly flag. Also, reverted the change made to operator
>+        function because we want the Cookie class to act as a wrapper for NSHTTPCookie and leverage
>+        its equal function. 
>+
>+        Modified API test: WebKit.WKHTTPCookieStoreHttpOnly
>+
>+        * platform/network/cocoa/CookieCocoa.mm:
>+        (WebCore::Cookie::operator NSHTTPCookie * const):
>+        (WebCore::Cookie::operator== const):
>+        * platform/network/cocoa/NetworkStorageSessionCocoa.mm:
>+        (WebCore::NetworkStorageSession::deleteCookie):
>+
> 2018-04-26  Simon Fraser  <simon.fraser@apple.com>
> 
>         Fix color-filter to apply to text decorations
>diff --git a/Source/WebCore/platform/network/cocoa/CookieCocoa.mm b/Source/WebCore/platform/network/cocoa/CookieCocoa.mm
>index 19adf2a21aa7929ba59bf5e1d22227c3cab7a910..575579dbbbdc3a104f9f461a14cb8380fa9c6abd 100644
>--- a/Source/WebCore/platform/network/cocoa/CookieCocoa.mm
>+++ b/Source/WebCore/platform/network/cocoa/CookieCocoa.mm
>@@ -82,9 +82,7 @@ Cookie::operator NSHTTPCookie *() const
>     if (isNull())
>         return nil;
> 
>-    // FIXME: existing APIs do not provide a way to set httpOnly without parsing headers from scratch.
>-
>-    NSMutableDictionary *properties = [NSMutableDictionary dictionaryWithCapacity:11];
>+    NSMutableDictionary *properties = [NSMutableDictionary dictionaryWithCapacity:12];
> 
>     if (!comment.isNull())
>         [properties setObject:(NSString *)comment forKey:NSHTTPCookieComment];
>@@ -118,6 +116,9 @@ Cookie::operator NSHTTPCookie *() const
> 
>     if (session)
>         [properties setObject:@YES forKey:NSHTTPCookieDiscard];
>+    
>+    if (httpOnly)
>+        [properties setObject:@YES forKey:@"HttpOnly"];
> 
>     [properties setObject:@"1" forKey:NSHTTPCookieVersion];
> 
>@@ -132,18 +133,8 @@ bool Cookie::operator==(const Cookie& other) const
>     if (thisNull || otherNull)
>         return thisNull == otherNull;
>     
>-    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
>-    && ports == other.ports;
>+    NSHTTPCookie *nsCookie(*this);
>+    return [nsCookie isEqual:other];
> }
>     
> unsigned Cookie::hash() const
>diff --git a/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm b/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm
>index f499c2b0e140f174172aaf9de6b36b6490961051..3a3df0fa5036ca1599ba323b051ab303291bfb94 100644
>--- a/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm
>+++ b/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm
>@@ -60,14 +60,7 @@ void NetworkStorageSession::setCookies(const Vector<Cookie>& cookies, const URL&
> void NetworkStorageSession::deleteCookie(const Cookie& cookie)
> {
>     ASSERT(hasProcessPrivilege(ProcessPrivilege::CanAccessRawCookies));
>-    
>-    NSArray *nsCookies = [nsCookieStorage() cookies];
>-    for (NSHTTPCookie *nsCookie in nsCookies) {
>-        if (Cookie(nsCookie) == cookie) {
>-            [nsCookieStorage() deleteCookie:nsCookie];
>-            break;
>-        }
>-    }
>+    [nsCookieStorage() deleteCookie:(NSHTTPCookie *)cookie];
> }
> 
> static Vector<Cookie> nsCookiesToCookieVector(NSArray<NSHTTPCookie *> *nsCookies)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 7dc32fbb5e067ee1a9d2181848698c20abfc9ae3..67b008d8edf905480393a16152688cc83a8f4764 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-04-27  Sihui Liu  <sihui_liu@apple.com>
>+
>+        [Cocoa] Set HTTPOnly flag when converting Cookie to NSHTTPCookie
>+        https://bugs.webkit.org/show_bug.cgi?id=185052
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Modified API test to provide correct test cases for HTTPOnly flag.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
>+        (TEST):
>+
> 2018-04-27  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [Extra zoom mode] Add a mechanism to override default viewport behaviors in extra zoom mode
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>index 2a4b7f74d0078a7a8e4962d7cfd98b8fc68b254b..9d5d7ecd6dc2a4719cd7d49fcc0a254ecd619798 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>@@ -206,13 +206,9 @@ TEST(WebKit, WKHTTPCookieStoreHttpOnly)
>     [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;
>@@ -220,29 +216,37 @@ TEST(WebKit, WKHTTPCookieStoreHttpOnly)
>         *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];
>+    NSMutableDictionary *cookieProperties = [[NSMutableDictionary alloc] init];
>+    [cookieProperties setObject:@"cookieName" forKey:NSHTTPCookieName];
>+    [cookieProperties setObject:@"cookieValue" forKey:NSHTTPCookieValue];
>+    [cookieProperties setObject:@".www.webkit.org" forKey:NSHTTPCookieDomain];
>+    [cookieProperties setObject:@"/path" forKey:NSHTTPCookiePath];
>+    [cookieProperties setObject:@YES forKey:@"HttpOnly"];
>+    RetainPtr<NSHTTPCookie> httpOnlyCookie = [NSHTTPCookie cookieWithProperties:cookieProperties];
>+    [cookieProperties setObject:@"cookieValue2" forKey:NSHTTPCookieValue];
>+    RetainPtr<NSHTTPCookie> httpOnlyCookie2 = [NSHTTPCookie cookieWithProperties:cookieProperties];
>+    [cookieProperties removeObjectForKey:@"HttpOnly"];
>+    RetainPtr<NSHTTPCookie> notHttpOnlyCookie = [NSHTTPCookie cookieWithProperties:cookieProperties];
>+
>     EXPECT_TRUE(httpOnlyCookie.get().HTTPOnly);
>     EXPECT_FALSE(notHttpOnlyCookie.get().HTTPOnly);
> 
>-    [globalCookieStore setCookie:notHttpOnlyCookie.get() completionHandler:[]() {
>+    // Setting httpOnlyCookie should succeed.
>+    [globalCookieStore setCookie:httpOnlyCookie.get() completionHandler:[]() {
>         gotFlag = true;
>     }];
>+    TestWebKitAPI::Util::run(&gotFlag);
>+    gotFlag = false;
> 
>+    // Setting httpOnlyCookie2 should succeed.
>+    [globalCookieStore setCookie:httpOnlyCookie2.get() completionHandler:[]() {
>+        gotFlag = true;
>+    }];
>     TestWebKitAPI::Util::run(&gotFlag);
>     gotFlag = false;
> 
>@@ -250,29 +254,59 @@ TEST(WebKit, WKHTTPCookieStoreHttpOnly)
>         *cookiesPtr = [nsCookies retain];
>         gotFlag = true;
>     }];
>+    TestWebKitAPI::Util::run(&gotFlag);
>+    gotFlag = false;
>+    ASSERT_EQ(cookies.count, 1u);
>+    EXPECT_TRUE([[[cookies objectAtIndex:0] value] isEqual:@"cookieValue2"]);
>+    [cookies release];
> 
>+    // Setting notHttpOnlyCookie should fail because we cannot overwrite HTTPOnly property using public API.
>+    [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);
>+    EXPECT_TRUE([[cookies objectAtIndex:0] isHTTPOnly]);
>     [cookies release];
> 
>-    [globalCookieStore deleteCookie:httpOnlyCookie.get() completionHandler:[]() {
>+    // Deleting notHttpOnlyCookie should fail because the cookie stored is HTTPOnly.
>+    [globalCookieStore deleteCookie: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];
> 
>+    // Deleting httpOnlyCookie should succeed. 
>+    [globalCookieStore deleteCookie:httpOnlyCookie.get() completionHandler:[]() {
>+        gotFlag = true;
>+    }];
>     TestWebKitAPI::Util::run(&gotFlag);
>     gotFlag = false;
> 
>-    // Delete httpOnlyCookie should fail because it is different from notHttpOnlyCookie. 
>-    ASSERT_EQ(cookies.count, 1u);
>+    [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];
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 185052: 339042 | 339048