Unescape all escaped characters in externalfile: URL.

Resolve error opening files whose name contain blacklisted characters
for URL spoofing, such as "🔒" (U+1F512).

This change also removes the tentative hack at crrev.com/c/1085207 for
crbug.com/847039.


Bug: 847039,849998
Test: unit_tests --gtest_filter=ExternalFileURLUtilTest.*:FileManagerPath.*
Test: net_unittests --gtest_filter=EscapeTest.*
Change-Id: Icd8ba4c73f09b97e05c9af182d338e709d5876f6
Reviewed-on: https://chromium-review.googlesource.com/1142854
Commit-Queue: Asanka Herath <asanka@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579543}

chrome/browser/chromeos/fileapi/external_file_url_util.cc

@@ -44,11 +44,8 @@ GURL FileSystemURLToExternalFileURL(
 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.path(),
-      net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
-          net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-          net::UnescapeRule::NONASCII_SPACES);
+  const std::string path_string =
+      net::UnescapeBinaryURLComponent(url.path(), net::UnescapeRule::NORMAL);
   return base::FilePath::FromUTF8Unsafe(path_string);
 }
 

chrome/browser/chromeos/fileapi/external_file_url_util_unittest.cc

@@ -103,6 +103,14 @@ TEST_F(ExternalFileURLUtilTest, VirtualPathToExternalFileURL) {
   // 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");
+
+  // (U+3000) IDEOGRAPHIC SPACE and (U+1F512) LOCK are examples of characters
+  // potentially used for URL spoofing. Those are blacklisted from unescaping
+  // when a URL is displayed, but this should not prevent it from being
+  // unescaped when converting a URL to a virtual file path. See
+  // crbug.com/585422 for detail.
+  ExpectVirtualPathRoundtrip(FILE_PATH_LITERAL("foo/bar/space\u3000lock🔒.zip"),
+                             "foo/bar/space%E3%80%80lock%F0%9F%94%92.zip");
 }
 
 }  // namespace chromeos

net/base/escape.cc

@@ -223,8 +223,7 @@ bool ShouldUnescapeCodePoint(UnescapeRule::Type rules, uint32_t code_point) {
       code_point == 0x1F513 ||  // OPEN LOCK            (%F0%9F%94%93)
 
       // Spaces are also banned, as they can be used to scroll text out of view.
-      (!(rules & UnescapeRule::NONASCII_SPACES) &&
-       (code_point == 0x0085 ||  // NEXT LINE                  (%C2%85)
+      code_point == 0x0085 ||  // NEXT LINE                  (%C2%85)
       code_point == 0x00A0 ||  // NO-BREAK SPACE             (%C2%A0)
       code_point == 0x1680 ||  // OGHAM SPACE MARK           (%E1%9A%80)
       code_point == 0x2000 ||  // EN QUAD                    (%E2%80%80)
@@ -242,8 +241,7 @@ bool ShouldUnescapeCodePoint(UnescapeRule::Type rules, uint32_t code_point) {
       code_point == 0x2029 ||  // PARAGRAPH SEPARATOR        (%E2%80%A9)
       code_point == 0x202F ||  // NARROW NO-BREAK SPACE      (%E2%80%AF)
       code_point == 0x205F ||  // MEDIUM MATHEMATICAL SPACE  (%E2%81%9F)
-        code_point == 0x3000     // IDEOGRAPHIC SPACE          (%E3%80%80)
-        )));
+      code_point == 0x3000);   // IDEOGRAPHIC SPACE          (%E3%80%80)
 }
 
 // Unescapes |escaped_text| according to |rules|, returning the resulting

net/base/escape.h

@@ -102,9 +102,6 @@ class UnescapeRule {
 
     // URL queries use "+" for space. This flag controls that replacement.
     REPLACE_PLUS_WITH_SPACE = 1 << 4,
-
-    // Unescapes space characters that appears as plain blank in visual agents.
-    NONASCII_SPACES = 1 << 5,
   };
 };
 

net/base/escape_unittest.cc

@@ -246,17 +246,6 @@ TEST(EscapeTest, UnescapeURLComponent) {
        UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS,
        "%01%02%03%04%05%06%07%08%09 %"},
       {"Hello%20%13%10%02", UnescapeRule::SPACES, "Hello %13%10%02"},
-      // Ideographic space unescaped only if the NONASCII_SPACES flag is set.
-      {"日本語%E3%80%80日本語", UnescapeRule::NONASCII_SPACES,
-       "日本語　日本語"},
-      // Ideographic space remains escaped if the NONASCII_SPACES flag is not
-      // specified.
-      {"日本語%E3%80%80日本語", UnescapeRule::NORMAL, "日本語%E3%80%80日本語"},
-      {"日本語%E3%80%80日本語",
-       UnescapeRule::SPACES | UnescapeRule::PATH_SEPARATORS |
-           UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-           UnescapeRule::REPLACE_PLUS_WITH_SPACE,
-       "日本語%E3%80%80日本語"},
 
       // '/' and '\\' should only be unescaped by PATH_SEPARATORS.
       {"%2F%5C", UnescapeRule::PATH_SEPARATORS, "/\\"},