Ad Click Attribution redirects to well-known location should not trigger a conversion...
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Apr 2019 16:14:05 +0000 (16:14 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Apr 2019 16:14:05 +0000 (16:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197183
<rdar://problem/47763188>

Reviewed by Alex Christensen.

Source/WebKit:

Ad Click Attribution conversions are picked up in the redirect handler
in WebKit::NetworkResourceLoader. Content blocking typically happens in
the continued redirect request handling in the web content process and
a blocked request comes back empty.

We need to call the WebKit::NetworkLoadChecker in the network process
for these specific redirects, just like we do for Ping.

The change makes use of the existing function
NetworkLoadChecker::enableContentExtensionsCheck() for this purpose.

In essence, this change makes it possible to block all conversions made
to a "/.well-known/ad-click-attribution/" URL.

* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::handleAdClickAttributionConversion):
    New convenience function.
(WebKit::NetworkResourceLoader::willSendRedirectedRequest):
    Now calls NetworkLoadChecker::enableContentExtensionsCheck() if
    an Ad Click Attribution conversion was found in the redirect URL.
(WebKit::NetworkResourceLoader::continueWillSendRedirectedRequest):
    If the request was not blocked, it will store any found conversion here.
* NetworkProcess/NetworkResourceLoader.h:

LayoutTests:

* http/tests/contentextensions/block-ad-click-attribution-expected.txt: Added.
* http/tests/contentextensions/block-ad-click-attribution.html: Added.
* http/tests/contentextensions/block-ad-click-attribution.html.json: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/contentextensions/block-ad-click-attribution-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/contentextensions/block-ad-click-attribution.html [new file with mode: 0644]
LayoutTests/http/tests/contentextensions/block-ad-click-attribution.html.json [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
Source/WebKit/NetworkProcess/NetworkResourceLoader.h

index 7bc423a..754ab55 100644 (file)
@@ -1,3 +1,15 @@
+2019-04-23  John Wilander  <wilander@apple.com>
+
+        Ad Click Attribution redirects to well-known location should not trigger a conversion if they are blocked by content blockers
+        https://bugs.webkit.org/show_bug.cgi?id=197183
+        <rdar://problem/47763188>
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/contentextensions/block-ad-click-attribution-expected.txt: Added.
+        * http/tests/contentextensions/block-ad-click-attribution.html: Added.
+        * http/tests/contentextensions/block-ad-click-attribution.html.json: Added.
+
 2019-04-23  Shawn Roberts  <sroberts@apple.com>
 
         fast/selectors/matches-backtracking.html is a flaky timeout
diff --git a/LayoutTests/http/tests/contentextensions/block-ad-click-attribution-expected.txt b/LayoutTests/http/tests/contentextensions/block-ad-click-attribution-expected.txt
new file mode 100644 (file)
index 0000000..27074f7
--- /dev/null
@@ -0,0 +1,17 @@
+CONSOLE MESSAGE: Blocked by content extension
+CONSOLE MESSAGE: Cannot load image https://127.0.0.1:8443/adClickAttribution/resources/redirectToConversion.php?conversionData=12 due to access control checks.
+Tests that ad click attribution redirects to well-known location don't trigger a conversion if they are blocked by content blockers.
+
+
+
+--------
+Frame: '<!--frame1-->'
+--------
+Conversion not received - timed out.
+
+Unconverted Ad Click Attributions:
+WebCore::AdClickAttribution 1
+Source: 127.0.0.1
+Destination: localhost
+Campaign ID: 3
+No conversion data.
diff --git a/LayoutTests/http/tests/contentextensions/block-ad-click-attribution.html b/LayoutTests/http/tests/contentextensions/block-ad-click-attribution.html
new file mode 100644 (file)
index 0000000..a166bad
--- /dev/null
@@ -0,0 +1,72 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AdClickAttributionEnabled=true ] -->
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+    <script src="/js-test-resources/ui-helper.js"></script>
+    <script src="../adClickAttribution/resources/util.js"></script>
+</head>
+<body onload="setTimeout(runTest, 0)">
+<div id="description">Tests that ad click attribution redirects to well-known location don't trigger a conversion if they are blocked by content blockers.</div>
+<a id="targetLink" href="http://localhost:8000/contentextensions/block-ad-click-attribution.html?stepTwo" adcampaignid="3" addestination="http://localhost:8000">Link</a><br>
+<div id="output"></div>
+<script>
+    prepareTest();
+
+    if (window.testRunner) {
+        testRunner.setAdClickAttributionOverrideTimerForTesting(true);
+        testRunner.setAdClickAttributionConversionURLForTesting("http://127.0.0.1:8000/adClickAttribution/resources/conversionReport.php");
+    }
+
+    function activateElement(elementID) {
+        var element = document.getElementById(elementID);
+        var centerX = element.offsetLeft + element.offsetWidth / 2;
+        var centerY = element.offsetTop + element.offsetHeight / 2;
+        UIHelper.activateAt(centerX, centerY).then(
+            function () {
+            },
+            function () {
+                document.getElementById("output").innerText = "FAIL Promise rejected.";
+                tearDownAndFinish();
+            }
+        );
+    }
+
+    function appendIframe(url, onloadCallback) {
+        let iframeElement = document.createElement("iframe");
+        iframeElement.src = url;
+        if (onloadCallback)
+            iframeElement.onload = onloadCallback;
+        document.body.appendChild(iframeElement);
+    }
+
+    function appendConversionDataIframeAndFinish() {
+        testRunner.dumpAdClickAttribution();
+        document.body.removeChild(document.getElementById("targetLink"));
+        document.body.removeChild(document.getElementById("pixel"));
+
+        appendIframe("http://127.0.0.1:8000/adClickAttribution/resources/getConversionData.php?timeout_ms=1000", function() {
+            tearDownAndFinish();
+        });
+    }
+
+    function runTest() {
+        if (window.testRunner) {
+            if (window.location.search === "?stepTwo") {
+                let imageElement = document.createElement("img");
+                imageElement.src = "https://127.0.0.1:8443/adClickAttribution/resources/redirectToConversion.php?conversionData=12";
+                imageElement.id = "pixel";
+                imageElement.onerror = function() {
+                    appendConversionDataIframeAndFinish();
+                };
+                document.body.appendChild(imageElement);
+            } else {
+                activateElement("targetLink");
+            }
+        } else {
+            document.getElementById("output").innerText = "FAIL No testRunner.";
+        }
+    }
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/contentextensions/block-ad-click-attribution.html.json b/LayoutTests/http/tests/contentextensions/block-ad-click-attribution.html.json
new file mode 100644 (file)
index 0000000..5e394ef
--- /dev/null
@@ -0,0 +1,10 @@
+[
+  {
+    "trigger": {
+      "url-filter": "/.well-known/ad-click-attribution/"
+    },
+    "action": {
+      "type": "block"
+    }
+  }
+]
index 1c863a2..c4a0f56 100644 (file)
@@ -1,3 +1,35 @@
+2019-04-23  John Wilander  <wilander@apple.com>
+
+        Ad Click Attribution redirects to well-known location should not trigger a conversion if they are blocked by content blockers
+        https://bugs.webkit.org/show_bug.cgi?id=197183
+        <rdar://problem/47763188>
+
+        Reviewed by Alex Christensen.
+
+        Ad Click Attribution conversions are picked up in the redirect handler
+        in WebKit::NetworkResourceLoader. Content blocking typically happens in
+        the continued redirect request handling in the web content process and
+        a blocked request comes back empty.
+
+        We need to call the WebKit::NetworkLoadChecker in the network process
+        for these specific redirects, just like we do for Ping.
+
+        The change makes use of the existing function
+        NetworkLoadChecker::enableContentExtensionsCheck() for this purpose.
+
+        In essence, this change makes it possible to block all conversions made
+        to a "/.well-known/ad-click-attribution/" URL.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::handleAdClickAttributionConversion):
+            New convenience function.
+        (WebKit::NetworkResourceLoader::willSendRedirectedRequest):
+            Now calls NetworkLoadChecker::enableContentExtensionsCheck() if
+            an Ad Click Attribution conversion was found in the redirect URL.
+        (WebKit::NetworkResourceLoader::continueWillSendRedirectedRequest):
+            If the request was not blocked, it will store any found conversion here.
+        * NetworkProcess/NetworkResourceLoader.h:
+
 2019-04-23  Don Olmstead  <don.olmstead@sony.com>
 
         [CMake][Win] Use target oriented design for WebKit
index f9d0f5e..e30d494 100644 (file)
@@ -42,7 +42,6 @@
 #include "WebPageMessages.h"
 #include "WebResourceLoaderMessages.h"
 #include "WebsiteDataStoreParameters.h"
-#include <WebCore/AdClickAttribution.h>
 #include <WebCore/BlobDataFileReference.h>
 #include <WebCore/CertificateInfo.h>
 #include <WebCore/ContentSecurityPolicy.h>
@@ -581,29 +580,35 @@ Optional<Seconds> NetworkResourceLoader::validateCacheEntryForMaxAgeCapValidatio
     return WTF::nullopt;
 }
 
+void NetworkResourceLoader::handleAdClickAttributionConversion(AdClickAttribution::Conversion&& conversion, const URL& requestURL, const WebCore::ResourceRequest& redirectRequest)
+{
+    ASSERT(!sessionID().isEphemeral());
+
+    RegistrableDomain redirectDomain { redirectRequest.url() };
+    auto& firstPartyURL = redirectRequest.firstPartyForCookies();
+    NetworkSession* networkSession = nullptr;
+    // The redirect has to be done by the same registrable domain and it has to be a third-party request.
+    if (redirectDomain.matches(requestURL) && !redirectDomain.matches(firstPartyURL) && (networkSession = m_connection->networkProcess().networkSession(sessionID())))
+        networkSession->convertAdClickAttribution(AdClickAttribution::Source { WTFMove(redirectDomain) }, AdClickAttribution::Destination { firstPartyURL }, WTFMove(conversion));
+}
+
 void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse)
 {
     ++m_redirectCount;
 
-    auto& redirectURL = redirectRequest.url();
-    if (!sessionID().isEphemeral()) {
-        if (auto adClickConversion = AdClickAttribution::parseConversionRequest(redirectURL)) {
-            RegistrableDomain redirectDomain { redirectURL };
-            auto& firstPartyURL = redirectRequest.firstPartyForCookies();
-            NetworkSession* networkSession = nullptr;
-            // The redirect has to be done by the same registrable domain and it has to be a third-party request.
-            if (redirectDomain.matches(request.url()) && !redirectDomain.matches(firstPartyURL) && (networkSession = m_connection->networkProcess().networkSession(sessionID())))
-                networkSession->convertAdClickAttribution(AdClickAttribution::Source { WTFMove(redirectDomain) }, AdClickAttribution::Destination { firstPartyURL }, WTFMove(*adClickConversion));
-        }
-    }
+    Optional<AdClickAttribution::Conversion> adClickConversion;
+    if (!sessionID().isEphemeral())
+        adClickConversion = AdClickAttribution::parseConversionRequest(redirectRequest.url());
 
     auto maxAgeCap = validateCacheEntryForMaxAgeCapValidation(request, redirectRequest, redirectResponse);
     if (redirectResponse.source() == ResourceResponse::Source::Network && canUseCachedRedirect(request))
         m_cache->storeRedirect(request, redirectResponse, redirectRequest, maxAgeCap);
 
     if (m_networkLoadChecker) {
+        if (adClickConversion)
+            m_networkLoadChecker->enableContentExtensionsCheck();
         m_networkLoadChecker->storeRedirectionIfNeeded(request, redirectResponse);
-        m_networkLoadChecker->checkRedirection(WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse), this, [protectedThis = makeRef(*this), this, storedCredentialsPolicy = m_networkLoadChecker->storedCredentialsPolicy()](auto&& result) mutable {
+        m_networkLoadChecker->checkRedirection(WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse), this, [protectedThis = makeRef(*this), this, storedCredentialsPolicy = m_networkLoadChecker->storedCredentialsPolicy(), adClickConversion = WTFMove(adClickConversion)](auto&& result) mutable {
             if (!result.has_value()) {
                 if (result.error().isCancellation())
                     return;
@@ -631,17 +636,19 @@ void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request,
             }
 
             m_shouldRestartLoad = storedCredentialsPolicy != m_networkLoadChecker->storedCredentialsPolicy();
-            this->continueWillSendRedirectedRequest(WTFMove(result->request), WTFMove(result->redirectRequest), WTFMove(result->redirectResponse));
+            this->continueWillSendRedirectedRequest(WTFMove(result->request), WTFMove(result->redirectRequest), WTFMove(result->redirectResponse), WTFMove(adClickConversion));
         });
         return;
     }
-    continueWillSendRedirectedRequest(WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse));
+    continueWillSendRedirectedRequest(WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse), WTFMove(adClickConversion));
 }
 
-void NetworkResourceLoader::continueWillSendRedirectedRequest(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse)
+void NetworkResourceLoader::continueWillSendRedirectedRequest(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse, Optional<AdClickAttribution::Conversion>&& adClickConversion)
 {
     ASSERT(!isSynchronous());
 
+    if (adClickConversion)
+        handleAdClickAttributionConversion(WTFMove(*adClickConversion), request.url(), redirectRequest);
     send(Messages::WebResourceLoader::WillSendRequest(redirectRequest, sanitizeResponseIfPossible(WTFMove(redirectResponse), ResourceResponse::SanitizationType::Redirection)));
 }
 
index 5bee9e2..0298511 100644 (file)
@@ -31,6 +31,7 @@
 #include "NetworkConnectionToWebProcessMessages.h"
 #include "NetworkLoadClient.h"
 #include "NetworkResourceLoadParameters.h"
+#include <WebCore/AdClickAttribution.h>
 #include <WebCore/ContentSecurityPolicyClient.h>
 #include <WebCore/ResourceResponse.h>
 #include <WebCore/SecurityPolicyViolationEvent.h>
@@ -159,7 +160,7 @@ private:
     void logCookieInformation() const;
 #endif
 
-    void continueWillSendRedirectedRequest(WebCore::ResourceRequest&&, WebCore::ResourceRequest&& redirectRequest, WebCore::ResourceResponse&&);
+    void continueWillSendRedirectedRequest(WebCore::ResourceRequest&&, WebCore::ResourceRequest&& redirectRequest, WebCore::ResourceResponse&&, Optional<WebCore::AdClickAttribution::Conversion>&&);
     void didFinishWithRedirectResponse(WebCore::ResourceResponse&&);
     WebCore::ResourceResponse sanitizeResponseIfPossible(WebCore::ResourceResponse&&, WebCore::ResourceResponse::SanitizationType);
 
@@ -170,6 +171,8 @@ private:
 
     void logSlowCacheRetrieveIfNeeded(const NetworkCache::Cache::RetrieveInfo&);
 
+    void handleAdClickAttributionConversion(WebCore::AdClickAttribution::Conversion&&, const URL&, const WebCore::ResourceRequest&);
+
     Optional<Seconds> validateCacheEntryForMaxAgeCapValidation(const WebCore::ResourceRequest&, const WebCore::ResourceRequest& redirectRequest, const WebCore::ResourceResponse&);
 
     const NetworkResourceLoadParameters m_parameters;