Remove use of UnescapeRule::URL_SPECIAL_CHARS from extensions/file_util.

We're removing this, in favor of PATH_SEPARATORS and
URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS, to avoid any
security issues with this, going forward.

Both of the uses of URL_SPECIAL_CHARS in file_util.cc look problematic,
so modifying them to no longer escape paragraph separators, and adding
a pair of tests.

BUG=589257

Review URL: https://codereview.chromium.org/1760073003

Cr-Commit-Position: refs/heads/master@{#379291}

2 files changed, 39 insertions, 43 deletions

diff --git a/extensions/common/file_util.cc b/extensions/common/file_util.cc
index ae12f2c..830cc48 100644
--- a/extensions/common/file_util.cc
+++ b/extensions/common/file_util.cc
@@ -441,8 +441,10 @@ base::FilePath ExtensionURLToRelativeFilePath(const GURL& url) {
     return base::FilePath();
 
   // Drop the leading slashes and convert %-encoded UTF8 to regular UTF8.
-  std::string file_path = net::UnescapeURLComponent(url_path,
-      net::UnescapeRule::SPACES | net::UnescapeRule::URL_SPECIAL_CHARS);
+  std::string file_path = net::UnescapeURLComponent(
+      url_path,
+      net::UnescapeRule::SPACES |
+          net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS);
   size_t skip = file_path.find_first_not_of("/\\");
   if (skip != file_path.npos)
     file_path = file_path.substr(skip);
@@ -460,8 +462,10 @@ base::FilePath ExtensionURLToRelativeFilePath(const GURL& url) {
 
 base::FilePath ExtensionResourceURLToFilePath(const GURL& url,
                                               const base::FilePath& root) {
-  std::string host = net::UnescapeURLComponent(url.host(),
-      net::UnescapeRule::SPACES | net::UnescapeRule::URL_SPECIAL_CHARS);
+  std::string host = net::UnescapeURLComponent(
+      url.host(),
+      net::UnescapeRule::SPACES |
+          net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS);
   if (host.empty())
     return base::FilePath();
 
diff --git a/extensions/common/file_util_unittest.cc b/extensions/common/file_util_unittest.cc
index c058400..e63856c 100644
--- a/extensions/common/file_util_unittest.cc
+++ b/extensions/common/file_util_unittest.cc
@@ -454,26 +454,20 @@ TEST_F(FileUtilTest, ExtensionURLToRelativeFilePath) {
     const char* url;
     const char* expected_relative_path;
   } test_cases[] = {
-    { URL_PREFIX "simple.html",
-      "simple.html" },
-    { URL_PREFIX "directory/to/file.html",
-      "directory/to/file.html" },
-    { URL_PREFIX "escape%20spaces.html",
-      "escape spaces.html" },
-    { URL_PREFIX "%C3%9Cber.html",
-      "\xC3\x9C" "ber.html" },
+    {URL_PREFIX "simple.html", "simple.html"},
+    {URL_PREFIX "directory/to/file.html", "directory/to/file.html"},
+    {URL_PREFIX "escape%20spaces.html", "escape spaces.html"},
+    {URL_PREFIX "%C3%9Cber.html",
+     "\xC3\x9C"
+     "ber.html"},
 #if defined(OS_WIN)
-    { URL_PREFIX "C%3A/simple.html",
-      "" },
+    {URL_PREFIX "C%3A/simple.html", ""},
 #endif
-    { URL_PREFIX "////simple.html",
-      "simple.html" },
-    { URL_PREFIX "/simple.html",
-      "simple.html" },
-    { URL_PREFIX "\\simple.html",
-      "simple.html" },
-    { URL_PREFIX "\\\\foo\\simple.html",
-      "foo/simple.html" },
+    {URL_PREFIX "////simple.html", "simple.html"},
+    {URL_PREFIX "/simple.html", "simple.html"},
+    {URL_PREFIX "\\simple.html", "simple.html"},
+    {URL_PREFIX "\\\\foo\\simple.html", "foo/simple.html"},
+    {URL_PREFIX "..%2f..%2fsimple.html", "..%2f..%2fsimple.html"},
   };
 #undef URL_PREFIX
 
@@ -506,6 +500,10 @@ TEST_F(FileUtilTest, ExtensionResourceURLToFilePath) {
   ASSERT_TRUE(base::WriteFile(resource_path, data, sizeof(data)));
   resource_path = api_path.Append(FILE_PATH_LITERAL("escape spaces.js"));
   ASSERT_TRUE(base::WriteFile(resource_path, data, sizeof(data)));
+  resource_path = api_path.Append(FILE_PATH_LITERAL("escape spaces.js"));
+  ASSERT_TRUE(base::WriteFile(resource_path, data, sizeof(data)));
+  resource_path = api_path.Append(FILE_PATH_LITERAL("..%2f..%2fsimple.html"));
+  ASSERT_TRUE(base::WriteFile(resource_path, data, sizeof(data)));
 
 #ifdef FILE_PATH_USES_WIN_SEPARATORS
 #define SEP "\\"
@@ -517,27 +515,21 @@ TEST_F(FileUtilTest, ExtensionResourceURLToFilePath) {
     const char* url;
     const base::FilePath::CharType* expected_path;
   } test_cases[] = {
-    { URL_PREFIX "apiname/test.js",
-      FILE_PATH_LITERAL("test.js") },
-    { URL_PREFIX "/apiname/test.js",
-      FILE_PATH_LITERAL("test.js") },
-    // Test % escape
-    { URL_PREFIX "apiname/%74%65st.js",
-      FILE_PATH_LITERAL("test.js") },
-    { URL_PREFIX "apiname/escape%20spaces.js",
-      FILE_PATH_LITERAL("escape spaces.js") },
-    // Test file does not exist.
-    { URL_PREFIX "apiname/directory/to/file.js",
-      NULL },
-    // Test apiname/../../test.js
-    { URL_PREFIX "apiname/../../test.js",
-      FILE_PATH_LITERAL("test.js") },
-    { URL_PREFIX "apiname/..%2F../test.js",
-      NULL },
-    { URL_PREFIX "apiname/f/../../../test.js",
-      FILE_PATH_LITERAL("test.js") },
-    { URL_PREFIX "apiname/f%2F..%2F..%2F../test.js",
-      NULL },
+      {URL_PREFIX "apiname/test.js", FILE_PATH_LITERAL("test.js")},
+      {URL_PREFIX "/apiname/test.js", FILE_PATH_LITERAL("test.js")},
+      // Test % escape
+      {URL_PREFIX "apiname/%74%65st.js", FILE_PATH_LITERAL("test.js")},
+      {URL_PREFIX "apiname/escape%20spaces.js",
+       FILE_PATH_LITERAL("escape spaces.js")},
+      // Test file does not exist.
+      {URL_PREFIX "apiname/directory/to/file.js", NULL},
+      // Test apiname/../../test.js
+      {URL_PREFIX "apiname/../../test.js", FILE_PATH_LITERAL("test.js")},
+      {URL_PREFIX "apiname/..%2F../test.js", NULL},
+      {URL_PREFIX "apiname/f/../../../test.js", FILE_PATH_LITERAL("test.js")},
+      {URL_PREFIX "apiname/f%2F..%2F..%2F../test.js", NULL},
+      {URL_PREFIX "apiname/..%2f..%2fsimple.html",
+       FILE_PATH_LITERAL("..%2f..%2fsimple.html")},
   };
 #undef SEP
 #undef URL_PREFIX