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/components/url_formatter/url_fixer_unittest.cc b/components/url_formatter/url_fixer_unittest.cc
index 9c2acd7..607cee3 100644
--- a/components/url_formatter/url_fixer_unittest.cc
+++ b/components/url_formatter/url_fixer_unittest.cc
@@ -515,7 +515,7 @@
// Much of the work here comes from GURL's canonicalization stage.
{"file://C:/foo/bar", "file:///C:/foo/bar"},
- {"file:c:", "file:///C:/"},
+ {"file:c:", "file:///C:"},
{"file:c:WINDOWS", "file:///C:/WINDOWS"},
{"file:c|Program Files", "file:///C:/Program%20Files"},
{"file:/file", "file://file/"},
diff --git a/third_party/blink/web_tests/platform/win/external/wpt/url/a-element-expected.txt b/third_party/blink/web_tests/platform/win/external/wpt/url/a-element-expected.txt
index c2ace28..c5c521b 100644
--- a/third_party/blink/web_tests/platform/win/external/wpt/url/a-element-expected.txt
+++ b/third_party/blink/web_tests/platform/win/external/wpt/url/a-element-expected.txt
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
-Found 527 tests; 334 PASS, 193 FAIL, 0 TIMEOUT, 0 NOTRUN.
+Found 527 tests; 337 PASS, 190 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS Loading data…
PASS Parsing: <http://example .
org> against <http://example.org/foo/bar>
@@ -410,7 +410,7 @@
PASS Parsing: <..> against <file:///C:/>
PASS Parsing: <..> against <file:///>
PASS Parsing: </> against <file:///C:/a/b>
-FAIL Parsing: <//d:> against <file:///C:/a/b> assert_equals: href expected "file:///d:" but got "file:///D:/"
+FAIL Parsing: <//d:> against <file:///C:/a/b> assert_equals: href expected "file:///d:" but got "file:///D:"
FAIL Parsing: <//d:/..> against <file:///C:/a/b> assert_equals: href expected "file:///d:/" but got "file:///D:/"
PASS Parsing: <..> against <file:///ab:/>
PASS Parsing: <..> against <file:///1:/>
@@ -437,9 +437,9 @@
PASS Parsing: <file://> against <file://ape/>
PASS Parsing: </rooibos> against <file://tea/>
PASS Parsing: </?chai> against <file://tea/>
-FAIL Parsing: <C|> against <file://host/dir/file> assert_equals: href expected "file:///C:" but got "file:///C:/"
-FAIL Parsing: <C|#> against <file://host/dir/file> assert_equals: href expected "file:///C:#" but got "file:///C:/#"
-FAIL Parsing: <C|?> against <file://host/dir/file> assert_equals: href expected "file:///C:?" but got "file:///C:/?"
+PASS Parsing: <C|> against <file://host/dir/file>
+PASS Parsing: <C|#> against <file://host/dir/file>
+PASS Parsing: <C|?> against <file://host/dir/file>
PASS Parsing: <C|/> against <file://host/dir/file>
PASS Parsing: <C|
/> against <file://host/dir/file>
diff --git a/third_party/blink/web_tests/platform/win/external/wpt/url/a-element-xhtml-expected.txt b/third_party/blink/web_tests/platform/win/external/wpt/url/a-element-xhtml-expected.txt
index c2ace28..c5c521b 100644
--- a/third_party/blink/web_tests/platform/win/external/wpt/url/a-element-xhtml-expected.txt
+++ b/third_party/blink/web_tests/platform/win/external/wpt/url/a-element-xhtml-expected.txt
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
-Found 527 tests; 334 PASS, 193 FAIL, 0 TIMEOUT, 0 NOTRUN.
+Found 527 tests; 337 PASS, 190 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS Loading data…
PASS Parsing: <http://example .
org> against <http://example.org/foo/bar>
@@ -410,7 +410,7 @@
PASS Parsing: <..> against <file:///C:/>
PASS Parsing: <..> against <file:///>
PASS Parsing: </> against <file:///C:/a/b>
-FAIL Parsing: <//d:> against <file:///C:/a/b> assert_equals: href expected "file:///d:" but got "file:///D:/"
+FAIL Parsing: <//d:> against <file:///C:/a/b> assert_equals: href expected "file:///d:" but got "file:///D:"
FAIL Parsing: <//d:/..> against <file:///C:/a/b> assert_equals: href expected "file:///d:/" but got "file:///D:/"
PASS Parsing: <..> against <file:///ab:/>
PASS Parsing: <..> against <file:///1:/>
@@ -437,9 +437,9 @@
PASS Parsing: <file://> against <file://ape/>
PASS Parsing: </rooibos> against <file://tea/>
PASS Parsing: </?chai> against <file://tea/>
-FAIL Parsing: <C|> against <file://host/dir/file> assert_equals: href expected "file:///C:" but got "file:///C:/"
-FAIL Parsing: <C|#> against <file://host/dir/file> assert_equals: href expected "file:///C:#" but got "file:///C:/#"
-FAIL Parsing: <C|?> against <file://host/dir/file> assert_equals: href expected "file:///C:?" but got "file:///C:/?"
+PASS Parsing: <C|> against <file://host/dir/file>
+PASS Parsing: <C|#> against <file://host/dir/file>
+PASS Parsing: <C|?> against <file://host/dir/file>
PASS Parsing: <C|/> against <file://host/dir/file>
PASS Parsing: <C|
/> against <file://host/dir/file>
diff --git a/third_party/blink/web_tests/platform/win/external/wpt/url/url-constructor-expected.txt b/third_party/blink/web_tests/platform/win/external/wpt/url/url-constructor-expected.txt
index 9c6ddd5..7934a6b 100644
--- a/third_party/blink/web_tests/platform/win/external/wpt/url/url-constructor-expected.txt
+++ b/third_party/blink/web_tests/platform/win/external/wpt/url/url-constructor-expected.txt
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
-Found 527 tests; 399 PASS, 128 FAIL, 0 TIMEOUT, 0 NOTRUN.
+Found 527 tests; 402 PASS, 125 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS Loading data…
PASS Parsing: <http://example .
org> against <http://example.org/foo/bar>
@@ -450,7 +450,7 @@
PASS Parsing: <..> against <file:///C:/>
PASS Parsing: <..> against <file:///>
PASS Parsing: </> against <file:///C:/a/b>
-FAIL Parsing: <//d:> against <file:///C:/a/b> assert_equals: href expected "file:///d:" but got "file:///D:/"
+FAIL Parsing: <//d:> against <file:///C:/a/b> assert_equals: href expected "file:///d:" but got "file:///D:"
FAIL Parsing: <//d:/..> against <file:///C:/a/b> assert_equals: href expected "file:///d:/" but got "file:///D:/"
PASS Parsing: <..> against <file:///ab:/>
PASS Parsing: <..> against <file:///1:/>
@@ -477,9 +477,9 @@
PASS Parsing: <file://> against <file://ape/>
PASS Parsing: </rooibos> against <file://tea/>
PASS Parsing: </?chai> against <file://tea/>
-FAIL Parsing: <C|> against <file://host/dir/file> assert_equals: href expected "file:///C:" but got "file:///C:/"
-FAIL Parsing: <C|#> against <file://host/dir/file> assert_equals: href expected "file:///C:#" but got "file:///C:/#"
-FAIL Parsing: <C|?> against <file://host/dir/file> assert_equals: href expected "file:///C:?" but got "file:///C:/?"
+PASS Parsing: <C|> against <file://host/dir/file>
+PASS Parsing: <C|#> against <file://host/dir/file>
+PASS Parsing: <C|?> against <file://host/dir/file>
PASS Parsing: <C|/> against <file://host/dir/file>
PASS Parsing: <C|
/> against <file://host/dir/file>
diff --git a/third_party/blink/web_tests/platform/win/fast/url/relative-win-expected.txt b/third_party/blink/web_tests/platform/win/fast/url/relative-win-expected.txt
index 54c11a5..f84c2ee 100644
--- a/third_party/blink/web_tests/platform/win/fast/url/relative-win-expected.txt
+++ b/third_party/blink/web_tests/platform/win/fast/url/relative-win-expected.txt
@@ -9,7 +9,7 @@
FAIL canonicalize('\\\\another\\path') should be . Was file://another/path.
PASS canonicalize('//c:/foo') is 'file:///C:/foo'
PASS canonicalize('//localhost/c:/foo') is 'file:///C:/foo'
-FAIL canonicalize('c:') should be . Was file:///C:/.
+FAIL canonicalize('c:') should be . Was file:///C:.
FAIL canonicalize('c:/foo') should be . Was file:///C:/foo.
FAIL canonicalize('c:\\foo') should be . Was file:///C:/foo.
PASS canonicalize('/z:/bar') is 'file:///Z:/bar'
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);