[Cocoa] Set HTTPOnly flag when converting Cookie to NSHTTPCookie
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Apr 2018 05:26:40 +0000 (05:26 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Apr 2018 05:26:40 +0000 (05:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185052

Patch by Sihui Liu <sihui_liu@apple.com> on 2018-04-28
Reviewed by Geoffrey Garen.

Source/WebCore:

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):

Tools:

Modified API test to provide correct test cases for HTTPOnly flag.

* TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
(TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231146 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/network/cocoa/CookieCocoa.mm
Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm

index a9717c7..fc0ba27 100644 (file)
@@ -1,3 +1,23 @@
+2018-04-28  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 Geoffrey Garen.
+
+        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-28  Zalan Bujtas  <zalan@apple.com>
 
         [LFC] Add LayoutTreeBuilder class to generate the layout tree
index 19adf2a..575579d 100644 (file)
@@ -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
index f499c2b..3a3df0f 100644 (file)
@@ -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)
index 32e0e56..5f79440 100644 (file)
@@ -1,3 +1,15 @@
+2018-04-28  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 Geoffrey Garen.
+
+        Modified API test to provide correct test cases for HTTPOnly flag.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
+        (TEST):
+
 2018-04-28  Rick Waldron  <waldron.rick@gmail.com>
 
         Token misspelled "tocken" in error message string
index 2a4b7f7..9d5d7ec 100644 (file)
@@ -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];
 }