Check if ITP is on before applying third-party cookie blocking
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Nov 2019 06:30:46 +0000 (06:30 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Nov 2019 06:30:46 +0000 (06:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204322
<rdar://problem/57120772>

Reviewed by Chris Dumez and Alexey Proskuryakov.

Source/WebCore:

This change makes sure WebCore::NetworkStorageSession knows
whether ITP is on or off and checks that first thing in
WebCore::NetworkStorageSession::shouldBlockCookies().

This check was never needed before since if ITP was off,
there would be no classified domains and thus the function
would always return false. However,
https://trac.webkit.org/changeset/251353/webkit introduced
full third-party cookie blocking for websites without user
interaction and that rule is checked before checking domain
classification. The effect was unconditional third-party
cookie blocking if ITP is off. This changes fixes that bug.

Note that this patch already landed as branch-specific in
https://trac.webkit.org/changeset/252549/webkit

Test: http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off.html

* platform/network/NetworkStorageSession.cpp:
(WebCore::NetworkStorageSession::shouldBlockThirdPartyCookies const):
(WebCore::NetworkStorageSession::shouldBlockThirdPartyCookiesButKeepFirstPartyCookiesFor const):
(WebCore::NetworkStorageSession::shouldBlockCookies const):
    Now checks whether ITP is on or off.
* platform/network/NetworkStorageSession.h:
(WebCore::NetworkStorageSession::setResourceLoadStatisticsEnabled):

Source/WebKit:

This change makes sure WebCore::NetworkStorageSession knows
whether ITP is on or off and checks that first thing in
WebCore::NetworkStorageSession::shouldBlockCookies().

This check was never needed before since if ITP was off,
there would be no classified domains and thus the function
would always return false. However,
https://trac.webkit.org/changeset/251353/webkit introduced
full third-party cookie blocking for websites without user
interaction and that rule is checked before checking domain
classification. The effect was unconditional third-party
cookie blocking if ITP is off. This changes fixes that bug.

Note that this patch already landed as branch-specific in
https://trac.webkit.org/changeset/252549/webkit

* NetworkProcess/NetworkSession.cpp:
(WebKit::NetworkSession::setResourceLoadStatisticsEnabled):
    Now tells WebCore::NetworkStorageSession the status of
    ITP.

Tools:

This is test infrastructure to allow a layout test to
disable ITP in the network process.

* WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::setStatisticsEnabled):
(WTR::TestRunner::setStatisticsDebugMode):
* WebKitTestRunner/InjectedBundle/TestRunner.h:
* WebKitTestRunner/TestController.cpp:
(WTR::TestController::setStatisticsEnabled):
* WebKitTestRunner/TestController.h:
* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):

LayoutTests:

* http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off-expected.txt: Added.
* http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off.html: Added.

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/network/NetworkStorageSession.cpp
Source/WebCore/platform/network/NetworkStorageSession.h
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkSession.cpp
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
Tools/WebKitTestRunner/TestController.cpp
Tools/WebKitTestRunner/TestController.h
Tools/WebKitTestRunner/TestInvocation.cpp

index 6699925..ce6be1c 100644 (file)
@@ -1,3 +1,14 @@
+2019-11-18  John Wilander  <wilander@apple.com>
+
+        Check if ITP is on before applying third-party cookie blocking
+        https://bugs.webkit.org/show_bug.cgi?id=204322
+        <rdar://problem/57120772>
+
+        Reviewed by Chris Dumez and Alexey Proskuryakov.
+
+        * http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off-expected.txt: Added.
+        * http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off.html: Added.
+
 2019-11-18  Simon Fraser  <simon.fraser@apple.com>
 
         -webkit-font-smoothing: none leaves subsequent elements unantialiased
diff --git a/LayoutTests/http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off-expected.txt b/LayoutTests/http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off-expected.txt
new file mode 100644 (file)
index 0000000..70532d7
--- /dev/null
@@ -0,0 +1,16 @@
+Tests that existing third-party cookies are not blocked when ITP is off.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
+--------
+Frame: '<!--frame1-->'
+--------
+Should receive its cookie.
+Received cookie named 'firstPartyCookie'.
+Client-side document.cookie: firstPartyCookie=value
diff --git a/LayoutTests/http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off.html b/LayoutTests/http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off.html
new file mode 100644 (file)
index 0000000..cdf78fb
--- /dev/null
@@ -0,0 +1,60 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <script src="/js-test-resources/js-test.js"></script>
+    <script src="resources/util.js"></script>
+</head>
+<body onload="runTest()">
+<script>
+    description("Tests that existing third-party cookies are not blocked when ITP is off.");
+    jsTestIsAsync = true;
+
+    const partitionHost = "127.0.0.1:8000";
+    const partitionOrigin = "http://" + partitionHost;
+    const thirdPartyOrigin = "http://localhost:8000";
+    const resourcePath = "/resourceLoadStatistics/resources";
+    const thirdPartyBaseUrl = thirdPartyOrigin + resourcePath;
+    const firstPartyCookieName = "firstPartyCookie";
+    const subPathToSetFirstPartyCookie = "/set-cookie.php?name=" + firstPartyCookieName + "&value=value";
+    const returnUrl = partitionOrigin + "/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off.html";
+    const subPathToGetCookies = "/get-cookies.php?name1=" + firstPartyCookieName;
+
+    function openIframe(url, onLoadHandler) {
+        const element = document.createElement("iframe");
+        if (onLoadHandler) {
+            element.onload = onLoadHandler;
+        }
+        element.src = url;
+        document.body.appendChild(element);
+    }
+
+    function runTest() {
+        switch (document.location.hash) {
+            case "":
+                document.location.hash = "step2";
+                testRunner.dumpChildFramesAsText();
+                // Enable the flag even though ITP will be turned off.
+                testRunner.setStatisticsShouldBlockThirdPartyCookies(true, function () {
+                    testRunner.setStatisticsEnabled(false);
+                    runTest();
+                });
+                break;
+            case "#step2":
+                // Set first-party cookie for localhost.
+                document.location.href = thirdPartyBaseUrl + subPathToSetFirstPartyCookie + "#" + returnUrl + "#step3";
+                break;
+            case "#step3":
+                // Check that the cookie is not blocked for localhost under 127.0.0.1.
+                openIframe(thirdPartyBaseUrl + subPathToGetCookies + "&message=Should receive its cookie.", function() {
+                    testRunner.setStatisticsEnabled(true);
+                    testRunner.setStatisticsShouldBlockThirdPartyCookies(false, function() {
+                        finishJSTest();
+                    });
+                });
+                break;
+        }
+    }
+</script>
+</body>
+</html>
\ No newline at end of file
index 60fa6a3..8dff815 100644 (file)
@@ -1,3 +1,37 @@
+2019-11-18  John Wilander  <wilander@apple.com>
+
+        Check if ITP is on before applying third-party cookie blocking
+        https://bugs.webkit.org/show_bug.cgi?id=204322
+        <rdar://problem/57120772>
+
+        Reviewed by Chris Dumez and Alexey Proskuryakov.
+
+        This change makes sure WebCore::NetworkStorageSession knows
+        whether ITP is on or off and checks that first thing in
+        WebCore::NetworkStorageSession::shouldBlockCookies().
+
+        This check was never needed before since if ITP was off,
+        there would be no classified domains and thus the function
+        would always return false. However,
+        https://trac.webkit.org/changeset/251353/webkit introduced
+        full third-party cookie blocking for websites without user
+        interaction and that rule is checked before checking domain
+        classification. The effect was unconditional third-party
+        cookie blocking if ITP is off. This changes fixes that bug.
+
+        Note that this patch already landed as branch-specific in
+        https://trac.webkit.org/changeset/252549/webkit
+
+        Test: http/tests/resourceLoadStatistics/no-third-party-cookie-blocking-when-itp-is-off.html
+
+        * platform/network/NetworkStorageSession.cpp:
+        (WebCore::NetworkStorageSession::shouldBlockThirdPartyCookies const):
+        (WebCore::NetworkStorageSession::shouldBlockThirdPartyCookiesButKeepFirstPartyCookiesFor const):
+        (WebCore::NetworkStorageSession::shouldBlockCookies const):
+            Now checks whether ITP is on or off.
+        * platform/network/NetworkStorageSession.h:
+        (WebCore::NetworkStorageSession::setResourceLoadStatisticsEnabled):
+
 2019-11-18  Simon Fraser  <simon.fraser@apple.com>
 
         -webkit-font-smoothing: none leaves subsequent elements unantialiased
index 089bf69..369d119 100644 (file)
@@ -59,7 +59,7 @@ void NetworkStorageSession::permitProcessToUseCookieAPI(bool value)
 
 bool NetworkStorageSession::shouldBlockThirdPartyCookies(const RegistrableDomain& registrableDomain) const
 {
-    if (registrableDomain.isEmpty())
+    if (!m_isResourceLoadStatisticsEnabled || registrableDomain.isEmpty())
         return false;
 
     ASSERT(!(m_registrableDomainsToBlockAndDeleteCookiesFor.contains(registrableDomain) && m_registrableDomainsToBlockButKeepCookiesFor.contains(registrableDomain)));
@@ -70,7 +70,7 @@ bool NetworkStorageSession::shouldBlockThirdPartyCookies(const RegistrableDomain
 
 bool NetworkStorageSession::shouldBlockThirdPartyCookiesButKeepFirstPartyCookiesFor(const RegistrableDomain& registrableDomain) const
 {
-    if (registrableDomain.isEmpty())
+    if (!m_isResourceLoadStatisticsEnabled || registrableDomain.isEmpty())
         return false;
 
     ASSERT(!(m_registrableDomainsToBlockAndDeleteCookiesFor.contains(registrableDomain) && m_registrableDomainsToBlockButKeepCookiesFor.contains(registrableDomain)));
@@ -88,11 +88,17 @@ bool NetworkStorageSession::hasHadUserInteractionAsFirstParty(const RegistrableD
 
 bool NetworkStorageSession::shouldBlockCookies(const ResourceRequest& request, Optional<FrameIdentifier> frameID, Optional<PageIdentifier> pageID) const
 {
+    if (!m_isResourceLoadStatisticsEnabled)
+        return false;
+
     return shouldBlockCookies(request.firstPartyForCookies(), request.url(), frameID, pageID);
 }
     
 bool NetworkStorageSession::shouldBlockCookies(const URL& firstPartyForCookies, const URL& resource, Optional<FrameIdentifier> frameID, Optional<PageIdentifier> pageID) const
 {
+    if (!m_isResourceLoadStatisticsEnabled)
+        return false;
+
     RegistrableDomain firstPartyDomain { firstPartyForCookies };
     if (firstPartyDomain.isEmpty())
         return false;
index 6699b5d..17410ba 100644 (file)
@@ -143,6 +143,7 @@ public:
     WEBCORE_EXPORT std::pair<String, bool> cookieRequestHeaderFieldValue(const CookieRequestHeaderFieldProxy&) const;
 
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
+    void setResourceLoadStatisticsEnabled(bool enabled) { m_isResourceLoadStatisticsEnabled = enabled; }
     WEBCORE_EXPORT bool shouldBlockCookies(const ResourceRequest&, Optional<FrameIdentifier>, Optional<PageIdentifier>) const;
     WEBCORE_EXPORT bool shouldBlockCookies(const URL& firstPartyForCookies, const URL& resource, Optional<FrameIdentifier>, Optional<PageIdentifier>) const;
     WEBCORE_EXPORT bool shouldBlockThirdPartyCookies(const RegistrableDomain&) const;
@@ -188,6 +189,7 @@ private:
     CredentialStorage m_credentialStorage;
 
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
+    bool m_isResourceLoadStatisticsEnabled = false;
     Optional<Seconds> clientSideCookieCap(const RegistrableDomain& firstParty, Optional<PageIdentifier>) const;
     HashSet<RegistrableDomain> m_registrableDomainsToBlockAndDeleteCookiesFor;
     HashSet<RegistrableDomain> m_registrableDomainsToBlockButKeepCookiesFor;
index 5ac2c6f..a54cffc 100644 (file)
@@ -1,3 +1,32 @@
+2019-11-18  John Wilander  <wilander@apple.com>
+
+        Check if ITP is on before applying third-party cookie blocking
+        https://bugs.webkit.org/show_bug.cgi?id=204322
+        <rdar://problem/57120772>
+
+        Reviewed by Chris Dumez and Alexey Proskuryakov.
+
+        This change makes sure WebCore::NetworkStorageSession knows
+        whether ITP is on or off and checks that first thing in
+        WebCore::NetworkStorageSession::shouldBlockCookies().
+
+        This check was never needed before since if ITP was off,
+        there would be no classified domains and thus the function
+        would always return false. However,
+        https://trac.webkit.org/changeset/251353/webkit introduced
+        full third-party cookie blocking for websites without user
+        interaction and that rule is checked before checking domain
+        classification. The effect was unconditional third-party
+        cookie blocking if ITP is off. This changes fixes that bug.
+
+        Note that this patch already landed as branch-specific in
+        https://trac.webkit.org/changeset/252549/webkit
+
+        * NetworkProcess/NetworkSession.cpp:
+        (WebKit::NetworkSession::setResourceLoadStatisticsEnabled):
+            Now tells WebCore::NetworkStorageSession the status of
+            ITP.
+
 2019-11-18  David Kilzer  <ddkilzer@apple.com>
 
         IPC::Decoder should use nullptr as invalid value
index b5c5950..bd21a3c 100644 (file)
@@ -153,6 +153,8 @@ void NetworkSession::invalidateAndCancel()
 void NetworkSession::setResourceLoadStatisticsEnabled(bool enable)
 {
     ASSERT(!m_isInvalidated);
+    if (auto* storageSession = networkStorageSession())
+        storageSession->setResourceLoadStatisticsEnabled(enable);
     if (!enable) {
         destroyResourceLoadStatistics();
         return;
index dcf55fc..8754501 100644 (file)
@@ -1,3 +1,25 @@
+2019-11-18  John Wilander  <wilander@apple.com>
+
+        Check if ITP is on before applying third-party cookie blocking
+        https://bugs.webkit.org/show_bug.cgi?id=204322
+        <rdar://problem/57120772>
+
+        Reviewed by Chris Dumez and Alexey Proskuryakov.
+
+        This is test infrastructure to allow a layout test to
+        disable ITP in the network process.
+
+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::setStatisticsEnabled):
+        (WTR::TestRunner::setStatisticsDebugMode):
+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::setStatisticsEnabled):
+        * WebKitTestRunner/TestController.h:
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
+
 2019-11-18  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         [Win] kill-old-processes should kill WebKitWebProcess.exe and WebKitNetworkProcess.exe
index faef0a0..b2ec8c6 100644 (file)
@@ -292,6 +292,7 @@ interface TestRunner {
     void disconnectMockGamepad(unsigned long index);
 
     // Resource Load Statistics
+    void setStatisticsEnabled(boolean value);
     void installStatisticsDidModifyDataRecordsCallback(object callback);
     void installStatisticsDidScanDataRecordsCallback(object callback);
     void installStatisticsDidRunTelemetryCallback(object callback);
index ed1933f..3025b9a 100644 (file)
@@ -1408,6 +1408,13 @@ void TestRunner::callDidRemoveSwipeSnapshotCallback()
     callTestRunnerCallback(DidRemoveSwipeSnapshotCallbackID);
 }
 
+void TestRunner::setStatisticsEnabled(bool value)
+{
+    WKRetainPtr<WKStringRef> messageName = adoptWK(WKStringCreateWithUTF8CString("SetStatisticsEnabled"));
+    WKRetainPtr<WKBooleanRef> messageBody = adoptWK(WKBooleanCreate(value));
+    WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), messageBody.get(), nullptr);
+}
+
 void TestRunner::setStatisticsDebugMode(bool value, JSValueRef completionHandler)
 {
     cacheTestRunnerCallback(SetStatisticsDebugModeCallbackID, completionHandler);
@@ -1415,7 +1422,6 @@ void TestRunner::setStatisticsDebugMode(bool value, JSValueRef completionHandler
     WKRetainPtr<WKStringRef> messageName = adoptWK(WKStringCreateWithUTF8CString("SetStatisticsDebugMode"));
     WKRetainPtr<WKBooleanRef> messageBody = adoptWK(WKBooleanCreate(value));
     WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), messageBody.get(), nullptr);
-
 }
 
 void TestRunner::statisticsCallDidSetDebugModeCallback()
index cd9b71f..642c59d 100644 (file)
@@ -380,6 +380,7 @@ public:
     void setMockGamepadButtonValue(unsigned index, unsigned buttonIndex, double value);
     
     // Resource Load Statistics
+    void setStatisticsEnabled(bool value);
     void installStatisticsDidModifyDataRecordsCallback(JSValueRef callback);
     void installStatisticsDidScanDataRecordsCallback(JSValueRef callback);
     void installStatisticsDidRunTelemetryCallback(JSValueRef callback);
index 8a07325..3a07c13 100644 (file)
@@ -3254,6 +3254,11 @@ static void resourceStatisticsBooleanResultCallback(bool result, void* userData)
     context->testController.notifyDone();
 }
 
+void TestController::setStatisticsEnabled(bool value)
+{
+    WKWebsiteDataStoreSetResourceLoadStatisticsEnabled(TestController::websiteDataStore(), value);
+}
+
 void TestController::setStatisticsDebugMode(bool value)
 {
     ResourceStatisticsCallbackContext context(*this);
index ed169ea..1b261bc 100644 (file)
@@ -207,6 +207,7 @@ public:
     void setShouldDownloadUndisplayableMIMETypes(bool value) { m_shouldDownloadUndisplayableMIMETypes = value; }
     void setShouldAllowDeviceOrientationAndMotionAccess(bool value) { m_shouldAllowDeviceOrientationAndMotionAccess = value; }
 
+    void setStatisticsEnabled(bool value);
     void setStatisticsDebugMode(bool value);
     void setStatisticsPrevalentResourceForDebugMode(WKStringRef hostName);
     void setStatisticsLastSeen(WKStringRef hostName, double seconds);
index 87ae56b..5bbeb57 100644 (file)
@@ -1071,6 +1071,13 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB
         return result;
     }
 
+    if (WKStringIsEqualToUTF8CString(messageName, "SetStatisticsEnabled")) {
+        ASSERT(WKGetTypeID(messageBody) == WKBooleanGetTypeID());
+        WKBooleanRef value = static_cast<WKBooleanRef>(messageBody);
+        TestController::singleton().setStatisticsEnabled(WKBooleanGetValue(value));
+        return nullptr;
+    }
+
     if (WKStringIsEqualToUTF8CString(messageName, "SetStatisticsDebugMode")) {
         ASSERT(WKGetTypeID(messageBody) == WKBooleanGetTypeID());
         WKBooleanRef value = static_cast<WKBooleanRef>(messageBody);