Resource requests from Save-Page-As should go through CanRequestURL checks.

chrome/browser/download/save_page_browsertest.cc

Index: chrome/browser/download/save_page_browsertest.cc
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index 544d49935ad23def57a279a2ebfcfeb6611b5b31..73fcdd26246eab98a6a641a5749f4d0506283cbb 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -18,6 +18,7 @@
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/test_file_util.h"
 #include "build/build_config.h"
@@ -72,6 +73,7 @@ using content::WebContents;
 using net::URLRequestMockHTTPJob;
 using testing::ContainsRegex;
 using testing::HasSubstr;
+using testing::Not;
 
 namespace {
 
@@ -811,6 +813,41 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveDownloadableIFrame) {
   EXPECT_TRUE(base::PathExists(dir.AppendASCII("no-such-file.html")));
 }
 
+// Test that file: URI won't be saved when referred to from an HTTP page.
+// See also https://crbug.com/616429.
+IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveUnauthorizedResource) {
+  GURL url = NavigateToMockURL("unauthorized-access");
+
+  // Create a test file (that the web page should not have access to).
+  base::ScopedTempDir temp_dir2;
+  ASSERT_TRUE(temp_dir2.CreateUniqueTempDir());
+  base::FilePath file_path =
+      temp_dir2.path().Append(FILE_PATH_LITERAL("should-not-save.jpg"));
+  std::string file_content("fake-jpg");
+  ASSERT_LT(
+      0, base::WriteFile(file_path, file_content.data(), file_content.size()));
+
+  // Refer to the test file from the test page.
+  GURL file_url = net::FilePathToFileURL(file_path);
+  ASSERT_TRUE(ExecuteScript(
+      browser()->tab_strip_model()->GetWebContentsAt(0),
+      base::StringPrintf("document.getElementById('resource1').src = '%s';",
+                         file_url.spec().data())));
+
+  // Save the current page.
+  base::FilePath full_file_name, dir;
+  SaveCurrentTab(url, content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML,
+                 "unauthorized-access", 2, &dir, &full_file_name);
+
+  // We should not save resource that the web page didn't have access to.
+  // (because executing a resource request can have side effects - for example
+  // after https://crbug.com/590714 a website from the internet should not be
+  // able to issue a resource request to an intranet website and trigger
+  // server-side actions in the internet;  this test uses a file: URI as a
+  // canary for detecting whether a website can access restricted resources).
+  EXPECT_FALSE(base::PathExists(dir.AppendASCII("should-not-save.jpg")));
+}
+
 // Test suite that allows testing --site-per-process against cross-site frames.
 // See http://dev.chromium.org/developers/design-documents/site-isolation.
 class SavePageSitePerProcessBrowserTest : public SavePageBrowserTest {
@@ -1021,6 +1058,33 @@ class SavePageOriginalVsSavedComparisonTest
                                       expected_substrings);
   }
 
+  // Helper method to deduplicate some code across 2 tests.
+  void RunObjectElementsTest(GURL url) {
+    content::SavePageType save_page_type = GetParam();
+
+    // 4 = main frame + iframe + object w/ html doc + object w/ pdf doc + plain
+    // text (svg and png objects do not get a separate frame)

[Randy Smith (Not in Mondays), 2016/07/21 19:29:53]

I do not understand the relationship between the comment and the following line,
probably because 4 != 7?

[Łukasz Anforowicz, 2016/07/21 23:44:30]

Ooops - done.  I screwed that up in
https://chromium.googlesource.com/chromium/src/+/c5aebc965aa088c133f20b51f627...

+    int expected_number_of_frames = 7;
+
+    std::string arr[] = {
+        "frames-objects.htm: 8da13db4-a512-4d9b-b1c5-dc1c134234b9",
+        "a.htm: 1b8aae2b-e164-462f-bd5b-98aa366205f2",
+        "b.htm: 3a35f7fa-96a9-4487-9f18-4470263907fa",
+        "frames-nested.htm: 4388232f-8d45-4d2e-9807-721b381be153",
+        "frames-nested2.htm: 6d23dc47-f283-4977-96ec-66bcf72301a4",
+        "text-object.txt: ae52dd09-9746-4b7e-86a6-6ada5e2680c2",
+    };
+    std::vector<std::string> expected_substrings(std::begin(arr),
+                                                 std::end(arr));
+
+    // TODO(lukasza): crbug.com/553478: Enable <object> testing of MHTML.
+    if (save_page_type == content::SAVE_PAGE_TYPE_AS_MHTML)
+      return;
+
+    TestOriginalVsSavedPage(save_page_type, url, expected_number_of_frames,
+                            expected_substrings);
+  }
+
  private:
   void AssertExpectationsAboutCurrentTab(
       int expected_number_of_frames,
@@ -1090,31 +1154,24 @@ IN_PROC_BROWSER_TEST_P(SavePageOriginalVsSavedComparisonTest, CrossSite) {
 
 // Test compares original-vs-saved for a page with <object> elements.
 // (see crbug.com/553478).
-IN_PROC_BROWSER_TEST_P(SavePageOriginalVsSavedComparisonTest, ObjectElements) {
-  content::SavePageType save_page_type = GetParam();
-
-  // 4 = main frame + iframe + object w/ html doc + object w/ pdf doc
-  // (svg and png objects do not get a separate frame)
-  int expected_number_of_frames = 6;
-
-  std::string arr[] = {
-      "frames-objects.htm: 8da13db4-a512-4d9b-b1c5-dc1c134234b9",
-      "a.htm: 1b8aae2b-e164-462f-bd5b-98aa366205f2",
-      "b.htm: 3a35f7fa-96a9-4487-9f18-4470263907fa",
-      "frames-nested.htm: 4388232f-8d45-4d2e-9807-721b381be153",
-      "frames-nested2.htm: 6d23dc47-f283-4977-96ec-66bcf72301a4",
-  };
-  std::vector<std::string> expected_substrings(std::begin(arr), std::end(arr));
-
+IN_PROC_BROWSER_TEST_P(SavePageOriginalVsSavedComparisonTest,
+                       ObjectElementsViaHttp) {
   GURL url(
       embedded_test_server()->GetURL("a.com", "/save_page/frames-objects.htm"));
 
-  // TODO(lukasza): crbug.com/553478: Enable <object> testing of MHTML.
-  if (save_page_type == content::SAVE_PAGE_TYPE_AS_MHTML)
-    return;
+  RunObjectElementsTest(url);
+}
+
+// Tests that saving a page from file: URI works.
+IN_PROC_BROWSER_TEST_P(SavePageOriginalVsSavedComparisonTest,
+                       ObjectElementsViaFile) {
+  base::FilePath test_data_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir));
+  GURL url(net::FilePathToFileURL(
+      test_data_dir.Append(FILE_PATH_LITERAL("save_page/frames-objects.htm"))));
+  EXPECT_TRUE(url.SchemeIsFile());
 
-  TestOriginalVsSavedPage(save_page_type, url, expected_number_of_frames,
-                          expected_substrings);
+  RunObjectElementsTest(url);
 }
 
 // Test compares original-vs-saved for a page with frames at about:blank uri.