Attachment 400076 Details for Bug 212279 – Patch

[patch] Patch

bug-212279-20200522142243.patch (text/plain), 7.59 KB, created by Chris Dumez on 2020-05-22 14:22:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2020-05-22 14:22:44 PDT

Size: 7.59 KB

patch

obsolete

>Subversion Revision: 262068
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ef3b40626aed0d913f90db36569a3b9798b84076..99c8775cb45b03516e6a33ebcfae6ec021026bb8 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2020-05-22  Chris Dumez  <cdumez@apple.com>
>+
>+        Revoking an object URL immediately after triggering navigation causes navigation to fail
>+        https://bugs.webkit.org/show_bug.cgi?id=212279
>+        <rdar://problem/63553090>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When doing a policy check for a Blob URL, we clone the blob and create a new temporary Blob URL
>+        that stays alive for the duration of the policy check. We made sure to update the ResourceRequest
>+        URL with the new Blob URL, however, we were failing to update the DocumentLoader's request.
>+        As a result, if the client responded with Policy USE, the DocumentLoader would still attempt to
>+        navigate to the old Blob URL.
>+
>+        Test: fast/loader/revoke-blob-url-after-navigation.html
>+
>+        * loader/PolicyChecker.cpp:
>+        (WebCore::FrameLoader::PolicyChecker::extendBlobURLLifetimeIfNecessary const):
>+        (WebCore::FrameLoader::PolicyChecker::checkNavigationPolicy):
>+        (WebCore::FrameLoader::PolicyChecker::checkNewWindowPolicy):
>+        * loader/PolicyChecker.h:
>+
> 2020-05-22  Alex Christensen  <achristensen@webkit.org>
> 
>         Add SPI to unblock third party cookies from WKWebViews with ResourceLoadStatistics turned on
>diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp
>index c5c4ea85a464f8d9d332d4476876c2cc934b29c4..276148a63f36cc46ae0cf2bb535c33f2aa07e30b 100644
>--- a/Source/WebCore/loader/PolicyChecker.cpp
>+++ b/Source/WebCore/loader/PolicyChecker.cpp
>@@ -104,7 +104,7 @@ void FrameLoader::PolicyChecker::checkNavigationPolicy(ResourceRequest&& newRequ
>     checkNavigationPolicy(WTFMove(newRequest), redirectResponse, m_frame.loader().activeDocumentLoader(), { }, WTFMove(function));
> }
> 
>-CompletionHandlerCallingScope FrameLoader::PolicyChecker::extendBlobURLLifetimeIfNecessary(ResourceRequest& request) const
>+CompletionHandlerCallingScope FrameLoader::PolicyChecker::extendBlobURLLifetimeIfNecessary(ResourceRequest& request, DocumentLoader* loader) const
> {
>     if (!request.url().protocolIsBlob())
>         return { };
>@@ -113,6 +113,8 @@ CompletionHandlerCallingScope FrameLoader::PolicyChecker::extendBlobURLLifetimeI
>     URL temporaryBlobURL = BlobURL::createPublicURL(&m_frame.document()->securityOrigin());
>     blobRegistry().registerBlobURL(temporaryBlobURL, request.url());
>     request.setURL(temporaryBlobURL);
>+    if (loader)
>+        loader->request().setURL(temporaryBlobURL);
>     return CompletionHandler<void()>([temporaryBlobURL = WTFMove(temporaryBlobURL)] {
>         blobRegistry().unregisterBlobURL(temporaryBlobURL);
>     });
>@@ -197,7 +199,7 @@ void FrameLoader::PolicyChecker::checkNavigationPolicy(ResourceRequest&& request
> 
>     m_frame.loader().clearProvisionalLoadForPolicyCheck();
> 
>-    auto blobURLLifetimeExtension = policyDecisionMode == PolicyDecisionMode::Asynchronous ? extendBlobURLLifetimeIfNecessary(request) : CompletionHandlerCallingScope { };
>+    auto blobURLLifetimeExtension = policyDecisionMode == PolicyDecisionMode::Asynchronous ? extendBlobURLLifetimeIfNecessary(request, loader) : CompletionHandlerCallingScope { };
> 
>     bool isInitialEmptyDocumentLoad = !m_frame.loader().stateMachine().committedFirstRealDocumentLoad() && request.url().protocolIsAbout() && !substituteData.isValid();
>     auto requestIdentifier = PolicyCheckIdentifier::create();
>@@ -255,7 +257,7 @@ void FrameLoader::PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigat
>     if (!DOMWindow::allowPopUp(m_frame))
>         return function({ }, nullptr, { }, { }, ShouldContinuePolicyCheck::No);
> 
>-    auto blobURLLifetimeExtension = extendBlobURLLifetimeIfNecessary(request);
>+    auto blobURLLifetimeExtension = extendBlobURLLifetimeIfNecessary(request, nullptr);
> 
>     auto requestIdentifier = PolicyCheckIdentifier::create();
>     m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState.get(), frameName, requestIdentifier, [frame = makeRef(m_frame), request,
>diff --git a/Source/WebCore/loader/PolicyChecker.h b/Source/WebCore/loader/PolicyChecker.h
>index 3c92cca4f409e1f680909fee64ebe29e7eaf5696..df227b6c103a513ff54b6aba536f964b94bd1206 100644
>--- a/Source/WebCore/loader/PolicyChecker.h
>+++ b/Source/WebCore/loader/PolicyChecker.h
>@@ -89,7 +89,7 @@ public:
> 
> private:
>     void handleUnimplementablePolicy(const ResourceError&);
>-    WTF::CompletionHandlerCallingScope extendBlobURLLifetimeIfNecessary(ResourceRequest&) const;
>+    WTF::CompletionHandlerCallingScope extendBlobURLLifetimeIfNecessary(ResourceRequest&, DocumentLoader*) const;
> 
>     Frame& m_frame;
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index e0a9a991d0b30803a10a258a337aafd1058788a1..497a49247daa503376df32c9d6d140a01aa1f0db 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2020-05-22  Chris Dumez  <cdumez@apple.com>
>+
>+        Revoking an object URL immediately after triggering navigation causes navigation to fail
>+        https://bugs.webkit.org/show_bug.cgi?id=212279
>+        <rdar://problem/63553090>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add layout test coverage.
>+
>+        * fast/loader/revoke-blob-url-after-navigation-expected.txt: Added.
>+        * fast/loader/revoke-blob-url-after-navigation.html: Added.
>+
> 2020-05-22  Carlos Alberto Lopez Perez  <clopez@igalia.com>
> 
>         Gardening after r262056
>diff --git a/LayoutTests/fast/loader/revoke-blob-url-after-navigation-expected.txt b/LayoutTests/fast/loader/revoke-blob-url-after-navigation-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..967e3bad5b7706a0911a37e21c39c99447590da0
>--- /dev/null
>+++ b/LayoutTests/fast/loader/revoke-blob-url-after-navigation-expected.txt
>@@ -0,0 +1,11 @@
>+Tests that navigation to a Blob URL does not fail if the URL is revoked right after triggering the navigation.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS Was able to navigate the frame
>+PASS frame.contentDocument.body.innerText is "TEST"
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/loader/revoke-blob-url-after-navigation.html b/LayoutTests/fast/loader/revoke-blob-url-after-navigation.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..0c361e887f1078b77b9924721d4933b81e049234
>--- /dev/null
>+++ b/LayoutTests/fast/loader/revoke-blob-url-after-navigation.html
>@@ -0,0 +1,36 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script src="../../resources/js-test.js"></script>
>+<script>
>+description("Tests that navigation to a Blob URL does not fail if the URL is revoked right after triggering the navigation.");
>+jsTestIsAsync = true;
>+
>+onload = () => {
>+    let url = window.URL.createObjectURL(new Blob(["TEST", ], { type: 'text/html' }));
>+    let link = document.createElement('a');
>+    link.target = "testFrame";
>+    link.href = url;
>+
>+    frame = document.getElementById("testFrame");
>+    frame.onerror = () => {
>+        testFailed("Failed to navigate the frame");
>+        finishJSTest();
>+    }
>+
>+    frame.onload = () => {
>+        testPassed("Was able to navigate the frame");
>+        shouldBeEqualToString("frame.contentDocument.body.innerText", "TEST");
>+        finishJSTest();
>+    };
>+
>+    link.click();
>+
>+    URL.revokeObjectURL(url);
>+    link.remove();
>+}
>+
>+</script>
>+<iframe id="testFrame" name="testFrame" src="about:blank"></iframe>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 212279: 400076