Commit 5857da64 authored by Frédéric Wang's avatar Frédéric Wang Committed by Commit Bot

Do not remove %s token when validating (un)registerProtocolHandler's URL

The HTML specifications [1] indicates that the "%s" token is preserved
when parsing the URL and this is important to properly perform syntax
validation. Currently, Chrome removes the token, which means that
something like "ht%stp://example.org" is treated as a valid URL and
bypasses the security check. This is however treated as invalid for the
browser process validation and so leads to a crash (bad IPC message).

The WPT tests are updated to provide the correct expectations. Those
for which the %s token is inside the protocol are treated as relative
URLs and are valid. Others where the %s token is inside the hostname
or port should raise syntax errors.

[1] https://html.spec.whatwg.org/multipage/system-state.html#normalize-protocol-handler-parameters

Bug: 1112377
Change-Id: I962bcfdd593223568fc72475efeb299adaa72236
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2335434Reviewed-by: default avatarMike West <mkwst@chromium.org>
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Cr-Commit-Position: refs/heads/master@{#796404}
parent 008d0a49
...@@ -87,9 +87,7 @@ static bool VerifyCustomHandlerURLSecurity(const LocalDOMWindow& window, ...@@ -87,9 +87,7 @@ static bool VerifyCustomHandlerURLSecurity(const LocalDOMWindow& window,
static bool VerifyCustomHandlerURL(const LocalDOMWindow& window, static bool VerifyCustomHandlerURL(const LocalDOMWindow& window,
const String& user_url, const String& user_url,
ExceptionState& exception_state) { ExceptionState& exception_state) {
String new_url = user_url; KURL full_url = window.CompleteURL(user_url);
new_url.Remove(user_url.Find(kToken), base::size(kToken) - 1);
KURL full_url = window.CompleteURL(new_url);
KURL base_url = window.BaseURL(); KURL base_url = window.BaseURL();
String error_message; String error_message;
......
...@@ -24,6 +24,10 @@ test(() => { ...@@ -24,6 +24,10 @@ test(() => {
[ [
'%s', '%s',
'foo/%s', 'foo/%s',
`%s${location.href}`,
location.href.replace(location.protocol,
`${location.protocol[0]}%s${location.protocol.substring(1)}`),
location.href.replace(location.protocol, `${location.protocol}%s`),
location.href + '/%s', location.href + '/%s',
location.href + '#%s', location.href + '#%s',
location.href + '?foo=%s', location.href + '?foo=%s',
...@@ -45,6 +49,10 @@ test(() => { ...@@ -45,6 +49,10 @@ test(() => {
[ [
'', '',
'%S', '%S',
'http://%s.com',
'http://%s.example.com',
location.href.replace(location.hostname, `%s${location.hostname}`),
location.href.replace(location.port, `%s${location.port}`),
location.href + '', location.href + '',
location.href + '/%', location.href + '/%',
location.href + '/%a', location.href + '/%a',
...@@ -64,8 +72,6 @@ test(() => { ...@@ -64,8 +72,6 @@ test(() => {
}); });
[ [
'http://%s.com',
'http://%s.example.com',
'http://example.com/%s', 'http://example.com/%s',
'https://example.com/%s', 'https://example.com/%s',
'http://foobar.example.com/%s', 'http://foobar.example.com/%s',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment