Fix net::FilePathToFileURL to correctly handle control chars

While implementing file drag and drop, I have discovered that
net::FilePathToFileURL does not correctly handle paths which end with a
space char.

GURL does some modifications to the path in its constructor so we must
percent-encode control chars before calling it to avoid corruption.

Bug: 1144138
Change-Id: Ie8e601edbc42033e33389ff4bd48046d16571ab8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2522735
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825587}

content/renderer/dom_serializer_browsertest.cc

@@ -690,9 +690,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_DomSerializerTests, SerializeHTMLDOMWithBaseTag) {
 
   // Get page dir URL which is base URL of this file.
   base::FilePath dir_name = page_file_path.DirName();
-  dir_name = dir_name.Append(
-      base::FilePath::StringType(base::FilePath::kSeparators[0], 1));
-  GURL path_dir_url = net::FilePathToFileURL(dir_name);
+  GURL path_dir_url = net::FilePathToFileURL(dir_name.AsEndingWithSeparator());
 
   // Get file URL.
   GURL file_url = net::FilePathToFileURL(page_file_path);

net/base/filename_util.cc

@@ -32,31 +32,26 @@ GURL FilePathToFileURL(const base::FilePath& path) {
   // "file://///server/path" for UNC. The URL canonicalizer will fix up the
   // latter case to be the canonical UNC form: "file://server/path"
   base::FilePath::StringType url_string(kFileURLPrefix);
-  url_string.append(path.value());
 
-  // Now do replacement of some characters. Since we assume the input is a
-  // literal filename, anything the URL parser might consider special should
-  // be escaped here.
-
-  // must be the first substitution since others will introduce percents as the
-  // escape character
-  base::ReplaceSubstringsAfterOffset(
-      &url_string, 0, FILE_PATH_LITERAL("%"), FILE_PATH_LITERAL("%25"));
-
-  // semicolon is supposed to be some kind of separator according to RFC 2396
-  base::ReplaceSubstringsAfterOffset(
-      &url_string, 0, FILE_PATH_LITERAL(";"), FILE_PATH_LITERAL("%3B"));
-
-  base::ReplaceSubstringsAfterOffset(
-      &url_string, 0, FILE_PATH_LITERAL("#"), FILE_PATH_LITERAL("%23"));
-
-  base::ReplaceSubstringsAfterOffset(
-      &url_string, 0, FILE_PATH_LITERAL("?"), FILE_PATH_LITERAL("%3F"));
+  // GURL() strips some whitespace and trailing control chars which are valid
+  // in file paths. It also interprets chars such as `%;#?` and maybe `\`, so we
+  // must percent encode these first. Reserve max possible length up front.
+  url_string.reserve(url_string.size() + (3 * path.value().size()));
 
+  for (auto c : path.value()) {
+    if (c == '%' || c == ';' || c == '#' || c == '?' ||
 #if defined(OS_POSIX) || defined(OS_FUCHSIA)
-  base::ReplaceSubstringsAfterOffset(
-      &url_string, 0, FILE_PATH_LITERAL("\\"), FILE_PATH_LITERAL("%5C"));
+        c == '\\' ||
 #endif
+        c <= ' ') {
+      static const char kHexChars[] = "0123456789ABCDEF";
+      url_string += '%';
+      url_string += kHexChars[(c >> 4) & 0xf];
+      url_string += kHexChars[c & 0xf];
+    } else {
+      url_string += c;
+    }
+  }
 
   return GURL(base::AsCrossPlatformPiece(url_string));
 }

net/base/filename_util_unittest.cc

@@ -192,13 +192,15 @@ TEST(FilenameUtilTest, FileURLConversion) {
     // Other percent-encoded characters that are left alone when displaying a
     // URL are decoded in a file path (https://crbug.com/585422).
     {L"C:\\foo\\\U0001F512.txt",
-     "file:///C:/foo/%F0%9F%94%92.txt"},                       // Blocked.
-    {L"C:\\foo\\\u2001.txt", "file:///C:/foo/%E2%80%81.txt"},  // Blocked.
+     "file:///C:/foo/%F0%9F%94%92.txt"},                         // Blocked.
+    {L"C:\\foo\\\u2001.txt", "file:///C:/foo/%E2%80%81.txt"},    // Blocked.
+    {L"C:\\foo\\\a\tbar\n ", "file:///C:/foo/%07%09bar%0A%20"},  // Blocked.
 #elif defined(OS_POSIX) || defined(OS_FUCHSIA)
     {L"/foo/bar.txt", "file:///foo/bar.txt"},
     {L"/foo/BAR.txt", "file:///foo/BAR.txt"},
     {L"/C:/foo/bar.txt", "file:///C:/foo/bar.txt"},
     {L"/foo/bar?.txt", "file:///foo/bar%3F.txt"},
+    {L"/foo/\a\tbar\n ", "file:///foo/%07%09bar%0A%20"},
     // %5C ('\\') is not special on POSIX, and is therefore decoded as normal.
     {L"/foo/..\\bar", "file:///foo/..%5Cbar"},
     {L"/some computer/foo/bar.txt", "file:///some%20computer/foo/bar.txt"},