Correctly handle problematic nested escapes in URL paths.

url/url_canon_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <errno.h>
 
 #include "base/macros.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/third_party/mozilla/url_parse.h"
 #include "url/url_canon.h"
@@ skipping 982 matching lines @@
 
     EXPECT_EQ(port_cases[i].expected_success, success);
     EXPECT_EQ(std::string(port_cases[i].expected), out_str);
     EXPECT_EQ(port_cases[i].expected_component.begin, out_comp.begin);
     EXPECT_EQ(port_cases[i].expected_component.len, out_comp.len);
   }
 }
 
 TEST(URLCanonTest, Path) {
   DualComponentCase path_cases[] = {
+#if 0

[brettw, 2015/09/21 22:28:34]

Left by mistake?

[Peter Kasting, 2015/09/21 23:15:31]

Yeah, oops :(

     // ----- path collapsing tests -----
     {"/././foo", L"/././foo", "/foo", Component(0, 4), true},
     {"/./.foo", L"/./.foo", "/.foo", Component(0, 5), true},
     {"/foo/.", L"/foo/.", "/foo/", Component(0, 5), true},
     {"/foo/./", L"/foo/./", "/foo/", Component(0, 5), true},
       // double dots followed by a slash or the end of the string count
     {"/foo/bar/..", L"/foo/bar/..", "/foo/", Component(0, 5), true},
     {"/foo/bar/../", L"/foo/bar/../", "/foo/", Component(0, 5), true},
       // don't count double dots when they aren't followed by a slash
     {"/foo/..bar", L"/foo/..bar", "/foo/..bar", Component(0, 10), true},
@@ skipping 40 matching lines @@
       // Backslashes should get converted to forward slashes
     {"\\foo\\bar", L"\\foo\\bar", "/foo/bar", Component(0, 8), true},
       // Hashes found in paths (possibly only when the caller explicitly sets
       // the path on an already-parsed URL) should be escaped.
     {"/foo#bar", L"/foo#bar", "/foo%23bar", Component(0, 10), true},
       // %7f should be allowed and %3D should not be unescaped (these were wrong
       // in a previous version).
     {"/%7Ffp3%3Eju%3Dduvgw%3Dd", L"/%7Ffp3%3Eju%3Dduvgw%3Dd", "/%7Ffp3%3Eju%3Dduvgw%3Dd", Component(0, 24), true},
       // @ should be passed through unchanged (escaped or unescaped).
     {"/@asdf%40", L"/@asdf%40", "/@asdf%40", Component(0, 9), true},
+      // Nested escape sequences should result in escaping the leading '%' if
+      // unescaping would result in a new escape sequence.
+    {"/%0%30", L"/%0%30", "/%2500", Component(0, 6), true},

[brettw, 2015/09/21 22:28:34]

Can you make these cases use different characters for each possible character
rather than "%30" and "0", to prove we're not doing something silly duplicating
one of them to get the output?

[Peter Kasting, 2015/09/21 23:15:30]

Done.

+    {"/%%300", L"/%%300", "/%2500", Component(0, 6), true},
+    {"/%%30%30", L"/%%30%30", "/%2500", Component(0, 6), true},
+      // Make sure truncated "nested" escapes don't result in reading off the
+      // string end.
+    {"/%%30", L"/%%30", "/%0", Component(0, 3), true},
+      // Don't unescape the leading '%' if unescaping doesn't result in a valid
+      // new escape sequence.
+    {"/%%470", L"/%%470", "/%G0", Component(0, 4), true},
+    {"/%%2D%41", L"/%%2D%41", "/%-A", Component(0, 4), true},
+#endif
+      // Don't erroneously downcast a UTF-16 charater in a way that makes it
+      // look like part of an escape sequence.
+    {NULL, L"/%%30\x0130", "/%0%C4%B0", Component(0, 9), true},
+#if 0
+
 
     // ----- encoding tests -----
       // Basic conversions
     {"/\xe4\xbd\xa0\xe5\xa5\xbd\xe4\xbd\xa0\xe5\xa5\xbd", L"/\x4f60\x597d\x4f60\x597d", "/%E4%BD%A0%E5%A5%BD%E4%BD%A0%E5%A5%BD", Component(0, 37), true},
       // Invalid unicode characters should fail. We only do validation on
       // UTF-16 input, so this doesn't happen on 8-bit.
     {"/\xef\xb7\x90zyx", NULL, "/%EF%B7%90zyx", Component(0, 13), true},
     {NULL, L"/\xfdd0zyx", "/%EF%BF%BDzyx", Component(0, 13), false},
+#endif
   };
 
   for (size_t i = 0; i < arraysize(path_cases); i++) {
     if (path_cases[i].input8) {
       int len = static_cast<int>(strlen(path_cases[i].input8));
       Component in_comp(0, len);
       Component out_comp;
       std::string out_str;
       StdStringCanonOutput output(&out_str);
       bool success =
@@ skipping 1042 matching lines @@
   repl_output.Complete();
 
   // Generate the expected string and check.
   std::string expected("file:///foo?");
   for (size_t i = 0; i < new_query.length(); i++)
     expected.push_back('a');
   EXPECT_TRUE(expected == repl_str);
 }
 
 }  // namespace url