Revoking an object URL immediately after triggering navigation causes navigation...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 22:50:50 +0000 (22:50 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 22:50:50 +0000 (22:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212279
<rdar://problem/63553090>

Reviewed by Geoffrey Garen.

Source/WebCore:

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:

LayoutTests:

Add layout test coverage.

* fast/loader/revoke-blob-url-after-navigation-expected.txt: Added.
* fast/loader/revoke-blob-url-after-navigation.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/loader/revoke-blob-url-after-navigation-expected.txt [new file with mode: 0644]
LayoutTests/fast/loader/revoke-blob-url-after-navigation.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/PolicyChecker.cpp
Source/WebCore/loader/PolicyChecker.h

index 9f584a8..8d12e61 100644 (file)
@@ -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 Geoffrey Garen.
+
+        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  Jason Lawrence  <lawrence.j@apple.com>
 
         [ iOS wk2 Release ] editing/async-clipboard/clipboard-item-get-type-basic.html is flaky failing.
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 (file)
index 0000000..967e3ba
--- /dev/null
@@ -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 (file)
index 0000000..0c361e8
--- /dev/null
@@ -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>
index a59c9c9..8f04a82 100644 (file)
@@ -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 Geoffrey Garen.
+
+        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  Simon Fraser  <simon.fraser@apple.com>
 
         Make sure we clean up CFTimerRefs when destroying scrolling tree nodes
index c5c4ea8..276148a 100644 (file)
@@ -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,
index 3c92cca..df227b6 100644 (file)
@@ -89,7 +89,7 @@ public:
 
 private:
     void handleUnimplementablePolicy(const ResourceError&);
-    WTF::CompletionHandlerCallingScope extendBlobURLLifetimeIfNecessary(ResourceRequest&) const;
+    WTF::CompletionHandlerCallingScope extendBlobURLLifetimeIfNecessary(ResourceRequest&, DocumentLoader*) const;
 
     Frame& m_frame;