Title: [214270] trunk
Revision
214270
Author
wilan...@apple.com
Date
2017-03-22 12:43:25 -0700 (Wed, 22 Mar 2017)

Log Message

Resource Load Statistics: Fix partitioning bug for client-side cookie access
https://bugs.webkit.org/show_bug.cgi?id=169906
<rdar://problem/31164456>

Reviewed by Alex Christensen.

Source/WebCore:

The existing test case was expanded to cover this change.

* platform/network/NetworkStorageSession.h:
    Moved the two cookieStoragePartition() functions into the class.
    Also declared them const.
* platform/network/cf/NetworkStorageSessionCFNet.cpp:
(WebCore::NetworkStorageSession::cookieStoragePartition):
    Now checks whether it should partition or not.
(WebCore::getPartitioningDomain):
    Inline convenience function.
(WebCore::NetworkStorageSession::shouldPartitionCookies):
    Renamed since it now receives a top privately controlled domain
    instead of a host.
(WebCore::cookieStoragePartition): Deleted.
    This moved into NetworkStorageSession.
(WebCore::hostIsInDomain): Deleted.
    No longer needed since we generate the top privately controlled
    domain for both the resource and the partition.
(WebCore::NetworkStorageSession::shouldPartitionCookiesForHost): Deleted.
    Renamed WebCore::NetworkStorageSession::shouldPartitionCookies().
* platform/network/mac/CookieJarMac.mm:
(WebCore::cookiesInPartitionForURL):
    Now calls WebCore::NetworkStorageSession::cookieStoragePartition().
(WebCore::setCookiesFromDOM):
    Now calls WebCore::NetworkStorageSession::cookieStoragePartition().
* platform/network/mac/ResourceHandleMac.mm:
(WebCore::ResourceHandle::createNSURLConnection):
    Now calls WebCore::NetworkStorageSession::cookieStoragePartition().

Source/WebKit2:

* NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
(WebKit::NetworkDataTaskCocoa::NetworkDataTaskCocoa):
    This function no longer asks the NetworkStorageSession
    whether it should partition or not. That decision is
    now made as part of
    WebCore::NetworkStorageSession::cookieStoragePartition().
    The empty partition signals no partitioning.

LayoutTests:

* http/tests/loading/resourceLoadStatistics/partitioned-cookies-with-and-without-user-interaction-expected.txt:
* http/tests/loading/resourceLoadStatistics/resources/get-cookies.php:
    Now also outputs document.cookie client-side.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (214269 => 214270)


--- trunk/LayoutTests/ChangeLog	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/LayoutTests/ChangeLog	2017-03-22 19:43:25 UTC (rev 214270)
@@ -1,3 +1,15 @@
+2017-03-22  John Wilander  <wilan...@apple.com>
+
+        Resource Load Statistics: Fix partitioning bug for client-side cookie access
+        https://bugs.webkit.org/show_bug.cgi?id=169906
+        <rdar://problem/31164456>
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/loading/resourceLoadStatistics/partitioned-cookies-with-and-without-user-interaction-expected.txt:
+        * http/tests/loading/resourceLoadStatistics/resources/get-cookies.php:
+            Now also outputs document.cookie client-side.
+
 2017-03-22  Jer Noble  <jer.no...@apple.com>
 
         Media files served without an extension will not load in <video> tag.

Modified: trunk/LayoutTests/http/tests/loading/resourceLoadStatistics/partitioned-cookies-with-and-without-user-interaction-expected.txt (214269 => 214270)


--- trunk/LayoutTests/http/tests/loading/resourceLoadStatistics/partitioned-cookies-with-and-without-user-interaction-expected.txt	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/LayoutTests/http/tests/loading/resourceLoadStatistics/partitioned-cookies-with-and-without-user-interaction-expected.txt	2017-03-22 19:43:25 UTC (rev 214270)
@@ -50,8 +50,8 @@
 Should receive no cookies.
 Did not receive cookie named 'firstPartyCookie'.
 Did not receive cookie named 'thirdPartyCookie'.
+Client-side document.cookie:
 
-
 --------
 Frame: '<!--framePath //<!--frame1-->-->'
 --------
@@ -64,8 +64,8 @@
 Should only receive partitioned, third party cookie.
 Did not receive cookie named 'firstPartyCookie'.
 Received cookie named 'thirdPartyCookie'.
+Client-side document.cookie: thirdPartyCookie=value
 
-
 --------
 Frame: '<!--framePath //<!--frame3-->-->'
 --------
@@ -72,4 +72,4 @@
 After user interaction, should only receive non-partitioned, first party cookie.
 Received cookie named 'firstPartyCookie'.
 Did not receive cookie named 'thirdPartyCookie'.
-
+Client-side document.cookie: firstPartyCookie=value

Modified: trunk/LayoutTests/http/tests/loading/resourceLoadStatistics/resources/get-cookies.php (214269 => 214270)


--- trunk/LayoutTests/http/tests/loading/resourceLoadStatistics/resources/get-cookies.php	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/LayoutTests/http/tests/loading/resourceLoadStatistics/resources/get-cookies.php	2017-03-22 19:43:25 UTC (rev 214270)
@@ -11,3 +11,7 @@
     echo "Received cookie named '" . $_GET["name2"] . "'.<br>";
 }
 ?>
+<p id="output"></p>
+<script>
+    document.getElementById("output").textContent = "Client-side document.cookie: " + document.cookie;
+</script>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (214269 => 214270)


--- trunk/Source/WebCore/ChangeLog	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/Source/WebCore/ChangeLog	2017-03-22 19:43:25 UTC (rev 214270)
@@ -1,3 +1,40 @@
+2017-03-22  John Wilander  <wilan...@apple.com>
+
+        Resource Load Statistics: Fix partitioning bug for client-side cookie access
+        https://bugs.webkit.org/show_bug.cgi?id=169906
+        <rdar://problem/31164456>
+
+        Reviewed by Alex Christensen.
+
+        The existing test case was expanded to cover this change.
+
+        * platform/network/NetworkStorageSession.h:
+            Moved the two cookieStoragePartition() functions into the class.
+            Also declared them const.
+        * platform/network/cf/NetworkStorageSessionCFNet.cpp:
+        (WebCore::NetworkStorageSession::cookieStoragePartition):
+            Now checks whether it should partition or not.
+        (WebCore::getPartitioningDomain):
+            Inline convenience function.
+        (WebCore::NetworkStorageSession::shouldPartitionCookies):
+            Renamed since it now receives a top privately controlled domain
+            instead of a host.
+        (WebCore::cookieStoragePartition): Deleted.
+            This moved into NetworkStorageSession.
+        (WebCore::hostIsInDomain): Deleted.
+            No longer needed since we generate the top privately controlled
+            domain for both the resource and the partition.
+        (WebCore::NetworkStorageSession::shouldPartitionCookiesForHost): Deleted.
+            Renamed WebCore::NetworkStorageSession::shouldPartitionCookies().
+        * platform/network/mac/CookieJarMac.mm:
+        (WebCore::cookiesInPartitionForURL):
+            Now calls WebCore::NetworkStorageSession::cookieStoragePartition().
+        (WebCore::setCookiesFromDOM):
+            Now calls WebCore::NetworkStorageSession::cookieStoragePartition().
+        * platform/network/mac/ResourceHandleMac.mm:
+        (WebCore::ResourceHandle::createNSURLConnection):
+            Now calls WebCore::NetworkStorageSession::cookieStoragePartition().
+
 2017-03-22  Jer Noble  <jer.no...@apple.com>
 
         Media files served without an extension will not load in <video> tag.

Modified: trunk/Source/WebCore/platform/network/NetworkStorageSession.h (214269 => 214270)


--- trunk/Source/WebCore/platform/network/NetworkStorageSession.h	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/Source/WebCore/platform/network/NetworkStorageSession.h	2017-03-22 19:43:25 UTC (rev 214270)
@@ -79,7 +79,8 @@
     WEBCORE_EXPORT RetainPtr<CFHTTPCookieStorageRef> cookieStorage() const;
     WEBCORE_EXPORT static void setCookieStoragePartitioningEnabled(bool);
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
-    WEBCORE_EXPORT bool shouldPartitionCookiesForHost(const String&);
+    WEBCORE_EXPORT String cookieStoragePartition(const ResourceRequest&) const;
+    String cookieStoragePartition(const URL& firstPartyForCookies, const URL& resource) const;
     WEBCORE_EXPORT void setShouldPartitionCookiesForHosts(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd);
 #endif
 #elif USE(SOUP)
@@ -130,11 +131,9 @@
     CredentialStorage m_credentialStorage;
 
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
+    bool shouldPartitionCookies(const String& topPrivatelyControlledDomain) const;
     HashSet<String> m_topPrivatelyControlledDomainsForCookiePartitioning;
 #endif
 };
 
-WEBCORE_EXPORT String cookieStoragePartition(const ResourceRequest&);
-String cookieStoragePartition(const URL& firstPartyForCookies, const URL& resource);
-
 }

Modified: trunk/Source/WebCore/platform/network/cf/NetworkStorageSessionCFNet.cpp (214269 => 214270)


--- trunk/Source/WebCore/platform/network/cf/NetworkStorageSessionCFNet.cpp	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/Source/WebCore/platform/network/cf/NetworkStorageSessionCFNet.cpp	2017-03-22 19:43:25 UTC (rev 214270)
@@ -108,50 +108,47 @@
     cookieStoragePartitioningEnabled = enabled;
 }
 
-#if PLATFORM(COCOA)
+#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
 
-String cookieStoragePartition(const ResourceRequest& request)
+String NetworkStorageSession::cookieStoragePartition(const ResourceRequest& request) const
 {
     return cookieStoragePartition(request.firstPartyForCookies(), request.url());
 }
 
-static inline bool hostIsInDomain(StringView host, StringView domain)
+static inline String getPartitioningDomain(const URL& url) 
 {
-    if (!host.endsWithIgnoringASCIICase(domain))
-        return false;
-
-    ASSERT(host.length() >= domain.length());
-    unsigned suffixOffset = host.length() - domain.length();
-    return suffixOffset == 0 || host[suffixOffset - 1] == '.';
+#if ENABLE(PUBLIC_SUFFIX_LIST)
+    auto domain = topPrivatelyControlledDomain(url.host());
+    if (domain.isEmpty())
+        domain = url.host();
+#else
+    auto domain = url.host();
+#endif
+    return domain;
 }
 
-String cookieStoragePartition(const URL& firstPartyForCookies, const URL& resource)
+String NetworkStorageSession::cookieStoragePartition(const URL& firstPartyForCookies, const URL& resource) const
 {
     if (!cookieStoragePartitioningEnabled)
         return emptyString();
-
-    String firstPartyDomain = firstPartyForCookies.host();
-#if ENABLE(PUBLIC_SUFFIX_LIST)
-    firstPartyDomain = topPrivatelyControlledDomain(firstPartyDomain);
-#endif
     
-    return hostIsInDomain(resource.host(), firstPartyDomain) ? emptyString() : firstPartyDomain;
-}
+    auto resourceDomain = getPartitioningDomain(resource);
+    if (!shouldPartitionCookies(resourceDomain))
+        return emptyString();
 
-#endif
+    auto firstPartyDomain = getPartitioningDomain(firstPartyForCookies);
+    if (firstPartyDomain == resourceDomain)
+        return emptyString();
 
-#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
+    return firstPartyDomain;
+}
 
-bool NetworkStorageSession::shouldPartitionCookiesForHost(const String& host)
+bool NetworkStorageSession::shouldPartitionCookies(const String& topPrivatelyControlledDomain) const
 {
-    if (host.isEmpty())
+    if (topPrivatelyControlledDomain.isEmpty())
         return false;
 
-    auto domain = topPrivatelyControlledDomain(host);
-    if (domain.isEmpty())
-        domain = host;
-
-    return m_topPrivatelyControlledDomainsForCookiePartitioning.contains(domain);
+    return m_topPrivatelyControlledDomainsForCookiePartitioning.contains(topPrivatelyControlledDomain);
 }
 
 void NetworkStorageSession::setShouldPartitionCookiesForHosts(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd)

Modified: trunk/Source/WebCore/platform/network/mac/CookieJarMac.mm (214269 => 214270)


--- trunk/Source/WebCore/platform/network/mac/CookieJarMac.mm	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/Source/WebCore/platform/network/mac/CookieJarMac.mm	2017-03-22 19:43:25 UTC (rev 214270)
@@ -98,7 +98,7 @@
 
 static NSArray *cookiesInPartitionForURL(const NetworkStorageSession& session, const URL& firstParty, const URL& url)
 {
-    String partition = cookieStoragePartition(firstParty, url);
+    String partition = session.cookieStoragePartition(firstParty, url);
     if (partition.isEmpty())
         return nil;
 
@@ -197,7 +197,7 @@
     ASSERT([filteredCookies.get() count] <= 1);
 
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
-    String partition = cookieStoragePartition(firstParty, url);
+    String partition = session.cookieStoragePartition(firstParty, url);
     if (!partition.isEmpty())
         filteredCookies = applyPartitionToCookies(partition, filteredCookies.get());
 #endif

Modified: trunk/Source/WebCore/platform/network/mac/ResourceHandleMac.mm (214269 => 214270)


--- trunk/Source/WebCore/platform/network/mac/ResourceHandleMac.mm	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/Source/WebCore/platform/network/mac/ResourceHandleMac.mm	2017-03-22 19:43:25 UTC (rev 214270)
@@ -173,7 +173,7 @@
     }
 
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
-    String storagePartition = cookieStoragePartition(firstRequest());
+    String storagePartition = d->m_context->storageSession().cookieStoragePartition(firstRequest());
     if (!storagePartition.isEmpty()) {
         NSMutableURLRequest *mutableRequest = [[nsRequest mutableCopy] autorelease];
         [mutableRequest _setProperty:storagePartition forKey:@"__STORAGE_PARTITION_IDENTIFIER"];

Modified: trunk/Source/WebKit2/ChangeLog (214269 => 214270)


--- trunk/Source/WebKit2/ChangeLog	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/Source/WebKit2/ChangeLog	2017-03-22 19:43:25 UTC (rev 214270)
@@ -1,3 +1,19 @@
+2017-03-22  John Wilander  <wilan...@apple.com>
+
+        Resource Load Statistics: Fix partitioning bug for client-side cookie access
+        https://bugs.webkit.org/show_bug.cgi?id=169906
+        <rdar://problem/31164456>
+
+        Reviewed by Alex Christensen.
+
+        * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
+        (WebKit::NetworkDataTaskCocoa::NetworkDataTaskCocoa):
+            This function no longer asks the NetworkStorageSession
+            whether it should partition or not. That decision is
+            now made as part of
+            WebCore::NetworkStorageSession::cookieStoragePartition().
+            The empty partition signals no partitioning.
+
 2017-03-22  Andy Estes  <aes...@apple.com>
 
         [QuickLook] Rename QuickLookHandle to PreviewLoader

Modified: trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm (214269 => 214270)


--- trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm	2017-03-22 19:32:38 UTC (rev 214269)
+++ trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm	2017-03-22 19:43:25 UTC (rev 214270)
@@ -123,11 +123,10 @@
     LOG(NetworkSession, "%llu Creating NetworkDataTask with URL %s", [m_task taskIdentifier], nsRequest.URL.absoluteString.UTF8String);
 
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
-    if (session.networkStorageSession().shouldPartitionCookiesForHost(url.host())) {
+    String storagePartition = session.networkStorageSession().cookieStoragePartition(request);
+    if (!storagePartition.isEmpty()) {
         LOG(NetworkSession, "%llu Partitioning cookies for URL %s", [m_task taskIdentifier], nsRequest.URL.absoluteString.UTF8String);
-        String storagePartition = WebCore::cookieStoragePartition(request);
-        if (!storagePartition.isEmpty())
-            m_task.get()._storagePartitionIdentifier = storagePartition;
+        m_task.get()._storagePartitionIdentifier = storagePartition;
     }
 #endif
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to