Skip to content

Commit

Permalink
URL: Disallow a switch from a special scheme URL to a non-special scheme
Browse files Browse the repository at this point in the history
The CL is a part of the URL Interop 2023.

Chrome currently allows setting a non-special scheme on a special
schema URL. This is not compliant with the URL standard.

Before this CL:

> const url = new URL("https://example.com/");
> url.protocol = "non-special";
> url.href
"non-special://example.com/"

After this CL:

> const url = new URL("https://example.com/");
> url.protocol = "non-special";
> url.href
"https://example.com/"

UMA [1] (Canary) is nearly 0%, so we can probably skip "Intent to Ship"
process, considering this is a bug fix.

[1] https://uma.googleplex.com/p/chrome/timeline_v2/?sid=a15c7dcb7c7442a06bd62f9007872d64

Bug: 1416018
Change-Id: Ibb496f2dc0c15e6579aab4a4b03e773b362dae4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4744799
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184500}
  • Loading branch information
hayatoito authored and Chromium LUCI CQ committed Aug 17, 2023
1 parent 46f5f92 commit 61d4ebd
Show file tree
Hide file tree
Showing 24 changed files with 83 additions and 283 deletions.
25 changes: 12 additions & 13 deletions third_party/blink/renderer/platform/weborigin/kurl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -491,20 +491,19 @@ bool KURL::SetProtocol(const String& protocol) {
String(canon_protocol.data(), protocol_length);

if (SchemeRegistry::IsSpecialScheme(Protocol())) {
base::UmaHistogramBoolean(
"URL.Scheme.SetNonSpecialSchemeOnSpecialScheme",
!SchemeRegistry::IsSpecialScheme(new_protocol_canon));
}
bool new_protocol_is_special_scheme =
SchemeRegistry::IsSpecialScheme(new_protocol_canon);

base::UmaHistogramBoolean("URL.Scheme.SetNonSpecialSchemeOnSpecialScheme",
!new_protocol_is_special_scheme);

// https://url.spec.whatwg.org/#scheme-state
// 2.1.1 If url’s scheme is a special scheme and buffer is not a special
// scheme, then return.
if (!new_protocol_is_special_scheme) {
return true;
}

// We don't currently perform the check from
// https://url.spec.whatwg.org/#scheme-state that special schemes are not
// converted to non-special schemes and vice-versa, but the following logic
// should only be applied to special schemes.
// TODO(ricea): Maybe disallow switching between special and non-special
// schemes, to match the standard, if we can develop confidence that it won't
// break pages.
if (SchemeRegistry::IsSpecialScheme(Protocol()) &&
SchemeRegistry::IsSpecialScheme(new_protocol_canon)) {
// The protocol is lower-cased during canonicalization.
const bool new_protocol_is_file = new_protocol_canon == url::kFileScheme;
const bool old_protocol_is_file = ProtocolIs(url::kFileScheme);
Expand Down
5 changes: 3 additions & 2 deletions third_party/blink/renderer/platform/weborigin/kurl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1058,9 +1058,10 @@ TEST(KURLTest, SetFileProtocolFromNonSpecial) {

TEST(KURLTest, SetFileProtocolToNonSpecial) {
KURL url("file:///path");
EXPECT_EQ(url.GetPath(), "/path");
EXPECT_TRUE(url.SetProtocol("non-special-scheme"));
EXPECT_EQ(url.Protocol(), "non-special-scheme");
EXPECT_EQ(url.GetPath(), "///path");
EXPECT_EQ(url.Protocol(), "file");
EXPECT_EQ(url.GetPath(), "/path");
}

TEST(KURLTest, SetNonSpecialSchemeOnSpecialSchemeHistogram) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ PASS <a>: Setting <file:///test>.protocol = 'https'
PASS <area>: Setting <file:///test>.protocol = 'https'
PASS <a>: Setting <file:>.protocol = 'wss'
PASS <area>: Setting <file:>.protocol = 'wss'
FAIL <a>: Setting <file://hi/path>.protocol = 's' assert_equals: expected "file://hi/path" but got "s://hi/path"
FAIL <area>: Setting <file://hi/path>.protocol = 's' assert_equals: expected "file://hi/path" but got "s://hi/path"
PASS <a>: Setting <file://hi/path>.protocol = 's'
PASS <area>: Setting <file://hi/path>.protocol = 's'
PASS <a>: Setting <file:///home/you/index.html>.username = 'me' No host means no username
PASS <area>: Setting <file:///home/you/index.html>.username = 'me' No host means no username
PASS <a>: Setting <file://test/>.username = 'test'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ PASS Loading data…
FAIL URL: Setting <file://localhost/>.protocol = 'http' Can’t switch from file URL with no host assert_equals: expected "file:///" but got "http://localhost/"
PASS URL: Setting <file:///test>.protocol = 'https'
PASS URL: Setting <file:>.protocol = 'wss'
FAIL URL: Setting <file://hi/path>.protocol = 's' assert_equals: expected "file://hi/path" but got "s://hi/path"
PASS URL: Setting <file://hi/path>.protocol = 's'
PASS URL: Setting <file:///home/you/index.html>.username = 'me' No host means no username
PASS URL: Setting <file://test/>.username = 'test'
PASS URL: Setting <file:///home/me/index.html>.password = 'secret' No host means no password
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ PASS Loading data…
FAIL URL: Setting <file://localhost/>.protocol = 'http' Can’t switch from file URL with no host assert_equals: expected "file:///" but got "http://localhost/"
PASS URL: Setting <file:///test>.protocol = 'https'
PASS URL: Setting <file:>.protocol = 'wss'
FAIL URL: Setting <file://hi/path>.protocol = 's' assert_equals: expected "file://hi/path" but got "s://hi/path"
PASS URL: Setting <file://hi/path>.protocol = 's'
PASS URL: Setting <file:///home/you/index.html>.username = 'me' No host means no username
PASS URL: Setting <file://test/>.username = 'test'
PASS URL: Setting <file:///home/me/index.html>.password = 'secret' No host means no password
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
debug("Basic test");
a.href = "https://www.mydomain.com/path/";
a.protocol = "http-foo";
shouldBe("a.href", "'http-foo://www.mydomain.com/path/'");
shouldBe("a.href", "'https://www.mydomain.com/path/'");

// IE8 throws "Invalid argument" exception.
debug("Set a protocol that contains ':'");
Expand Down Expand Up @@ -54,7 +54,7 @@
debug("Set protocol to null");
a.href = "https://www.mydomain.com/path/";
a.protocol = null;
shouldBe("a.href", "'null://www.mydomain.com/path/'");
shouldBe("a.href", "'https://www.mydomain.com/path/'");

// IE8 throws "Invalid argument" exception.
try {
Expand All @@ -81,7 +81,7 @@
debug("Set protocol to undefined");
a.href = "https://www.mydomain.com/path/";
a.protocol = undefined;
shouldBe("a.href", "'undefined://www.mydomain.com/path/'");
shouldBe("a.href", "'https://www.mydomain.com/path/'");
</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
Found 443 tests; 303 PASS, 140 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 443 tests; 309 PASS, 134 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS Loading data…
PASS <a>: Setting <a://example.net>.protocol = '' The empty string is not a valid scheme. Setter leaves the URL unchanged.
PASS <area>: Setting <a://example.net>.protocol = '' The empty string is not a valid scheme. Setter leaves the URL unchanged.
Expand All @@ -25,12 +25,12 @@ PASS <a>: Setting <https://example.net:1234>.protocol = 'file'
PASS <area>: Setting <https://example.net:1234>.protocol = 'file'
PASS <a>: Setting <wss://x:x@example.net:1234>.protocol = 'file'
PASS <area>: Setting <wss://x:x@example.net:1234>.protocol = 'file'
FAIL <a>: Setting <http://example.net>.protocol = 'b' Can’t switch from special scheme to non-special assert_equals: expected "http://example.net/" but got "b://example.net/"
FAIL <area>: Setting <http://example.net>.protocol = 'b' Can’t switch from special scheme to non-special assert_equals: expected "http://example.net/" but got "b://example.net/"
FAIL <a>: Setting <https://example.net>.protocol = 's' assert_equals: expected "https://example.net/" but got "s://example.net/"
FAIL <area>: Setting <https://example.net>.protocol = 's' assert_equals: expected "https://example.net/" but got "s://example.net/"
FAIL <a>: Setting <ftp://example.net>.protocol = 'test' assert_equals: expected "ftp://example.net/" but got "test://example.net/"
FAIL <area>: Setting <ftp://example.net>.protocol = 'test' assert_equals: expected "ftp://example.net/" but got "test://example.net/"
PASS <a>: Setting <http://example.net>.protocol = 'b' Can’t switch from special scheme to non-special
PASS <area>: Setting <http://example.net>.protocol = 'b' Can’t switch from special scheme to non-special
PASS <a>: Setting <https://example.net>.protocol = 's'
PASS <area>: Setting <https://example.net>.protocol = 's'
PASS <a>: Setting <ftp://example.net>.protocol = 'test'
PASS <area>: Setting <ftp://example.net>.protocol = 'test'
FAIL <a>: Setting <ssh://me@example.net>.protocol = 'http' Can’t switch from non-special scheme to special assert_equals: expected "ssh://me@example.net" but got "http://me@example.net/"
FAIL <area>: Setting <ssh://me@example.net>.protocol = 'http' Can’t switch from non-special scheme to special assert_equals: expected "ssh://me@example.net" but got "http://me@example.net/"
FAIL <a>: Setting <ssh://me@example.net>.protocol = 'https' assert_equals: expected "ssh://me@example.net" but got "https://me@example.net/"
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
Found 222 tests; 148 PASS, 74 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 222 tests; 151 PASS, 71 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS Loading data…
PASS URL: Setting <a://example.net>.protocol = '' The empty string is not a valid scheme. Setter leaves the URL unchanged.
PASS URL: Setting <a://example.net>.protocol = 'b'
Expand All @@ -13,9 +13,9 @@ PASS URL: Setting <a://example.net>.protocol = 'bé' Non-ASCII is rejected
PASS URL: Setting <http://test@example.net>.protocol = 'file' Can’t switch from URL containing username/password/port to file
PASS URL: Setting <https://example.net:1234>.protocol = 'file'
PASS URL: Setting <wss://x:x@example.net:1234>.protocol = 'file'
FAIL URL: Setting <http://example.net>.protocol = 'b' Can’t switch from special scheme to non-special assert_equals: expected "http://example.net/" but got "b://example.net/"
FAIL URL: Setting <https://example.net>.protocol = 's' assert_equals: expected "https://example.net/" but got "s://example.net/"
FAIL URL: Setting <ftp://example.net>.protocol = 'test' assert_equals: expected "ftp://example.net/" but got "test://example.net/"
PASS URL: Setting <http://example.net>.protocol = 'b' Can’t switch from special scheme to non-special
PASS URL: Setting <https://example.net>.protocol = 's'
PASS URL: Setting <ftp://example.net>.protocol = 'test'
FAIL URL: Setting <ssh://me@example.net>.protocol = 'http' Can’t switch from non-special scheme to special assert_equals: expected "ssh://me@example.net" but got "http://me@example.net/"
FAIL URL: Setting <ssh://me@example.net>.protocol = 'https' assert_equals: expected "ssh://me@example.net" but got "https://me@example.net/"
FAIL URL: Setting <ssh://me@example.net>.protocol = 'file' assert_equals: expected "ssh://me@example.net" but got "file://me%40example.net/"
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
Found 222 tests; 148 PASS, 74 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 222 tests; 151 PASS, 71 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS Loading data…
PASS URL: Setting <a://example.net>.protocol = '' The empty string is not a valid scheme. Setter leaves the URL unchanged.
PASS URL: Setting <a://example.net>.protocol = 'b'
Expand All @@ -13,9 +13,9 @@ PASS URL: Setting <a://example.net>.protocol = 'bé' Non-ASCII is rejected
PASS URL: Setting <http://test@example.net>.protocol = 'file' Can’t switch from URL containing username/password/port to file
PASS URL: Setting <https://example.net:1234>.protocol = 'file'
PASS URL: Setting <wss://x:x@example.net:1234>.protocol = 'file'
FAIL URL: Setting <http://example.net>.protocol = 'b' Can’t switch from special scheme to non-special assert_equals: expected "http://example.net/" but got "b://example.net/"
FAIL URL: Setting <https://example.net>.protocol = 's' assert_equals: expected "https://example.net/" but got "s://example.net/"
FAIL URL: Setting <ftp://example.net>.protocol = 'test' assert_equals: expected "ftp://example.net/" but got "test://example.net/"
PASS URL: Setting <http://example.net>.protocol = 'b' Can’t switch from special scheme to non-special
PASS URL: Setting <https://example.net>.protocol = 's'
PASS URL: Setting <ftp://example.net>.protocol = 'test'
FAIL URL: Setting <ssh://me@example.net>.protocol = 'http' Can’t switch from non-special scheme to special assert_equals: expected "ssh://me@example.net" but got "http://me@example.net/"
FAIL URL: Setting <ssh://me@example.net>.protocol = 'https' assert_equals: expected "ssh://me@example.net" but got "https://me@example.net/"
FAIL URL: Setting <ssh://me@example.net>.protocol = 'file' assert_equals: expected "ssh://me@example.net" but got "file://me%40example.net/"
Expand Down

This file was deleted.

0 comments on commit 61d4ebd

Please sign in to comment.