[go: nahoru, domu]

Make file:///Y: canonical on all systems

Currently, on Windows only, the canonical form of file:///Y: is
file:///Y:/. However, this causes the operation of taking a
canonical form not being idempotent, because the canonical form of
file:///./Y: is file:///Y:, which itself is not canonical.

After a discussion on https://crbug.com/1078698 and inspecting
https://url.spec.whatwg.org/, it seems like the correct solution is to
actually make file:///Y: canonical (note: this is already the case
everywhere but on Windows).

That solution is implemented in this CL, including adding test cases
for valid-URL examples mentioned in https://crbug.com/1078698#c13.

Bug: 1078698
Change-Id: Ib16f92541b715d2137a0ca21b2c0ec0e0af18db7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2190679
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#767758}
diff --git a/url/url_canon_fileurl.cc b/url/url_canon_fileurl.cc
index 6277289..067ed58 100644
--- a/url/url_canon_fileurl.cc
+++ b/url/url_canon_fileurl.cc
@@ -76,8 +76,8 @@
     Component sub_path = MakeRange(after_drive, path.end());
     Component fake_output_path;
     success = CanonicalizePath(spec, sub_path, output, &fake_output_path);
-  } else {
-    // No input path, canonicalize to a slash.
+  } else if (after_drive == path.begin) {
+    // No input path and no drive spec, canonicalize to a slash.
     output->push_back('/');
   }
 
diff --git a/url/url_canon_unittest.cc b/url/url_canon_unittest.cc
index b1b6dfd..87fb406 100644
--- a/url/url_canon_unittest.cc
+++ b/url/url_canon_unittest.cc
@@ -1517,6 +1517,10 @@
     {"file:///C:/gaba?query#ref", NULL, NULL, NULL, "filer", NULL, "/foo", "b", "c", "file://filer/foo?b#c"},
       // Replace nothing
     {"file:///C:/gaba?query#ref", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, "file:///C:/gaba?query#ref"},
+    {"file:///Y:", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, "file:///Y:"},
+    {"file:///Y:/", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, "file:///Y:/"},
+    {"file:///./Y", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, "file:///Y"},
+    {"file:///./Y:", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, "file:///Y:"},
       // Clear non-path components (common)
     {"file:///C:/gaba?query#ref", NULL, NULL, NULL, NULL, NULL, NULL, kDeleteComp, kDeleteComp, "file:///C:/gaba"},
       // Replace path with something that doesn't begin with a slash and make
@@ -1532,6 +1536,7 @@
 
   for (size_t i = 0; i < base::size(replace_cases); i++) {
     const ReplaceCase& cur = replace_cases[i];
+    SCOPED_TRACE(cur.base);
     int base_len = static_cast<int>(strlen(cur.base));
     Parsed parsed;
     ParseFileURL(cur.base, base_len, &parsed);