Separate out methods to unescape URLs for use as binary data and as text

In a followup CL, invalid UTF-8 characters will never be unescaped by
the text methods.

Bug: 829868
Change-Id: I71df885005f5faa9f72ac5ff6cfb2789c64ae846
Reviewed-on: https://chromium-review.googlesource.com/1024771Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554562}

components/policy/core/common/cloud/device_management_service_unittest.cc

@@ -439,24 +439,14 @@ class QueryParams {
   bool Check(const std::string& name, const std::string& expected_value) {
     bool found = false;
     for (ParamMap::const_iterator i(params_.begin()); i != params_.end(); ++i) {
-      std::string unescaped_name(net::UnescapeURLComponent(
-          i->first,
-          net::UnescapeRule::NORMAL | net::UnescapeRule::SPACES |
-              net::UnescapeRule::PATH_SEPARATORS |
-              net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-              net::UnescapeRule::SPOOFING_AND_CONTROL_CHARS |
-              net::UnescapeRule::REPLACE_PLUS_WITH_SPACE));
+      std::string unescaped_name(net::UnescapeBinaryURLComponent(
+          i->first, net::UnescapeRule::REPLACE_PLUS_WITH_SPACE));
       if (unescaped_name == name) {
         if (found)
           return false;
         found = true;
-        std::string unescaped_value(net::UnescapeURLComponent(
-            i->second,
-            net::UnescapeRule::NORMAL | net::UnescapeRule::SPACES |
-                net::UnescapeRule::PATH_SEPARATORS |
-                net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-                net::UnescapeRule::SPOOFING_AND_CONTROL_CHARS |
-                net::UnescapeRule::REPLACE_PLUS_WITH_SPACE));
+        std::string unescaped_value(net::UnescapeBinaryURLComponent(
+            i->second, net::UnescapeRule::REPLACE_PLUS_WITH_SPACE));
         if (unescaped_value != expected_value)
           return false;
       }

components/safe_browsing/db/v4_protocol_manager_util.cc

@@ -38,11 +38,7 @@ std::string Unescape(const std::string& url) {
   int loop_var = 0;
   do {
     old_size = unescaped_str.size();
-    unescaped_str = net::UnescapeURLComponent(
-        unescaped_str,
-        net::UnescapeRule::SPOOFING_AND_CONTROL_CHARS |
-            net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
-            net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS);
+    unescaped_str = net::UnescapeBinaryURLComponent(unescaped_str);
   } while (old_size != unescaped_str.size() &&
            ++loop_var <= kMaxLoopIterations);
 

content/browser/web_contents/web_drag_source_mac.mm

@@ -145,12 +145,8 @@ void PromiseWriterHelper(const DropData& drop_data,
     // If NSURL creation failed, check for a badly-escaped JavaScript URL.
     // Strip out any existing escapes and then re-escape uniformly.
     if (!url && dropData_->url.SchemeIs(url::kJavaScriptScheme)) {
-      net::UnescapeRule::Type unescapeRules =
-          net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
-          net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-          net::UnescapeRule::SPOOFING_AND_CONTROL_CHARS;
       std::string unescapedUrlString =
-          net::UnescapeURLComponent(dropData_->url.spec(), unescapeRules);
+          net::UnescapeBinaryURLComponent(dropData_->url.spec());
       std::string escapedUrlString =
           net::EscapeUrlEncodedData(unescapedUrlString, false);
       url = [NSURL URLWithString:SysUTF8ToNSString(escapedUrlString)];

extensions/browser/api/web_request/form_data_parser.cc

@@ -100,7 +100,6 @@ class FormDataParserUrlEncoded : public FormDataParser {
   // Auxiliary constant for using RE2. Number of arguments for parsing
   // name-value pairs (one for name, one for value).
   static const size_t args_size_ = 2u;
-  static const net::UnescapeRule::Type unescape_rules_;
 
   re2::StringPiece source_;
   bool source_set_;
@@ -369,12 +368,6 @@ std::unique_ptr<FormDataParser> FormDataParser::CreateFromContentTypeHeader(
 
 FormDataParser::FormDataParser() {}
 
-const net::UnescapeRule::Type FormDataParserUrlEncoded::unescape_rules_ =
-    net::UnescapeRule::PATH_SEPARATORS |
-    net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-    net::UnescapeRule::SPOOFING_AND_CONTROL_CHARS | net::UnescapeRule::SPACES |
-    net::UnescapeRule::REPLACE_PLUS_WITH_SPACE;
-
 FormDataParserUrlEncoded::FormDataParserUrlEncoded()
     : source_(NULL),
       source_set_(false),
@@ -399,9 +392,12 @@ bool FormDataParserUrlEncoded::GetNextNameValue(Result* result) {
 
   bool success = RE2::ConsumeN(&source_, pattern(), args_, args_size_);
   if (success) {
-    result->set_name(net::UnescapeURLComponent(name_, unescape_rules_));
+    const net::UnescapeRule::Type kUnescapeRules =
+        net::UnescapeRule::REPLACE_PLUS_WITH_SPACE;
+
+    result->set_name(net::UnescapeBinaryURLComponent(name_, kUnescapeRules));
     const std::string unescaped_value =
-        net::UnescapeURLComponent(value_, unescape_rules_);
+        net::UnescapeBinaryURLComponent(value_, kUnescapeRules);
     const base::StringPiece unescaped_data(unescaped_value.data(),
                                            unescaped_value.length());
     if (base::IsStringUTF8(unescaped_data)) {
@@ -550,11 +546,7 @@ bool FormDataParserMultipart::GetNextNameValue(Result* result) {
     return_value = FinishReadingPart(value_assigned ? nullptr : &value);
   }
 
-  std::string unescaped_name = net::UnescapeURLComponent(
-      name.as_string(),
-      net::UnescapeRule::PATH_SEPARATORS |
-          net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-          net::UnescapeRule::SPOOFING_AND_CONTROL_CHARS);
+  std::string unescaped_name = net::UnescapeBinaryURLComponent(name);
   result->set_name(unescaped_name);
   if (value_assigned) {
     // Hold filename as value.

net/base/data_url.cc

@@ -103,10 +103,7 @@ bool DataURL::Parse(const GURL& url,
   // of the data, and should be stripped. Otherwise, the escaped whitespace
   // could be part of the payload, so don't strip it.
   if (base64_encoded) {
-    temp_data = UnescapeURLComponent(
-        temp_data, UnescapeRule::SPACES | UnescapeRule::PATH_SEPARATORS |
-                       UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-                       UnescapeRule::SPOOFING_AND_CONTROL_CHARS);
+    temp_data = UnescapeBinaryURLComponent(temp_data);
   }
 
   // Strip whitespace.
@@ -116,10 +113,7 @@ bool DataURL::Parse(const GURL& url,
   }
 
   if (!base64_encoded) {
-    temp_data = UnescapeURLComponent(
-        temp_data, UnescapeRule::SPACES | UnescapeRule::PATH_SEPARATORS |
-                       UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-                       UnescapeRule::SPOOFING_AND_CONTROL_CHARS);
+    temp_data = UnescapeBinaryURLComponent(temp_data);
   }
 
   if (base64_encoded) {

net/base/escape.cc

@@ -179,20 +179,9 @@ bool ShouldUnescapeCodePoint(UnescapeRule::Type rules, uint32_t code_point) {
            ((code_point == '/' || code_point == '\\') &&
             (rules & UnescapeRule::PATH_SEPARATORS)) ||
            (code_point > ' ' && code_point != '/' && code_point != '\\' &&
-            (rules & UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS)) ||
-           // Additionally allow non-display characters if requested.
-           (code_point < ' ' &&
-            (rules & UnescapeRule::SPOOFING_AND_CONTROL_CHARS));
+            (rules & UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS));
   }
 
-  // Some schemes such as data: and file: need to parse the exact binary data
-  // when loading the URL. For that reason, SPOOFING_AND_CONTROL_CHARS allows
-  // unescaping UTF-8 byte sequences that are not safe to display. DO NOT use
-  // SPOOFING_AND_CONTROL_CHARS if the parsed URL is going to be displayed in
-  // the UI.
-  if (rules & UnescapeRule::SPOOFING_AND_CONTROL_CHARS)
-    return true;
-
   // Compare the codepoint against a list of characters that can be used
   // to spoof other URLs.
   //
@@ -284,9 +273,8 @@ std::string UnescapeURLWithAdjustmentsImpl(
       // character. In that case, just unescaped and write the non-sense
       // character.
       //
-      // TODO(https://crbug.com/829868): Do not unescape illegal UTF-8 sequences
-      // unless SPOOFING_AND_CONTROL_CHARS is given. Should also split that
-      // behaviour off into a separate function.
+      // TODO(https://crbug.com/829868): Do not unescape illegal UTF-8
+      // sequences.
       unsigned char non_utf8_byte;
       if (UnescapeUnsignedByteAtIndex(escaped_text, i, &non_utf8_byte)) {
         result.push_back(non_utf8_byte);
@@ -482,6 +470,43 @@ base::string16 UnescapeAndDecodeUTF8URLComponentWithAdjustments(
   return base::UTF8ToUTF16WithAdjustments(text, adjustments);
 }
 
+std::string UnescapeBinaryURLComponent(base::StringPiece escaped_text,
+                                       UnescapeRule::Type rules) {
+  // Only NORMAL and REPLACE_PLUS_WITH_SPACE are supported.
+  DCHECK(rules != UnescapeRule::NONE);
+  DCHECK(!(rules &
+           ~(UnescapeRule::NORMAL | UnescapeRule::REPLACE_PLUS_WITH_SPACE)));
+
+  // The output of the unescaping is always smaller than the input, so we can
+  // reserve the input size to make sure we have enough buffer and don't have
+  // to allocate in the loop below.
+  std::string result;
+  result.reserve(escaped_text.length());
+
+  for (size_t i = 0, max = escaped_text.size(); i < max;) {
+    unsigned char byte;
+    // UnescapeUnsignedByteAtIndex does bounds checking, so this is always safe
+    // to call.
+    if (UnescapeUnsignedByteAtIndex(escaped_text, i, &byte)) {
+      result.push_back(byte);
+      i += 3;
+      continue;
+    }
+
+    if ((rules & UnescapeRule::REPLACE_PLUS_WITH_SPACE) &&
+        escaped_text[i] == '+') {
+      result.push_back(' ');
+      ++i;
+      continue;
+    }
+
+    result.push_back(escaped_text[i]);
+    ++i;
+  }
+
+  return result;
+}
+
 base::string16 UnescapeForHTML(base::StringPiece16 input) {
   static const struct {
     const char* ampersand_code;

net/base/escape.h

@@ -81,8 +81,7 @@ class UnescapeRule {
     // Convert %20 to spaces. In some places where we're showing URLs, we may
     // want this. In places where the URL may be copied and pasted out, then
     // you wouldn't want this since it might not be interpreted in one piece
-    // by other applications.  Other unicode spaces will not be unescaped unless
-    // SPOOFING_AND_CONTROL_CHARS is used.
+    // by other applications.  Other UTF-8 spaces will not be unescaped.
     SPACES = 1 << 1,
 
     // Unescapes '/' and '\\'. If these characters were unescaped, the resulting
@@ -100,17 +99,8 @@ class UnescapeRule {
     // as much unescaping as possible.
     URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS = 1 << 3,
 
-    // 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
-    // where the result is binary data.
-    //
-    // 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 = 1 << 4,
-
     // URL queries use "+" for space. This flag controls that replacement.
-    REPLACE_PLUS_WITH_SPACE = 1 << 5,
+    REPLACE_PLUS_WITH_SPACE = 1 << 4,
   };
 };
 
@@ -121,10 +111,11 @@ class UnescapeRule {
 // "UnescapeRule::SPACES" used.
 //
 // This method does not ensure that the output is a valid string using any
-// character encoding. However, unless SPOOFING_AND_CONTROL_CHARS is set, it
-// does leave escaped certain byte sequences that would be dangerous to display
-// to the user, because if interpreted as UTF-8, they could be used to mislead
-// the user.
+// character encoding. However, it does leave escaped certain byte sequences
+// that would be dangerous to display to the user, because if interpreted as
+// UTF-8, they could be used to mislead the user. Callers that want to
+// unconditionally unescape everything for uses other than displaying data to
+// the user should use UnescapeBinaryURLComponent().
 NET_EXPORT std::string UnescapeURLComponent(base::StringPiece escaped_text,
                                             UnescapeRule::Type rules);
 
@@ -142,6 +133,16 @@ NET_EXPORT base::string16 UnescapeAndDecodeUTF8URLComponentWithAdjustments(
     UnescapeRule::Type rules,
     base::OffsetAdjuster::Adjustments* adjustments);
 
+// Unescapes a component of a URL for use as binary data. Unlike
+// UnescapeURLComponent, leaves nothing unescaped, including nulls, invalid
+// characters, characters that are unsafe to display, etc. This should *not*
+// be used when displaying the decoded data to the user.
+//
+// Only the NORMAL and REPLACE_PLUS_WITH_SPACE rules are allowed.
+NET_EXPORT std::string UnescapeBinaryURLComponent(
+    base::StringPiece escaped_text,
+    UnescapeRule::Type rules = UnescapeRule::NORMAL);
+
 // Unescapes the following ampersand character codes from |text|:
 // &lt; &gt; &amp; &quot; &#39;
 NET_EXPORT base::string16 UnescapeForHTML(base::StringPiece16 text);

net/base/escape_unittest.cc

@@ -160,9 +160,7 @@ TEST(EscapeTest, UnescapeURLComponent) {
       {"Some%20random text %25%E1%A6%99OK", UnescapeRule::NORMAL,
        "Some%20random text %25\xE1\xA6\x99OK"},
 
-      // BiDi Control characters should not be unescaped unless explicity told
-      // to
-      // do so with UnescapeRule::SPOOFING_AND_CONTROL_CHARS
+      // BiDi Control characters should not be unescaped.
       {"Some%20random text %25%D8%9COK", UnescapeRule::NORMAL,
        "Some%20random text %25%D8%9COK"},
       {"Some%20random text %25%E2%80%8EOK", UnescapeRule::NORMAL,
@@ -179,36 +177,8 @@ TEST(EscapeTest, UnescapeURLComponent) {
        "Some%20random text %25%E2%81%A6OK"},
       {"Some%20random text %25%E2%81%A9OK", UnescapeRule::NORMAL,
        "Some%20random text %25%E2%81%A9OK"},
-      // UnescapeRule::SPOOFING_AND_CONTROL_CHARS should unescape BiDi Control
-      // characters.
-      {"Some%20random text %25%D8%9COK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xD8\x9COK"},
-      {"Some%20random text %25%E2%80%8EOK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xE2\x80\x8EOK"},
-      {"Some%20random text %25%E2%80%8FOK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xE2\x80\x8FOK"},
-      {"Some%20random text %25%E2%80%AAOK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xE2\x80\xAAOK"},
-      {"Some%20random text %25%E2%80%ABOK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xE2\x80\xABOK"},
-      {"Some%20random text %25%E2%80%AEOK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xE2\x80\xAEOK"},
-      {"Some%20random text %25%E2%81%A6OK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xE2\x81\xA6OK"},
-      {"Some%20random text %25%E2%81%A9OK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xE2\x81\xA9OK"},
-
-      // Certain banned characters should not be unescaped unless explicitly
-      // told
-      // to do so with UnescapeRule::SPOOFING_AND_CONTROL_CHARS.
+
+      // Certain banned characters should not be unescaped.
       // U+1F50F LOCK WITH INK PEN
       {"Some%20random text %25%F0%9F%94%8FOK", UnescapeRule::NORMAL,
        "Some%20random text %25%F0%9F%94%8FOK"},
@@ -221,20 +191,6 @@ TEST(EscapeTest, UnescapeURLComponent) {
       // U+1F513 OPEN LOCK
       {"Some%20random text %25%F0%9F%94%93OK", UnescapeRule::NORMAL,
        "Some%20random text %25%F0%9F%94%93OK"},
-      // UnescapeRule::SPOOFING_AND_CONTROL_CHARS should unescape banned
-      // characters.
-      {"Some%20random text %25%F0%9F%94%8FOK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xF0\x9F\x94\x8FOK"},
-      {"Some%20random text %25%F0%9F%94%90OK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xF0\x9F\x94\x90OK"},
-      {"Some%20random text %25%F0%9F%94%92OK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xF0\x9F\x94\x92OK"},
-      {"Some%20random text %25%F0%9F%94%93OK",
-       UnescapeRule::NORMAL | UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Some%20random text %25\xF0\x9F\x94\x93OK"},
 
       // Spaces
       {"(%C2%85)(%C2%A0)(%E1%9A%80)(%E2%80%80)", UnescapeRule::NORMAL,
@@ -289,23 +245,10 @@ TEST(EscapeTest, UnescapeURLComponent) {
       {"%01%02%03%04%05%06%07%08%09 %25",
        UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS,
        "%01%02%03%04%05%06%07%08%09 %"},
-      {"%01%02%03%04%05%06%07%08%09 %25",
-       UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "\x01\x02\x03\x04\x05\x06\x07\x08\x09 %25"},
       {"Hello%20%13%10%02", UnescapeRule::SPACES, "Hello %13%10%02"},
-      {"Hello%20%13%10%02", UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Hello%20\x13\x10\x02"},
-      {"Hello\xE9\xA0\xA4\xE9\xA0\xA7",
-       UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "Hello\xE9\xA0\xA4\xE9\xA0\xA7"},
 
       // '/' and '\\' should only be unescaped by PATH_SEPARATORS.
       {"%2F%5C", UnescapeRule::PATH_SEPARATORS, "/\\"},
-      {"%2F%5C",
-       UnescapeRule::SPACES |
-           UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-           UnescapeRule::SPOOFING_AND_CONTROL_CHARS,
-       "%2F%5C"},
   };
 
   for (const auto unescape_case : kUnescapeCases) {
@@ -313,22 +256,13 @@ TEST(EscapeTest, UnescapeURLComponent) {
               UnescapeURLComponent(unescape_case.input, unescape_case.rules));
   }
 
-  // Test the NULL character unescaping (which wouldn't work above since those
-  // are just char pointers).
+  // Test NULL character unescaping, which can't be tested above since those are
+  // just char pointers.
   std::string input("Null");
   input.push_back(0);  // Also have a NULL in the input.
   input.append("%00%39Test");
 
-  // When we're unescaping NULLs
-  std::string expected("Null");
-  expected.push_back(0);
-  expected.push_back(0);
-  expected.append("9Test");
-  EXPECT_EQ(expected, UnescapeURLComponent(
-                          input, UnescapeRule::SPOOFING_AND_CONTROL_CHARS));
-
-  // When we're not unescaping NULLs.
-  expected = "Null";
+  std::string expected = "Null";
   expected.push_back(0);
   expected.append("%009Test");
   EXPECT_EQ(expected, UnescapeURLComponent(input, UnescapeRule::NORMAL));
@@ -449,6 +383,51 @@ TEST(EscapeTest, AdjustOffset) {
   }
 }
 
+TEST(EscapeTest, UnescapeBinaryURLComponent) {
+  const UnescapeURLCase kTestCases[] = {
+      // Check that ASCII characters with special handling in
+      // UnescapeURLComponent() are still unescaped.
+      {"%09%20%25foo%2F", UnescapeRule::NORMAL, "\x09 %foo/"},
+
+      // UTF-8 Characters banned by UnescapeURLComponent() should also be
+      // unescaped.
+      {"Some random text %D8%9COK", UnescapeRule::NORMAL,
+       "Some random text \xD8\x9COK"},
+      {"Some random text %F0%9F%94%8FOK", UnescapeRule::NORMAL,
+       "Some random text \xF0\x9F\x94\x8FOK"},
+
+      // As should invalid UTF-8 characters.
+      {"%A0%A0%E9%E9%A0%A0%A0%A0", UnescapeRule::NORMAL,
+       "\xA0\xA0\xE9\xE9\xA0\xA0\xA0\xA0"},
+
+      // And valid UTF-8 characters that are not banned by
+      // UnescapeURLComponent() should be unescaped, too!
+      {"%C2%A1%C2%A1", UnescapeRule::NORMAL, "\xC2\xA1\xC2\xA1"},
+
+      // '+' should be left alone by default
+      {"++%2B++", UnescapeRule::NORMAL, "+++++"},
+      // But should magically be turned into a space if requested.
+      {"++%2B++", UnescapeRule::REPLACE_PLUS_WITH_SPACE, "  +  "},
+  };
+
+  for (const auto& test_case : kTestCases) {
+    EXPECT_EQ(std::string(test_case.output),
+              UnescapeBinaryURLComponent(test_case.input, test_case.rules));
+  }
+
+  // Test NULL character unescaping, which can't be tested above since those are
+  // just char pointers.
+  std::string input("Null");
+  input.push_back(0);  // Also have a NULL in the input.
+  input.append("%00%39Test");
+
+  std::string expected("Null");
+  expected.push_back(0);
+  expected.push_back(0);
+  expected.append("9Test");
+  EXPECT_EQ(expected, UnescapeBinaryURLComponent(input));
+}
+
 TEST(EscapeTest, EscapeForHTML) {
   const EscapeForHTMLCase tests[] = {
     { "hello", "hello" },

net/test/embedded_test_server/default_handlers.cc

@@ -41,12 +41,6 @@ namespace net {
 namespace test_server {
 namespace {
 
-const UnescapeRule::Type kUnescapeAll =
-    UnescapeRule::SPACES | UnescapeRule::PATH_SEPARATORS |
-    UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-    UnescapeRule::SPOOFING_AND_CONTROL_CHARS |
-    UnescapeRule::REPLACE_PLUS_WITH_SPACE;
-
 const char kDefaultRealm[] = "testrealm";
 const char kDefaultPassword[] = "secret";
 const char kEtag[] = "abc";
@@ -254,7 +248,8 @@ std::unique_ptr<HttpResponse> HandleExpectAndSetCookie(
   if (got_all_expected) {
     for (const auto& cookie : query_list.at("set")) {
       http_response->AddCustomHeader(
-          "Set-Cookie", net::UnescapeURLComponent(cookie, kUnescapeAll));
+          "Set-Cookie", UnescapeBinaryURLComponent(
+                            cookie, UnescapeRule::REPLACE_PLUS_WITH_SPACE));
     }
   }
 
@@ -503,8 +498,7 @@ std::unique_ptr<HttpResponse> HandleAuthDigest(const HttpRequest& request) {
 std::unique_ptr<HttpResponse> HandleServerRedirect(HttpStatusCode redirect_code,
                                                    const HttpRequest& request) {
   GURL request_url = request.GetURL();
-  std::string dest =
-      net::UnescapeURLComponent(request_url.query(), kUnescapeAll);
+  std::string dest = UnescapeBinaryURLComponent(request_url.query());
 
   std::unique_ptr<BasicHttpResponse> http_response(new BasicHttpResponse);
   http_response->set_code(redirect_code);
@@ -524,9 +518,8 @@ std::unique_ptr<HttpResponse> HandleCrossSiteRedirect(
   if (!ShouldHandle(request, "/cross-site"))
     return nullptr;
 
-  std::string dest_all = net::UnescapeURLComponent(
-      request.relative_url.substr(std::string("/cross-site").size() + 1),
-      kUnescapeAll);
+  std::string dest_all = UnescapeBinaryURLComponent(
+      request.relative_url.substr(std::string("/cross-site").size() + 1));
 
   std::string dest;
   size_t delimiter = dest_all.find("/");
@@ -550,8 +543,7 @@ std::unique_ptr<HttpResponse> HandleCrossSiteRedirect(
 // Returns a meta redirect to URL.
 std::unique_ptr<HttpResponse> HandleClientRedirect(const HttpRequest& request) {
   GURL request_url = request.GetURL();
-  std::string dest =
-      net::UnescapeURLComponent(request_url.query(), kUnescapeAll);
+  std::string dest = UnescapeBinaryURLComponent(request_url.query());
 
   std::unique_ptr<BasicHttpResponse> http_response(new BasicHttpResponse);
   http_response->set_content_type("text/html");

net/test/embedded_test_server/request_handler_util.cc

@@ -27,12 +27,6 @@ namespace net {
 namespace test_server {
 namespace {
 
-const UnescapeRule::Type kUnescapeAll =
-    UnescapeRule::SPACES | UnescapeRule::PATH_SEPARATORS |
-    UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-    UnescapeRule::SPOOFING_AND_CONTROL_CHARS |
-    UnescapeRule::REPLACE_PLUS_WITH_SPACE;
-
 std::string GetContentType(const base::FilePath& path) {
   if (path.MatchesExtension(FILE_PATH_LITERAL(".crx")))
     return "application/x-chrome-extension";
@@ -92,8 +86,9 @@ std::unique_ptr<HttpResponse> HandlePrefixedRequest(
 RequestQuery ParseQuery(const GURL& url) {
   RequestQuery queries;
   for (QueryIterator it(url); !it.IsAtEnd(); it.Advance()) {
-    queries[net::UnescapeURLComponent(it.GetKey(), kUnescapeAll)].push_back(
-        it.GetUnescapedValue());
+    queries[UnescapeBinaryURLComponent(it.GetKey(),
+                                       UnescapeRule::REPLACE_PLUS_WITH_SPACE)]
+        .push_back(it.GetUnescapedValue());
   }
   return queries;
 }

storage/common/fileapi/file_system_util.cc

@@ -174,10 +174,7 @@ bool ParseFileSystemSchemeURL(const GURL& url,
   if (file_system_type == kFileSystemTypeUnknown)
     return false;
 
-  std::string path = net::UnescapeURLComponent(url.path(),
-      net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
-      net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
-      net::UnescapeRule::SPOOFING_AND_CONTROL_CHARS);
+  std::string path = net::UnescapeBinaryURLComponent(url.path());
 
   // Ensure the path is relative.
   while (!path.empty() && path[0] == '/')