[go: up one dir, main page]

[m76] Cross-Origin-Resource-Policy shouldn't apply to Save-Page-As.

This CL makes sure that download requests initiated by
content::SaveFileManager::SaveURL use
network::mojom::FetchRequestMode::kNavigate mode (rather than the
default kNoCors mode).

TBR=lukasza@chromium.org

(cherry picked from commit d84fd3d687cccbd66e0996e08a3623721532c9ec)

Bug: 974312
Change-Id: Ica57a78ca96d7a76e95e87c8f6c0b4db74f3de94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660882
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#669410}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662735
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#370}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index 0c04e002..c30ac87a 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -419,6 +419,28 @@
   EXPECT_TRUE(base::ContentsEqual(GetTestDirFile("a.htm"), full_file_name));
 }
 
+IN_PROC_BROWSER_TEST_F(SavePageBrowserTest,
+                       SaveHTMLOnly_CrossOriginReadPolicy) {
+  GURL url = embedded_test_server()->GetURL(
+      "/downloads/cross-origin-resource-policy-resource.txt");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  base::FilePath full_file_name, dir;
+  SaveCurrentTab(url, content::SAVE_PAGE_TYPE_AS_ONLY_HTML, "a", 1, &dir,
+                 &full_file_name);
+  ASSERT_FALSE(HasFailure());
+
+  base::ScopedAllowBlockingForTesting allow_blocking;
+  EXPECT_TRUE(base::PathExists(full_file_name));
+  EXPECT_FALSE(base::PathExists(dir));
+
+  const base::FilePath::CharType kTestDir[] = FILE_PATH_LITERAL("downloads");
+  const base::FilePath kTestFile =
+      test_dir_.Append(base::FilePath(kTestDir))
+          .AppendASCII("cross-origin-resource-policy-resource.txt");
+  EXPECT_TRUE(base::ContentsEqual(kTestFile, full_file_name));
+}
+
 IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveHTMLOnlyCancel) {
   GURL url = NavigateToMockURL("a");
   DownloadManager* manager = GetDownloadManager();
@@ -524,6 +546,8 @@
   EXPECT_TRUE(base::ContentsEqual(GetTestDirFile("a.htm"), full_file_name));
 }
 
+// Regression test for https://crbug.com/974312 (saving a page that was served
+// with `Cross-Origin-Resource-Policy: same-origin` http response header).
 IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveCompleteHTML) {
   GURL url = NavigateToMockURL("b");
 
diff --git a/content/browser/download/save_file_manager.cc b/content/browser/download/save_file_manager.cc
index 9aab419..78d9257d 100644
--- a/content/browser/download/save_file_manager.cc
+++ b/content/browser/download/save_file_manager.cc
@@ -241,6 +241,13 @@
     request->priority = net::DEFAULT_PRIORITY;
     request->load_flags = net::LOAD_SKIP_CACHE_VALIDATION;
 
+    // To avoid https://crbug.com/974312, downloads initiated by Save-Page-As
+    // should be treated as navigations. This definitely makes sense for the
+    // top-level page (e.g. in SAVE_PAGE_TYPE_AS_ONLY_HTML mode). This is
+    // probably also okay for subresources downloaded in
+    // SAVE_PAGE_TYPE_AS_COMPLETE_HTML mode.
+    request->fetch_request_mode = network::mojom::FetchRequestMode::kNavigate;
+
     network::mojom::URLLoaderFactory* factory = nullptr;
     std::unique_ptr<DataURLLoaderFactory> data_url_loader_factory;