Attachment 340148 Details for Bug 185531 – Patch

[patch] Patch

bug-185531-20180510170255.patch (text/plain), 6.53 KB, created by Chris Dumez on 2018-05-10 17:02:56 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-05-10 17:02:56 PDT

Size: 6.53 KB

patch

obsolete

>Subversion Revision: 231654
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 8c7ef67f825183403d835e517da46ea83a49b4d0..34f71f24b2aac46de4a10fbd3647f9d91642bf29 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-05-10  Chris Dumez  <cdumez@apple.com>
>+
>+        REGRESSION (async policy delegate): Revoking an object URL immediately after triggering download breaks file download
>+        https://bugs.webkit.org/show_bug.cgi?id=185531
>+        <rdar://problem/39909589>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Whenever we start an asynchronous navigation policy decision for a blob URL, create a temporary
>+        blob URL pointing to the same data, and update the request URL. This way, if the page's JS revokes
>+        the URL during the policy decision, the load will still succeed.
>+
>+        Test: fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html
>+
>+        * loader/PolicyChecker.cpp:
>+        (WebCore::PolicyChecker::checkNavigationPolicy):
>+
> 2018-05-10  Chris Dumez  <cdumez@apple.com>
> 
>         'Cross-Origin-Options header implementation follow-up
>diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp
>index fc2419498384d22bdf6d7a7d926ec53c6b04ad58..a20c34c821f0c02ce35f8aff5cb28dbb65fcf3da 100644
>--- a/Source/WebCore/loader/PolicyChecker.cpp
>+++ b/Source/WebCore/loader/PolicyChecker.cpp
>@@ -31,6 +31,8 @@
> #include "config.h"
> #include "PolicyChecker.h"
> 
>+#include "BlobRegistry.h"
>+#include "BlobURL.h"
> #include "ContentFilter.h"
> #include "ContentSecurityPolicy.h"
> #include "DOMWindow.h"
>@@ -44,6 +46,7 @@
> #include "HTMLFormElement.h"
> #include "HTMLFrameOwnerElement.h"
> #include "HTMLPlugInElement.h"
>+#include <wtf/CallbackAggregator.h>
> #include <wtf/CompletionHandler.h>
> 
> #if USE(QUICK_LOOK)
>@@ -147,10 +150,21 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
> 
>     m_frame.loader().clearProvisionalLoadForPolicyCheck();
> 
>+    RefPtr<CallbackAggregator> blobURLLifetimeExtension;
>+    if (request.url().protocolIsBlob() && policyDecisionMode == PolicyDecisionMode::Asynchronous) {
>+        // Create a new temporary blobURL in case this one gets revoked during the asynchronous navigation policy decision.
>+        URL temporaryBlobURL = BlobURL::createPublicURL(&m_frame.document()->securityOrigin());
>+        blobRegistry().registerBlobURL(temporaryBlobURL, request.url());
>+        request.setURL(temporaryBlobURL);
>+        blobURLLifetimeExtension = CallbackAggregator::create([temporaryBlobURL] {
>+            blobRegistry().unregisterBlobURL(temporaryBlobURL);
>+        });
>+    }
>+
>     m_delegateIsDecidingNavigationPolicy = true;
>     String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
>     ResourceRequest requestCopy = request;
>-    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, policyDecisionMode, [this, function = WTFMove(function), request = WTFMove(requestCopy), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename)](PolicyAction policyAction) mutable {
>+    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, policyDecisionMode, [this, function = WTFMove(function), request = WTFMove(requestCopy), formState = makeRefPtr(formState), suggestedFilename = WTFMove(suggestedFilename), blobURLLifetimeExtension](PolicyAction policyAction) mutable {
>         m_delegateIsDecidingNavigationPolicy = false;
> 
>         switch (policyAction) {
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index dd871f09cc6f5c49066135edcd7fdf9a65ee7eaa..6287122dc8c39d9cc8fb69a8560546ee5a8145dc 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-05-10  Chris Dumez  <cdumez@apple.com>
>+
>+        REGRESSION (async policy delegate): Revoking an object URL immediately after triggering download breaks file download
>+        https://bugs.webkit.org/show_bug.cgi?id=185531
>+        <rdar://problem/39909589>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add layout test coverage.
>+
>+        * fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt: Added.
>+        * fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html: Added.
>+
> 2018-05-10  Chris Dumez  <cdumez@apple.com>
> 
>         'Cross-Origin-Options header implementation follow-up
>diff --git a/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..d2b6224077a4f5ef1b6b69632c4bc245e9aef742
>--- /dev/null
>+++ b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke-expected.txt
>@@ -0,0 +1,7 @@
>+CONSOLE MESSAGE: line 37: PASS: URL was revoked
>+Download started.
>+Downloading URL with suggested filename "foo.txt"
>+Download completed.
>+The suggested filename above should be "foo.txt" and the download should succeed.
>+
>+Memory backed blob URL
>diff --git a/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..70723c4b16e3e2eb2998e19421654e20abf87b57
>--- /dev/null
>+++ b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-file-blob-download-then-revoke.html
>@@ -0,0 +1,42 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script>
>+if (window.testRunner) {
>+  testRunner.dumpAsText();
>+  testRunner.setShouldLogDownloadCallbacks(true);
>+  testRunner.waitUntilDownloadFinished();
>+}
>+</script>
>+</head>
>+<body>
>+<p>The suggested filename above should be "foo.txt" and the download should succeed.</p>
>+<a id="blob-url" download="foo">Memory backed blob URL</a>
>+<script>
>+function runTest()
>+{
>+    file = new File(["foo"], "foo.txt", {
>+        type: "text/plain"
>+    });
>+    var link = document.getElementById("blob-url");
>+    link.href = window.URL.createObjectURL(file);
>+    link.click();
>+    // Revoke the URL right away.
>+    window.URL.revokeObjectURL(link.href);
>+
>+    // Make sure the URL was revoked.
>+    xhr = new XMLHttpRequest();
>+    xhr.open("GET", link.href, false);
>+    try {
>+        xhr.send(null);
>+    } catch (e) {
>+    }
>+    if (xhr.status == 200)
>+        console.log("FAIL: URL was not revoked");
>+    else
>+        console.log("PASS: URL was revoked");
>+}
>+runTest();
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 185531: 340141 | 340148 | 340150 | 340156 | 340199