HitTestResult's linkSuggestedFilename should sanitize download attribute
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 26 Feb 2017 18:31:13 +0000 (18:31 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 26 Feb 2017 18:31:13 +0000 (18:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=168856
<rdar://problem/30683109>

Reviewed by Antti Koivisto.

Source/WebCore:

HitTestResult's linkSuggestedFilename should sanitize download attribute.
This is used by the context menu's "Download Linked File" & "Download Linked
File As..." actions.

* rendering/HitTestResult.cpp:
(WebCore::HitTestResult::linkSuggestedFilename):
* rendering/HitTestResult.h:

Source/WebKit2:

HitTestResult's linkSuggestedFilename should sanitize download attribute.
This is used by the context menu's "Download Linked File" & "Download Linked
File As..." actions.

* Shared/WebHitTestResultData.cpp:
(WebKit::WebHitTestResultData::WebHitTestResultData):
* WebProcess/InjectedBundle/InjectedBundleHitTestResult.cpp:
(WebKit::InjectedBundleHitTestResult::linkSuggestedFilename):

Tools:

Add test coverage.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit2/link-with-download-attribute-with-slashes.html: Added.
* TestWebKitAPI/Tests/WebKit2/mac/ContextMenuDownload.mm:
(TestWebKitAPI::decideDestinationWithSuggestedFilenameContainingSlashes):
(TestWebKitAPI::TEST):

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/HitTestResult.cpp
Source/WebCore/rendering/HitTestResult.h
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/WebHitTestResultData.cpp
Source/WebKit2/WebProcess/InjectedBundle/InjectedBundleHitTestResult.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKit2/link-with-download-attribute-with-slashes.html [new file with mode: 0644]
Tools/TestWebKitAPI/Tests/WebKit2/mac/ContextMenuDownload.mm

index f761999..8285559 100644 (file)
@@ -1,3 +1,19 @@
+2017-02-26  Chris Dumez  <cdumez@apple.com>
+
+        HitTestResult's linkSuggestedFilename should sanitize download attribute
+        https://bugs.webkit.org/show_bug.cgi?id=168856
+        <rdar://problem/30683109>
+
+        Reviewed by Antti Koivisto.
+
+        HitTestResult's linkSuggestedFilename should sanitize download attribute.
+        This is used by the context menu's "Download Linked File" & "Download Linked
+        File As..." actions.
+
+        * rendering/HitTestResult.cpp:
+        (WebCore::HitTestResult::linkSuggestedFilename):
+        * rendering/HitTestResult.h:
+
 2017-02-25  Zalan Bujtas  <zalan@apple.com>
 
         Simple line layout: Move coverage functions out of SimpleLineLayout.cpp
index d22585a..ba49407 100644 (file)
@@ -784,12 +784,12 @@ Element* HitTestResult::innerNonSharedElement() const
     return node->parentElement();
 }
 
-const AtomicString& HitTestResult::URLElementDownloadAttribute() const
+String HitTestResult::linkSuggestedFilename() const
 {
     auto* urlElement = URLElement();
     if (!is<HTMLAnchorElement>(urlElement))
         return nullAtom;
-    return urlElement->attributeWithoutSynchronization(HTMLNames::downloadAttr);
+    return ResourceResponse::sanitizeSuggestedFilename(urlElement->attributeWithoutSynchronization(HTMLNames::downloadAttr));
 }
 
 bool HitTestResult::mediaSupportsEnhancedFullscreen() const
index beedf8b..7a25126 100644 (file)
@@ -62,7 +62,7 @@ public:
     Scrollbar* scrollbar() const { return m_scrollbar.get(); }
     bool isOverWidget() const { return m_isOverWidget; }
 
-    WEBCORE_EXPORT const AtomicString& URLElementDownloadAttribute() const;
+    WEBCORE_EXPORT String linkSuggestedFilename() const;
 
     // Forwarded from HitTestLocation
     bool isRectBasedTest() const { return m_hitTestLocation.isRectBasedTest(); }
index 8ca04ee..53ef18f 100644 (file)
@@ -1,3 +1,20 @@
+2017-02-26  Chris Dumez  <cdumez@apple.com>
+
+        HitTestResult's linkSuggestedFilename should sanitize download attribute
+        https://bugs.webkit.org/show_bug.cgi?id=168856
+        <rdar://problem/30683109>
+
+        Reviewed by Antti Koivisto.
+
+        HitTestResult's linkSuggestedFilename should sanitize download attribute.
+        This is used by the context menu's "Download Linked File" & "Download Linked
+        File As..." actions.
+
+        * Shared/WebHitTestResultData.cpp:
+        (WebKit::WebHitTestResultData::WebHitTestResultData):
+        * WebProcess/InjectedBundle/InjectedBundleHitTestResult.cpp:
+        (WebKit::InjectedBundleHitTestResult::linkSuggestedFilename):
+
 2017-02-25  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [GTK] Unreviewed, document deficiency in webkit_website_data_manager_clear() API
index b7ce3e4..f3e6a5e 100644 (file)
@@ -46,7 +46,7 @@ WebHitTestResultData::WebHitTestResultData(const WebCore::HitTestResult& hitTest
     , absoluteMediaURL(hitTestResult.absoluteMediaURL().string())
     , linkLabel(hitTestResult.textContent())
     , linkTitle(hitTestResult.titleDisplayString())
-    , linkSuggestedFilename(hitTestResult.URLElementDownloadAttribute().string())
+    , linkSuggestedFilename(hitTestResult.linkSuggestedFilename())
     , isContentEditable(hitTestResult.isContentEditable())
     , elementBoundingBox(elementBoundingBoxInWindowCoordinates(hitTestResult))
     , isScrollbar(hitTestResult.scrollbar())
@@ -66,7 +66,7 @@ WebHitTestResultData::WebHitTestResultData(const WebCore::HitTestResult& hitTest
     , absoluteMediaURL(hitTestResult.absoluteMediaURL().string())
     , linkLabel(hitTestResult.textContent())
     , linkTitle(hitTestResult.titleDisplayString())
-    , linkSuggestedFilename(hitTestResult.URLElementDownloadAttribute().string())
+    , linkSuggestedFilename(hitTestResult.linkSuggestedFilename())
     , isContentEditable(hitTestResult.isContentEditable())
     , elementBoundingBox(elementBoundingBoxInWindowCoordinates(hitTestResult))
     , isScrollbar(hitTestResult.scrollbar())
index 3eecf51..826d372 100644 (file)
@@ -141,7 +141,7 @@ String InjectedBundleHitTestResult::linkTitle() const
 
 String InjectedBundleHitTestResult::linkSuggestedFilename() const
 {
-    return m_hitTestResult.URLElementDownloadAttribute();
+    return m_hitTestResult.linkSuggestedFilename();
 }
 
 IntRect InjectedBundleHitTestResult::imageRect() const
index b3d0f96..454a6b0 100644 (file)
@@ -1,3 +1,19 @@
+2017-02-26  Chris Dumez  <cdumez@apple.com>
+
+        HitTestResult's linkSuggestedFilename should sanitize download attribute
+        https://bugs.webkit.org/show_bug.cgi?id=168856
+        <rdar://problem/30683109>
+
+        Reviewed by Antti Koivisto.
+
+        Add test coverage.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKit2/link-with-download-attribute-with-slashes.html: Added.
+        * TestWebKitAPI/Tests/WebKit2/mac/ContextMenuDownload.mm:
+        (TestWebKitAPI::decideDestinationWithSuggestedFilenameContainingSlashes):
+        (TestWebKitAPI::TEST):
+
 2017-02-24  Joseph Pecoraro  <pecoraro@apple.com>
 
         [Resource Timing] Media elements initiated loads should set the initiatorType to their element name (video/audio)
index c47ad81..15f0d7d 100644 (file)
                8349D3C21DB96DDE004A9F65 /* ContextMenuDownload.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8349D3C11DB96DDA004A9F65 /* ContextMenuDownload.mm */; };
                8349D3C41DB9728E004A9F65 /* link-with-download-attribute.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 8349D3C31DB9724F004A9F65 /* link-with-download-attribute.html */; };
                835CF9671D25FCD6001A65D4 /* RestoreSessionStateWithoutNavigation.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 835CF9661D25FCD6001A65D4 /* RestoreSessionStateWithoutNavigation.cpp */; };
+               8361F1781E610B4E00759B25 /* link-with-download-attribute-with-slashes.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 8361F1771E610B2100759B25 /* link-with-download-attribute-with-slashes.html */; };
                837A35F11D9A1E7D00663C57 /* DownloadRequestBlobURL.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 837A35F01D9A1E6400663C57 /* DownloadRequestBlobURL.html */; };
                83CF1C301C4F1B8B00688447 /* StringUtilities.mm in Sources */ = {isa = PBXBuildFile; fileRef = 83CF1C2C1C4F19AE00688447 /* StringUtilities.mm */; };
                8E4A85371E1D1AB200F53B0F /* GridPosition.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8E4A85361E1D1AA100F53B0F /* GridPosition.cpp */; };
                        dstPath = TestWebKitAPI.resources;
                        dstSubfolderSpec = 7;
                        files = (
+                               8361F1781E610B4E00759B25 /* link-with-download-attribute-with-slashes.html in Copy Resources */,
                                C25CCA0D1E5141840026CB8A /* AllAhem.svg in Copy Resources */,
                                C25CCA0B1E5140C10026CB8A /* LineBreaking.html in Copy Resources */,
                                C9C60E651E53A9DC006DA181 /* autoplay-check-frame.html in Copy Resources */,
                8349D3C11DB96DDA004A9F65 /* ContextMenuDownload.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ContextMenuDownload.mm; sourceTree = "<group>"; };
                8349D3C31DB9724F004A9F65 /* link-with-download-attribute.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "link-with-download-attribute.html"; sourceTree = "<group>"; };
                835CF9661D25FCD6001A65D4 /* RestoreSessionStateWithoutNavigation.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RestoreSessionStateWithoutNavigation.cpp; sourceTree = "<group>"; };
+               8361F1771E610B2100759B25 /* link-with-download-attribute-with-slashes.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "link-with-download-attribute-with-slashes.html"; sourceTree = "<group>"; };
                837A35F01D9A1E6400663C57 /* DownloadRequestBlobURL.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = DownloadRequestBlobURL.html; sourceTree = "<group>"; };
                83B88A331C80056D00BB2418 /* HTMLParserIdioms.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HTMLParserIdioms.cpp; sourceTree = "<group>"; };
                83CF1C2C1C4F19AE00688447 /* StringUtilities.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = StringUtilities.mm; sourceTree = "<group>"; };
                                CE3524F51B142BBB0028A7C5 /* input-focus-blur.html */,
                                C99B675B1E3971FC00FC6C80 /* js-play-with-controls.html */,
                                8349D3C31DB9724F004A9F65 /* link-with-download-attribute.html */,
+                               8361F1771E610B2100759B25 /* link-with-download-attribute-with-slashes.html */,
                                378E647816326FDF00B6C676 /* link-with-title.html */,
                                9361002814DC957B0061379D /* lots-of-iframes.html */,
                                93AF4ECF1506F123007FD57E /* lots-of-images.html */,
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2/link-with-download-attribute-with-slashes.html b/Tools/TestWebKitAPI/Tests/WebKit2/link-with-download-attribute-with-slashes.html
new file mode 100644 (file)
index 0000000..75b926b
--- /dev/null
@@ -0,0 +1,10 @@
+<html>
+    <body>
+        <a id="testAnchor" style="display: block; height: 100%; width: 100%" download="test1/test2/downloadAttributeValue.txt"></a>
+        <script>
+            var blob = new Blob(["Hello world!"], {type: "application/octet-stream"});
+            var link = document.getElementById("testAnchor");
+            link.href = window.URL.createObjectURL(blob);
+        </script>
+    </body>
+</html>
index d16e87b..62e8e2f 100644 (file)
@@ -108,4 +108,50 @@ TEST(WebKit2, ContextMenuDownloadHTMLDownloadAttribute)
     Util::run(&didDecideDownloadDestination);
 }
 
+static WKStringRef decideDestinationWithSuggestedFilenameContainingSlashes(WKContextRef, WKDownloadRef download, WKStringRef suggestedFilename, bool*, const void*)
+{
+    // Make sure the suggested filename is provided and matches the value of the download attribute in the HTML, after sanitization.
+    EXPECT_WK_STREQ("test1_test2_downloadAttributeValue.txt", suggestedFilename);
+
+    WKDownloadCancel(download);
+    didDecideDownloadDestination = true;
+
+    return Util::toWK("/tmp/WebKitAPITest/ContextMenuDownload").leakRef();
+}
+
+TEST(WebKit2, ContextMenuDownloadHTMLDownloadAttributeWithSlashes)
+{
+    WKRetainPtr<WKContextRef> context(AdoptWK, Util::createContextWithInjectedBundle());
+
+    WKContextDownloadClientV0 client;
+    memset(&client, 0, sizeof(client));
+    client.base.version = 0;
+    client.decideDestinationWithSuggestedFilename = decideDestinationWithSuggestedFilenameContainingSlashes;
+    WKContextSetDownloadClient(context.get(), &client.base);
+
+    WKRetainPtr<WKPageGroupRef> pageGroup(AdoptWK, WKPageGroupCreateWithIdentifier(Util::toWK("MyGroup").get()));
+    PlatformWebView webView(context.get(), pageGroup.get());
+
+    WKPageLoaderClientV0 loaderClient;
+    memset(&loaderClient, 0, sizeof(loaderClient));
+    loaderClient.base.version = 0;
+    loaderClient.didFinishLoadForFrame = didFinishLoadForFrame;
+    WKPageSetPageLoaderClient(webView.page(), &loaderClient.base);
+
+    WKPageContextMenuClientV3 contextMenuClient;
+    memset(&contextMenuClient, 0, sizeof(contextMenuClient));
+    contextMenuClient.base.version = 3;
+    contextMenuClient.getContextMenuFromProposedMenu = getContextMenuFromProposedMenu;
+    WKPageSetPageContextMenuClient(webView.page(), &contextMenuClient.base);
+
+    WKRetainPtr<WKURLRef> url(AdoptWK, Util::createURLForResource("link-with-download-attribute-with-slashes", "html"));
+
+    WKPageLoadURL(webView.page(), url.get());
+    Util::run(&didFinishLoad);
+
+    // Right click on link.
+    webView.simulateButtonClick(kWKEventMouseButtonRightButton, 50, 50, 0);
+    Util::run(&didDecideDownloadDestination);
+}
+
 }