Skip to content

Commit

Permalink
Cross-Origin-Resource-Policy shouldn't apply to Save-Link-As downloads.
Browse files Browse the repository at this point in the history
This CL makes sure that download requests initiated by
download::ResourceDownloader::Start use
network::mojom::FetchRequestMode::kNavigate mode (rather than the
default kNoCors mode).

Bug: 952834
Change-Id: Ie99a4e024864f442e439e621d54ed7dae92f1393
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1572876
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652578}
  • Loading branch information
anforowicz authored and Commit Bot committed Apr 19, 2019
1 parent 1a8158b commit aadaf27
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 0 deletions.
70 changes: 70 additions & 0 deletions chrome/browser/download/download_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,9 @@ class DownloadTest : public InProcessBrowserTest {
base::BindOnce(&chrome_browser_net::SetUrlRequestMocksEnabled, true));
ASSERT_TRUE(InitialSetup());
host_resolver()->AddRule("www.a.com", "127.0.0.1");
host_resolver()->AddRule("foo.com", "127.0.0.1");
host_resolver()->AddRule("bar.com", "127.0.0.1");
content::SetupCrossSiteRedirector(embedded_test_server());
}

void TearDownOnMainThread() override {
Expand Down Expand Up @@ -3008,6 +3011,73 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SaveLinkAsReferrerPolicyOrigin) {
EXPECT_TRUE(VerifyFile(file, expected_contents, expected_contents.length()));
}

// This test ensures that Cross-Origin-Resource-Policy response header doesn't
// apply to download requests initiated via Save Link As context menu (such
// requests are considered browser-initiated). See also
// https://crbug.com/952834.
IN_PROC_BROWSER_TEST_F(DownloadTest, SaveLinkAsVsCrossOriginResourcePolicy) {
ASSERT_TRUE(embedded_test_server()->Start());
EnableFileChooser(true);

// Test's sanity check that initially there are no download items.
std::vector<DownloadItem*> download_items;
GetDownloads(browser(), &download_items);
ASSERT_TRUE(download_items.empty());

// Read the origin file now so that we can compare the downloaded files to it
// later.
base::FilePath origin(OriginFile(base::FilePath(FILE_PATH_LITERAL(
"downloads/cross-origin-resource-policy-resource.txt"))));
int64_t origin_file_size = 0;
std::string original_contents;
{
base::ScopedAllowBlockingForTesting allow_blocking;
ASSERT_TRUE(base::PathExists(origin));
EXPECT_TRUE(base::GetFileSize(origin, &origin_file_size));
EXPECT_TRUE(base::ReadFileToString(origin, &original_contents));
}

// Navigate to the test page.
GURL url = embedded_test_server()->GetURL(
"foo.com", "/downloads/cross-origin-resource-policy-test.html");
ui_test_utils::NavigateToURL(browser(), url);

// Right-click on the link and choose Save Link As. This will download the
// link target.
std::unique_ptr<content::DownloadTestObserver> download_waiter(
new content::DownloadTestObserverTerminal(
DownloadManagerForBrowser(browser()), 1,
content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
ContextMenuNotificationObserver context_menu_observer(
IDC_CONTENT_CONTEXT_SAVELINKAS);
WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
blink::WebMouseEvent mouse_event(
blink::WebInputEvent::kMouseDown, blink::WebInputEvent::kNoModifiers,
blink::WebInputEvent::GetStaticTimeStampForTests());
mouse_event.button = blink::WebMouseEvent::Button::kRight;
mouse_event.SetPositionInWidget(15, 15);
mouse_event.click_count = 1;
tab->GetRenderViewHost()->GetWidget()->ForwardMouseEvent(mouse_event);
mouse_event.SetType(blink::WebInputEvent::kMouseUp);
tab->GetRenderViewHost()->GetWidget()->ForwardMouseEvent(mouse_event);

download_waiter->WaitForFinished();
EXPECT_EQ(1u,
download_waiter->NumDownloadsSeenInState(DownloadItem::COMPLETE));
CheckDownloadStates(1, DownloadItem::COMPLETE);

// Validate that the correct file was downloaded.
GetDownloads(browser(), &download_items);
ASSERT_EQ(1u, download_items.size());
GURL expected_original_url = embedded_test_server()->GetURL(
"foo.com",
"/cross-site/bar.com/downloads/"
"cross-origin-resource-policy-resource.txt");
EXPECT_EQ(expected_original_url, download_items[0]->GetOriginalUrl());
EXPECT_TRUE(VerifyFile(download_items[0]->GetTargetFilePath(),
original_contents, origin_file_size));
}

// This test ensures that the Referer header is properly sanitized when
// Save Image As is chosen from the context menu.
IN_PROC_BROWSER_TEST_F(DownloadTest, SaveImageAsReferrerPolicyDefault) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Resource served with `Cross-Origin-Resource-Protected: same-origin`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
HTTP/1.0 200 OK
Content-Type: text/plain
Cross-Origin-Resource-Policy: same-origin
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<a
href="/cross-site/bar.com/downloads/cross-origin-resource-policy-resource.txt"
>link</a>
6 changes: 6 additions & 0 deletions components/download/internal/common/download_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,12 @@ std::unique_ptr<network::ResourceRequest> CreateResourceRequest(
request->allow_download = true;
request->is_main_frame = true;

// Downloads should be treated as navigations from Fetch spec perspective.
// See also:
// - https://crbug.com/952834
// - https://github.com/whatwg/fetch/issues/896#issuecomment-484423278
request->fetch_request_mode = network::mojom::FetchRequestMode::kNavigate;

if (params->render_process_host_id() >= 0)
request->render_frame_id = params->render_frame_host_routing_id();

Expand Down
2 changes: 2 additions & 0 deletions content/browser/frame_host/navigation_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1772,6 +1772,8 @@ void NavigationRequest::OnWillProcessResponseChecksComplete(
resource_request->request_initiator = common_params_.initiator_origin;
resource_request->referrer = common_params_.referrer.url;
resource_request->has_user_gesture = common_params_.has_user_gesture;
resource_request->fetch_request_mode =
network::mojom::FetchRequestMode::kNavigate;

BrowserContext* browser_context =
frame_tree_node_->navigator()->GetController()->GetBrowserContext();
Expand Down

0 comments on commit aadaf27

Please sign in to comment.