Content-Disposition header filename is ignored when 'download' attribute is specified...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Apr 2017 16:29:25 +0000 (16:29 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Apr 2017 16:29:25 +0000 (16:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171239
<rdar://problem/31789855>

Reviewed by Alex Christensen.

Source/WebCore:

Add isAttachmentWithFilename() utility method to ResourceResponse to implement:
- https://html.spec.whatwg.org/#as-a-download (Step 2)

Test: http/tests/download/anchor-download-attribute-content-disposition.html

* platform/network/ResourceResponseBase.cpp:
(WebCore::ResourceResponseBase::isAttachmentWithFilename):
* platform/network/ResourceResponseBase.h:

Source/WebKit2:

Content-Disposition header filename is ignored when 'download' attribute is specified in HTML.
This is not as per HTML specification:
- https://html.spec.whatwg.org/#as-a-download (Step 2)

Content-Disposition header filename is supposed to override the value of the download attribute.

Firefox and Chrome follow the specification.

* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::findPendingDownloadLocation):
* UIProcess/Downloads/DownloadProxy.cpp:
(WebKit::DownloadProxy::didReceiveResponse):

LayoutTests:

* http/tests/security/anchor-download-allow-sameorigin.html:
Stop using attachment.php as resource for this download attribute test because attachment.php
returns a Content-Disposition header with a filename. Given the behavior change in this patch,
this resource is no longer suitable for testing the download attribute.

* http/tests/download/anchor-download-attribute-content-disposition-expected.txt: Added.
* http/tests/download/anchor-download-attribute-content-disposition.html: Added.
* http/tests/download/resources/content-disposition-pass.php: Added.
Add layout test coverage.

* platform/ios-wk2/TestExpectations:
* platform/mac-wk1/TestExpectations:
* platform/win/TestExpectations:
Skip new test on platforms where the download attribute is not supported.

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/download/anchor-download-attribute-content-disposition-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/download/anchor-download-attribute-content-disposition.html [new file with mode: 0644]
LayoutTests/http/tests/download/resources/content-disposition-pass.php [new file with mode: 0644]
LayoutTests/http/tests/security/anchor-download-allow-sameorigin.html
LayoutTests/platform/ios-wk2/TestExpectations
LayoutTests/platform/mac-wk1/TestExpectations
LayoutTests/platform/win/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/network/ResourceResponseBase.cpp
Source/WebCore/platform/network/ResourceResponseBase.h
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/NetworkProcess.cpp
Source/WebKit2/UIProcess/Downloads/DownloadProxy.cpp

index 17779ec..03716a3 100644 (file)
@@ -1,3 +1,26 @@
+2017-04-25  Chris Dumez  <cdumez@apple.com>
+
+        Content-Disposition header filename is ignored when 'download' attribute is specified in HTML
+        https://bugs.webkit.org/show_bug.cgi?id=171239
+        <rdar://problem/31789855>
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/security/anchor-download-allow-sameorigin.html:
+        Stop using attachment.php as resource for this download attribute test because attachment.php
+        returns a Content-Disposition header with a filename. Given the behavior change in this patch,
+        this resource is no longer suitable for testing the download attribute.
+
+        * http/tests/download/anchor-download-attribute-content-disposition-expected.txt: Added.
+        * http/tests/download/anchor-download-attribute-content-disposition.html: Added.
+        * http/tests/download/resources/content-disposition-pass.php: Added.
+        Add layout test coverage.
+
+        * platform/ios-wk2/TestExpectations:
+        * platform/mac-wk1/TestExpectations:
+        * platform/win/TestExpectations:
+        Skip new test on platforms where the download attribute is not supported.
+
 2017-04-25  Ryan Haddad  <ryanhaddad@apple.com>
 
         Rebaseline fast/canvas/canvas-crash.html for ios-simulator.
diff --git a/LayoutTests/http/tests/download/anchor-download-attribute-content-disposition-expected.txt b/LayoutTests/http/tests/download/anchor-download-attribute-content-disposition-expected.txt
new file mode 100644 (file)
index 0000000..c8749b4
--- /dev/null
@@ -0,0 +1,8 @@
+Download started.
+Downloading URL with suggested filename "PASS.txt"
+Download completed.
+Tests that the filename from the Content-Disposition header is used even if a download attribute is specified in HTML.
+
+The suggested filename above should be "PASS.txt" and the download should succeed.
+
+Link with download attribute.
diff --git a/LayoutTests/http/tests/download/anchor-download-attribute-content-disposition.html b/LayoutTests/http/tests/download/anchor-download-attribute-content-disposition.html
new file mode 100644 (file)
index 0000000..7f5edd0
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script type='text/javascript'>
+if (window.testRunner) {
+  testRunner.dumpAsText();
+  testRunner.waitUntilDone();
+}
+</script>
+</head>
+<body>
+<p>Tests that the filename from the Content-Disposition header is used even if a download attribute is specified in HTML.</p>
+<p>The suggested filename above should be "PASS.txt" and the download should succeed.</p>
+<a id="download-url" href="resources/content-disposition-pass.php" download="FAIL.txt">Link with download attribute.</a>
+<script>
+function click(elmt)
+{
+    if (!window.eventSender) {
+        alert('Click the link to run the test.');
+        return;
+    }
+    eventSender.mouseMoveTo(elmt.offsetLeft + 5, elmt.offsetTop + 5);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+}
+
+function runTest()
+{
+    var link = document.getElementById("download-url");
+    click(link);
+}
+runTest();
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/download/resources/content-disposition-pass.php b/LayoutTests/http/tests/download/resources/content-disposition-pass.php
new file mode 100644 (file)
index 0000000..8bbce75
--- /dev/null
@@ -0,0 +1,6 @@
+<?php
+header("Content-Disposition: Attachment; filename=PASS.txt");
+header("Content-Type: text/plain");
+?>
+
+Test file content.
index 0d10b04..9752cf3 100644 (file)
@@ -10,7 +10,7 @@
 <body>
 <p>
 Tests that a suggested filename on a download attribute is allowed if
-<a id="dl" href="/security/resources/attachment.php" download="foo.pdf">the link</a> is in the same origin.
+<a id="dl" href="/media/resources/test.pdf" download="foo.pdf">the link</a> is in the same origin.
 <p>
 The suggested filename at the top should be foo.pdf.
 <script>
index be17657..d9daeb0 100644 (file)
@@ -1823,6 +1823,7 @@ webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-nodownload-set.html [ Skip
 webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-nodownload.html [ Skip ]
 webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-download-synthetic-click.html [ Skip ]
 webkit.org/b/156067 fast/dom/HTMLAnchorElement/anchor-download-user-triggered-synthetic-click.html [ Skip ]
+webkit.org/b/156067 http/tests/download/anchor-download-attribute-content-disposition.html [ Skip ]
 webkit.org/b/156067 http/tests/download/anchor-download-no-extension.html [ Skip ]
 webkit.org/b/156067 http/tests/download/area-download.html [ Skip ]
 webkit.org/b/156067 http/tests/security/anchor-download-allow-blob.html [ Skip ]
index 5742fc4..4b5ec6e 100644 (file)
@@ -227,6 +227,7 @@ webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-include
 webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-slashes.html [ Skip ]
 webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-unicode.html [ Skip ]
 webkit.org/b/156069 fast/dom/HTMLAnchorElement/anchor-file-blob-download-no-extension.html [ Skip ]
+webkit.org/b/156069 http/tests/download/anchor-download-attribute-content-disposition.html [ Skip ]
 webkit.org/b/156069 http/tests/download/anchor-download-no-extension.html [ Skip ]
 webkit.org/b/156069 http/tests/download/area-download.html [ Skip ]
 webkit.org/b/156069 http/tests/security/anchor-download-allow-blob.html [ Skip ]
index 705fe84..3ec0313 100644 (file)
@@ -450,6 +450,7 @@ fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-doublequote.html [
 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-slashes.html [ Skip ]
 fast/dom/HTMLAnchorElement/anchor-file-blob-download-includes-unicode.html [ Skip ]
 fast/dom/HTMLAnchorElement/anchor-file-blob-download-no-extension.html [ Skip ]
+http/tests/download/anchor-download-attribute-content-disposition.html [ Skip ]
 http/tests/download/anchor-download-no-extension.html [ Skip ]
 http/tests/download/area-download.html [ Skip ]
 http/tests/security/anchor-download-allow-data.html [ Skip ]
index 63f5d2f..0f7553e 100644 (file)
@@ -1,3 +1,20 @@
+2017-04-25  Chris Dumez  <cdumez@apple.com>
+
+        Content-Disposition header filename is ignored when 'download' attribute is specified in HTML
+        https://bugs.webkit.org/show_bug.cgi?id=171239
+        <rdar://problem/31789855>
+
+        Reviewed by Alex Christensen.
+
+        Add isAttachmentWithFilename() utility method to ResourceResponse to implement:
+        - https://html.spec.whatwg.org/#as-a-download (Step 2)
+
+        Test: http/tests/download/anchor-download-attribute-content-disposition.html
+
+        * platform/network/ResourceResponseBase.cpp:
+        (WebCore::ResourceResponseBase::isAttachmentWithFilename):
+        * platform/network/ResourceResponseBase.h:
+
 2017-04-25  Antti Koivisto  <antti@apple.com>
 
         Enable expired-only reload policy on Mac and iOS
index 78c269b..40b8ad6 100644 (file)
@@ -558,6 +558,21 @@ bool ResourceResponseBase::isAttachment() const
     return equalLettersIgnoringASCIICase(value.left(value.find(';')).stripWhiteSpace(), "attachment");
 }
 
+bool ResourceResponseBase::isAttachmentWithFilename() const
+{
+    lazyInit(AllFields);
+
+    String contentDisposition = m_httpHeaderFields.get(HTTPHeaderName::ContentDisposition);
+    if (contentDisposition.isNull())
+        return false;
+
+    if (!equalLettersIgnoringASCIICase(contentDisposition.left(contentDisposition.find(';')).stripWhiteSpace(), "attachment"))
+        return false;
+
+    String filename = filenameFromHTTPContentDisposition(contentDisposition);
+    return !filename.isNull();
+}
+
 ResourceResponseBase::Source ResourceResponseBase::source() const
 {
     lazyInit(AllFields);
index 3e32f03..8b0fd59 100644 (file)
@@ -113,6 +113,7 @@ public:
     bool isMultipart() const { return mimeType() == "multipart/x-mixed-replace"; }
 
     WEBCORE_EXPORT bool isAttachment() const;
+    WEBCORE_EXPORT bool isAttachmentWithFilename() const;
     WEBCORE_EXPORT String suggestedFilename() const;
     WEBCORE_EXPORT static String sanitizeSuggestedFilename(const String&);
 
index 0a444a9..876d5ca 100644 (file)
@@ -1,3 +1,24 @@
+2017-04-25  Chris Dumez  <cdumez@apple.com>
+
+        Content-Disposition header filename is ignored when 'download' attribute is specified in HTML
+        https://bugs.webkit.org/show_bug.cgi?id=171239
+        <rdar://problem/31789855>
+
+        Reviewed by Alex Christensen.
+
+        Content-Disposition header filename is ignored when 'download' attribute is specified in HTML.
+        This is not as per HTML specification:
+        - https://html.spec.whatwg.org/#as-a-download (Step 2)
+
+        Content-Disposition header filename is supposed to override the value of the download attribute.
+
+        Firefox and Chrome follow the specification.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::findPendingDownloadLocation):
+        * UIProcess/Downloads/DownloadProxy.cpp:
+        (WebKit::DownloadProxy::didReceiveResponse):
+
 2017-04-25  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Add WebKitInspectorWindow to create inspector windows from local and remote inspector
index b04acc8..04d807a 100644 (file)
@@ -519,7 +519,12 @@ void NetworkProcess::findPendingDownloadLocation(NetworkDataTask& networkDataTas
     downloadProxyConnection()->send(Messages::DownloadProxy::DidReceiveResponse(response), destinationID);
 
     downloadManager().willDecidePendingDownloadDestination(networkDataTask, WTFMove(completionHandler));
-    String suggestedFilename = MIMETypeRegistry::appendFileExtensionIfNecessary(networkDataTask.suggestedFilename(), response.mimeType());
+
+    // As per https://html.spec.whatwg.org/#as-a-download (step 2), the filename from the Content-Disposition header
+    // should override the suggested filename from the download attribute.
+    String suggestedFilename = response.isAttachmentWithFilename() ? response.suggestedFilename() : networkDataTask.suggestedFilename();
+    suggestedFilename = MIMETypeRegistry::appendFileExtensionIfNecessary(suggestedFilename, response.mimeType());
+
     downloadProxyConnection()->send(Messages::DownloadProxy::DecideDestinationWithSuggestedFilenameAsync(networkDataTask.pendingDownloadID(), suggestedFilename), destinationID);
 }
 #endif
index 57d8e3d..10925e4 100644 (file)
@@ -154,6 +154,13 @@ void DownloadProxy::didReceiveResponse(const ResourceResponse& response)
     if (!m_processPool)
         return;
 
+#if !USE(NETWORK_SESSION)
+    // As per https://html.spec.whatwg.org/#as-a-download (step 2), the filename from the Content-Disposition header
+    // should override the suggested filename from the download attribute.
+    if (!m_suggestedFilename.isNull() && response.isAttachmentWithFilename())
+        m_suggestedFilename = String();
+#endif
+
     m_processPool->downloadClient().didReceiveResponse(m_processPool.get(), this, response);
 }