Escape path string before storing as externalfile URL path.

Resolve errors on Chrome OS with opening Drive files whose file name
contains a valid URL-encoding like "%41.pdf".


Test: unit_tests --gtest_filter=ExternalFileURLUtilTest.*:FileManagerPathUtilConvertUrlTest.*
Bug: 850035
Change-Id: If4768ae07f82d722065601192d557eaea53b3f12
Reviewed-on: https://chromium-review.googlesource.com/1151172
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578599}

chrome/browser/chromeos/file_manager/path_util_unittest.cc

@@ -197,8 +197,12 @@ TEST_F(FileManagerPathUtilConvertUrlTest, ConvertPathToArcUrl_Special) {
   GURL url;
   EXPECT_TRUE(
       ConvertPathToArcUrl(drive_mount_point_.AppendASCII("a/b/c"), &url));
+  // "@" appears escaped 3 times here because escaping happens when:
+  // - creating drive mount point name for user
+  // - creating externalfile: URL from the path
+  // - encoding the URL to Chrome content provider URL
   EXPECT_EQ(GURL("content://org.chromium.arc.chromecontentprovider/"
-                 "externalfile%3Adrive-user%2540gmail.com-hash%2Fa%2Fb%2Fc"),
+                 "externalfile%3Adrive-user%252540gmail.com-hash%2Fa%2Fb%2Fc"),
             url);
 }
 
@@ -301,11 +305,10 @@ TEST_F(FileManagerPathUtilConvertUrlTest, ConvertToContentUrls_Special) {
           [](base::RunLoop* run_loop, const std::vector<GURL>& urls) {
             run_loop->Quit();
             ASSERT_EQ(1U, urls.size());
-            EXPECT_EQ(
-                GURL(
-                    "content://org.chromium.arc.chromecontentprovider/"
-                    "externalfile%3Adrive-user%2540gmail.com-hash%2Fa%2Fb%2Fc"),
-                urls[0]);
+            EXPECT_EQ(GURL("content://org.chromium.arc.chromecontentprovider/"
+                           "externalfile%3Adrive-user%252540gmail.com-hash%2Fa%"
+                           "2Fb%2Fc"),
+                      urls[0]);
           },
           &run_loop));
   run_loop.Run();
@@ -416,11 +419,10 @@ TEST_F(FileManagerPathUtilConvertUrlTest, ConvertToContentUrls_MultipleUrls) {
             EXPECT_EQ(
                 GURL("content://org.chromium.arc.removablemediaprovider/a/b/c"),
                 urls[1]);
-            EXPECT_EQ(
-                GURL(
-                    "content://org.chromium.arc.chromecontentprovider/"
-                    "externalfile%3Adrive-user%2540gmail.com-hash%2Fa%2Fb%2Fc"),
-                urls[2]);
+            EXPECT_EQ(GURL("content://org.chromium.arc.chromecontentprovider/"
+                           "externalfile%3Adrive-user%252540gmail.com-hash%2Fa%"
+                           "2Fb%2Fc"),
+                      urls[2]);
             EXPECT_EQ(
                 GURL("content://org.chromium.arc.intent_helper.fileprovider/"
                      "external_files/a/b/c"),

chrome/browser/chromeos/fileapi/external_file_url_util.cc

@@ -45,13 +45,17 @@ base::FilePath ExternalFileURLToVirtualPath(const GURL& url) {
   if (!url.is_valid() || url.scheme() != content::kExternalFileScheme)
     return base::FilePath();
   const std::string path_string = net::UnescapeURLComponent(
-      url.GetContent(), net::UnescapeRule::NONASCII_SPACES);
+      url.path(),
+      net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
+          net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
+          net::UnescapeRule::NONASCII_SPACES);
   return base::FilePath::FromUTF8Unsafe(path_string);
 }
 
 GURL VirtualPathToExternalFileURL(const base::FilePath& virtual_path) {
-  return GURL(base::StringPrintf("%s:%s", content::kExternalFileScheme,
-                                 virtual_path.AsUTF8Unsafe().c_str()));
+  return GURL(
+      base::StringPrintf("%s:%s", content::kExternalFileScheme,
+                         net::EscapePath(virtual_path.AsUTF8Unsafe()).c_str()));
 }
 
 GURL CreateExternalFileURLFromPath(Profile* profile,

chrome/browser/chromeos/fileapi/external_file_url_util_unittest.cc

@@ -79,12 +79,30 @@ TEST_F(ExternalFileURLUtilTest, FilePathToExternalFileURL) {
                 .AsUTF8Unsafe());
 }
 
-TEST_F(ExternalFileURLUtilTest, VirtualPathToExternalFileURL) {
-  base::FilePath virtual_path(FILE_PATH_LITERAL("foo/bar012.txt"));
+// Tests that given virtual path is encoded to an expected externalfile: URL
+// and then the original path is reconstructed from it.
+void ExpectVirtualPathRoundtrip(
+    const base::FilePath::StringType& virtual_path_string,
+    std::string expected_url) {
+  base::FilePath virtual_path(virtual_path_string);
   GURL result = VirtualPathToExternalFileURL(virtual_path);
   EXPECT_TRUE(result.is_valid());
   EXPECT_EQ(content::kExternalFileScheme, result.scheme());
+  EXPECT_EQ(expected_url, result.path());
   EXPECT_EQ(virtual_path.value(), ExternalFileURLToVirtualPath(result).value());
 }
 
+TEST_F(ExternalFileURLUtilTest, VirtualPathToExternalFileURL) {
+  ExpectVirtualPathRoundtrip(FILE_PATH_LITERAL("foo/bar012.txt"),
+                             "foo/bar012.txt");
+
+  // Path containing precent character, which is also used for URL encoding.
+  ExpectVirtualPathRoundtrip(FILE_PATH_LITERAL("foo/bar012%41%.txt"),
+                             "foo/bar012%2541%25.txt");
+
+  // Path containing some ASCII characters that are escaped by URL enconding.
+  ExpectVirtualPathRoundtrip(FILE_PATH_LITERAL("foo/bar \"#<>?`{}.txt"),
+                             "foo/bar%20%22%23%3C%3E%3F%60%7B%7D.txt");
+}
+
 }  // namespace chromeos