0

new URL('') should throw TypeError

As per the spec https://url.spec.whatwg.org/#no-scheme-state
If base url is null or not relative and starting character of
url is not # return failure. So empty url should also throw
exception in this case.

Few tests related to autofill were failing after this
change, because empty action url is by default resolved to
base url before this patch. Made appropriate code changes
as empty action url can be equal to base url only for
relative base urls.

BUG=463961

Review URL: https://codereview.chromium.org/1409293007

Cr-Commit-Position: refs/heads/master@{#362589}
This commit is contained in:
ramya.v
2015-12-01 18:24:46 -08:00
committed by Commit bot
parent 27d54fc7c8
commit 56d4220513
10 changed files with 30 additions and 12 deletions

@ -553,7 +553,7 @@ TEST_F(PasswordAutofillAgentTest, InitialAutocompleteForEmptyAction) {
// Set the expected form origin and action URLs.
UpdateOriginForHTML(kEmptyActionFormHTML);
fill_data_.action = fill_data_.origin;
fill_data_.action = GURL();
// Simulate the browser sending back the login info, it triggers the
// autocomplete.
@ -1400,7 +1400,7 @@ TEST_F(PasswordAutofillAgentTest,
TEST_F(PasswordAutofillAgentTest,
RememberLastNonEmptyUsernameAndPasswordOnSubmit_New) {
const char kNewPasswordFormHTML[] =
"<FORM name='LoginTestForm'>"
"<FORM name='LoginTestForm' action='http://www.bidule.com'>"
" <INPUT type='text' id='username' autocomplete='username'/>"
" <INPUT type='password' id='password' autocomplete='new-password'/>"
" <INPUT type='submit' value='Login'/>"

@ -449,6 +449,7 @@ TEST_F(PasswordGenerationAgentTest, DynamicFormTest) {
ExecuteJavaScriptForTests(
"var form = document.createElement('form');"
"form.action='http://www.random.com';"
"var username = document.createElement('input');"
"username.type = 'text';"
"username.id = 'dynamic_username';"

@ -1105,7 +1105,11 @@ bool IsFormVisible(blink::WebFrame* frame,
frame->document().forms(forms);
#if !defined(OS_MACOSX) && !defined(OS_ANDROID)
const bool action_is_empty = canonical_action == canonical_origin;
// Omitting the action attribute would result in |canonical_origin| for
// hierarchical schemes like http:, and in an empty URL for non-hierarchical
// schemes like about: or data: etc.
const bool action_is_empty = canonical_action.is_empty()
|| canonical_action == canonical_origin;
#endif
// Since empty or unspecified action fields are automatically set to page URL,
@ -1120,7 +1124,8 @@ bool IsFormVisible(blink::WebFrame* frame,
GURL iter_canonical_action = GetCanonicalActionForForm(form);
#if !defined(OS_MACOSX) && !defined(OS_ANDROID)
bool form_action_is_empty = iter_canonical_action == frame_url;
bool form_action_is_empty = iter_canonical_action.is_empty()
|| iter_canonical_action == frame_url;
if (action_is_empty != form_action_is_empty)
continue;

@ -31,6 +31,12 @@ test(function() {
assertThrows(function() {
new URL();
}, 'TypeError: Failed to construct \'URL\': 1 argument required, but only 0 present.');
assertThrows(function() {
new URL('');
}, 'TypeError: Failed to construct \'URL\': Invalid URL');
assertThrows(function() {
new URL('','about:blank');
}, 'TypeError: Failed to construct \'URL\': Invalid URL');
assertThrows(function() {
new URL('abc');
}, 'TypeError: Failed to construct \'URL\': Invalid URL');

@ -18,8 +18,8 @@ cases = [
["http://f:fifty-two/c", [":","","","","",""]],
["http://f:999999/c", [":","","0","","",""]],
["http://f: 21 / b ? d # e ", [":","","","","",""]],
["", ["data:","","","text/plain,baseURL","",""]],
[" \\t", ["data:","","","text/plain,baseURL","",""]],
["", [":","","","","",""]],
[" \\t", [":","","","","",""]],
[":foo.com/", [":","","","","",""]],
[":foo.com\\\\", [":","","","","",""]],
[":", [":","","","","",""]],

@ -19,8 +19,8 @@ FAIL segments('http://f:\n/c') should be [":","","","","",""]. Was ["http:","f",
FAIL segments('http://f:fifty-two/c') should be [":","","","","",""]. Was [":","","0","","",""].
PASS segments('http://f:999999/c') is '[":","","0","","",""]'
FAIL segments('http://f: 21 / b ? d # e ') should be [":","","","","",""]. Was [":","","0","","",""].
PASS segments('') is '["data:","","","text/plain,baseURL","",""]'
PASS segments(' \t') is '["data:","","","text/plain,baseURL","",""]'
PASS segments('') is '[":","","","","",""]'
PASS segments(' \t') is '[":","","","","",""]'
PASS segments(':foo.com/') is '[":","","","","",""]'
PASS segments(':foo.com\\') is '[":","","","","",""]'
PASS segments(':') is '[":","","","","",""]'

@ -19,8 +19,8 @@ FAIL segments('http://f:\n/c') should be [":","","","","",""]. Was ["http:","f",
FAIL segments('http://f:fifty-two/c') should be [":","","","","",""]. Was [":","","0","","",""].
PASS segments('http://f:999999/c') is '[":","","0","","",""]'
FAIL segments('http://f: 21 / b ? d # e ') should be [":","","","","",""]. Was [":","","0","","",""].
PASS segments('') is '["data:","","","text/plain,baseURL","",""]'
PASS segments(' \t') is '["data:","","","text/plain,baseURL","",""]'
PASS segments('') is '[":","","","","",""]'
PASS segments(' \t') is '[":","","","","",""]'
PASS segments(':foo.com/') is '[":","","","","",""]'
PASS segments(':foo.com\\') is '[":","","","","",""]'
PASS segments(':') is '[":","","","","",""]'

@ -19,8 +19,8 @@ FAIL segments('http://f:\n/c') should be [":","","","","",""]. Was ["http:","f",
FAIL segments('http://f:fifty-two/c') should be [":","","","","",""]. Was [":","","0","","",""].
PASS segments('http://f:999999/c') is '[":","","0","","",""]'
FAIL segments('http://f: 21 / b ? d # e ') should be [":","","","","",""]. Was [":","","0","","",""].
PASS segments('') is '["data:","","","text/plain,baseURL","",""]'
PASS segments(' \t') is '["data:","","","text/plain,baseURL","",""]'
PASS segments('') is '[":","","","","",""]'
PASS segments(' \t') is '[":","","","","",""]'
PASS segments(':foo.com/') is '[":","","","","",""]'
PASS segments(':foo.com\\') is '[":","","","","",""]'
PASS segments(':') is '[":","","","","",""]'

@ -75,6 +75,10 @@ bool DoIsRelativeURL(const char* base,
TrimURL(url, &begin, &url_len);
if (begin >= url_len) {
// Empty URLs are relative, but do nothing.
if (!is_base_hierarchical) {
// Don't allow relative URLs if the base scheme doesn't support it.
return false;
}
*relative_component = Component(begin, 0);
*is_relative = true;
return true;

@ -1990,6 +1990,8 @@ TEST(URLCanonTest, ResolveRelativeURL) {
// Non-hierarchical base: absolute input should succeed.
{"data:foobar", false, false, "http://host/", true, false, false, NULL},
{"data:foobar", false, false, "http:host", true, false, false, NULL},
// Non-hierarchical base: empty URL should give error.
{"data:foobar", false, false, "", false, false, false, NULL},
// Invalid schemes should be treated as relative.
{"http://foo/bar", true, false, "./asd:fgh", true, true, true, "http://foo/asd:fgh"},
{"http://foo/bar", true, false, ":foo", true, true, true, "http://foo/:foo"},