Changeset 262085 in webkit

Timestamp:

May 22, 2020 3:50:50 PM (4 years ago)

Author:

Chris Dumez

Message:

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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/loader/revoke-blob-url-after-navigation-expected.txt (added)
• LayoutTests/fast/loader/revoke-blob-url-after-navigation.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/PolicyChecker.cpp (modified) (4 diffs)
• Source/WebCore/loader/PolicyChecker.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/loader/PolicyChecker.cpp

@@ -105 +105 @@
 }
 
-CompletionHandlerCallingScope FrameLoader::PolicyChecker::extendBlobURLLifetimeIfNecessary(ResourceRequest& request) const
+CompletionHandlerCallingScope FrameLoader::PolicyChecker::extendBlobURLLifetimeIfNecessary(ResourceRequest& request, DocumentLoader* loader) const
 {
     if (!request.url().protocolIsBlob())
@@ -114 +114 @@
     blobRegistry().registerBlobURL(temporaryBlobURL, request.url());
     request.setURL(temporaryBlobURL);
+    if (loader)
+        loader->request().setURL(temporaryBlobURL);
     return CompletionHandler<void()>([temporaryBlobURL = WTFMove(temporaryBlobURL)] {
         blobRegistry().unregisterBlobURL(temporaryBlobURL);
@@ -198 +200 @@
     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();
@@ -256 +258 @@
         return function({ }, nullptr, { }, { }, ShouldContinuePolicyCheck::No);
 
-    auto blobURLLifetimeExtension = extendBlobURLLifetimeIfNecessary(request);
+    auto blobURLLifetimeExtension = extendBlobURLLifetimeIfNecessary(request, nullptr);
 
     auto requestIdentifier = PolicyCheckIdentifier::create();

trunk/Source/WebCore/loader/PolicyChecker.h

@@ -90 +90 @@
 private:
     void handleUnimplementablePolicy(const ResourceError&);
-    WTF::CompletionHandlerCallingScope extendBlobURLLifetimeIfNecessary(ResourceRequest&) const;
+    WTF::CompletionHandlerCallingScope extendBlobURLLifetimeIfNecessary(ResourceRequest&, DocumentLoader*) const;
 
     Frame& m_frame;