FileURLToFilePath: Don't unescape '/' and '\\'.

net/base/escape.h

Index: net/base/escape.h
diff --git a/net/base/escape.h b/net/base/escape.h
index c31dcf9aa2250899e76da977b168a9aafc82c596..f75d4502bc311b9a2cf24b71d53ed54241ce5b64 100644
--- a/net/base/escape.h
+++ b/net/base/escape.h
@@ -93,6 +93,12 @@ class UnescapeRule {
     // interpreting as a URL and want to do as much unescaping as possible.
     URL_SPECIAL_CHARS = 4,
 
+    // Subset of "URL_SPECIAL_CHARS" - excludes '/' and '\\'. For use with file
+    // URLs.
+    // TODO(mmenke): Should GURL unescape those two characters for file URLs?
+    // that's what FireFox and IE do.

[eroman, 2016/02/18 19:37:28]

Worth noting the obvious, that there are compatibility and web-security
ramifications to changing this for general URLs.

Apache for instance by default does not unescape these in paths:
http://httpd.apache.org/docs/current/mod/core.html#allowencodedslashes

Web servers may be checking for directory escaping in urls by just ../ and not
the escaped variants, so if Chrome started transforming could be problematic.

As I understand this comment though it is specifically for file URLs, so sounds
fine.

[mmenke, 2016/02/18 20:30:22]

Actually, I'm thinking that the compatibility and web-security ramifications are
more around *not* using this for URLs (Of any type), and that's where the scary
comment should go instead.  Unescaping slashes on GURLs or GURL components for
any reason seems like something that's generally not a good idea, with the
exception of Javascript URLs.  We mostly unescape for either display, or picking
local file names (On file or http URLs).  For display, it changes the semantic
meaning of a URL, so seems like a bad idea.  For local files, it seems like a
potential attack vector, regardless of whether we're talking file or http URLs.

I think we should get rid of URL_SPECIAL_CHARS in favor of this, and a separate
flag just for path characters - I've added a TODO about that, and a warning to
URL_SPECIAL_CHARS.

+    URL_SPECIAL_CHARS_EXCEPT_PATH_SEPERATORS = 8,
+
     // Unescapes characters that can be used in spoofing attempts (such as LOCK)
     // and control characters (such as BiDi control characters and %01).  This
     // INCLUDES NULLs.  This is used for rare cases such as data: URL decoding
@@ -100,10 +106,10 @@ class UnescapeRule {
     //
     // DO NOT use SPOOFING_AND_CONTROL_CHARS if the URL is going to be displayed
     // in the UI for security reasons.
-    SPOOFING_AND_CONTROL_CHARS = 8,
+    SPOOFING_AND_CONTROL_CHARS = 16,

[eroman, 2016/02/18 19:37:28]

optional style: I find the 1 << X notation simpler to edit for bit flags.

[mmenke, 2016/02/18 20:30:22]

SGTM, done.

 
     // URL queries use "+" for space. This flag controls that replacement.
-    REPLACE_PLUS_WITH_SPACE = 16,
+    REPLACE_PLUS_WITH_SPACE = 32,
   };
 };