LibURL+LibWeb: Do not percent decode in password/username getters

Doing it is not part of the spec. Whenever needed, the spec will
explicitly percent decode the username and password.

This fixes some URL WPT tests.
This commit is contained in:
Shannon Booth 2024-08-04 22:02:02 +12:00 committed by Tim Ledbetter
parent a10610a1ca
commit f511c0b441
Notes: github-actions[bot] 2024-08-04 11:59:57 +00:00
14 changed files with 62 additions and 45 deletions

View File

@ -456,8 +456,8 @@ TEST_CASE(username_and_password)
URL::URL url(url_with_username_and_password);
EXPECT(url.is_valid());
EXPECT_EQ(MUST(url.serialized_host()), "test.com"sv);
EXPECT_EQ(MUST(url.username()), "username"sv);
EXPECT_EQ(MUST(url.password()), "password"sv);
EXPECT_EQ(url.username(), "username"sv);
EXPECT_EQ(url.password(), "password"sv);
}
{
@ -465,8 +465,10 @@ TEST_CASE(username_and_password)
URL::URL url(url_with_percent_encoded_credentials);
EXPECT(url.is_valid());
EXPECT_EQ(MUST(url.serialized_host()), "test.com"sv);
EXPECT_EQ(MUST(url.username()), "username!$%"sv);
EXPECT_EQ(MUST(url.password()), "password!$%"sv);
EXPECT_EQ(url.username(), "username%21%24%25");
EXPECT_EQ(url.password(), "password%21%24%25");
EXPECT_EQ(URL::percent_decode(url.username()), "username!$%"sv);
EXPECT_EQ(URL::percent_decode(url.password()), "password!$%"sv);
}
{
@ -475,8 +477,8 @@ TEST_CASE(username_and_password)
URL::URL url(url_with_long_username);
EXPECT(url.is_valid());
EXPECT_EQ(MUST(url.serialized_host()), "test.com"sv);
EXPECT_EQ(MUST(url.username()), username);
EXPECT(MUST(url.password()).is_empty());
EXPECT_EQ(url.username(), username);
EXPECT(url.password().is_empty());
}
{
@ -485,8 +487,8 @@ TEST_CASE(username_and_password)
URL::URL url(url_with_long_password);
EXPECT(url.is_valid());
EXPECT_EQ(MUST(url.serialized_host()), "test.com"sv);
EXPECT(MUST(url.username()).is_empty());
EXPECT_EQ(MUST(url.password()), password);
EXPECT(url.username().is_empty());
EXPECT_EQ(url.password(), password);
}
}

View File

@ -0,0 +1,2 @@
user%20name
pa%40ss%3Aword

View File

@ -88,6 +88,16 @@ port => ''
pathname => '/p'
search => ''
hash => ''
new URL('http://user%20name:pa%40ss%3Aword@www.ladybird.org', undefined)
protocol => 'http:'
username => 'user%20name'
password => 'pa%40ss%3Aword'
host => 'www.ladybird.org'
hostname => 'www.ladybird.org'
port => ''
pathname => '/'
search => ''
hash => ''
=========================================
URL.parse('ftp://serenityos.org:21', undefined)
protocol => 'ftp:'
@ -179,3 +189,13 @@ port => ''
pathname => '/p'
search => ''
hash => ''
URL.parse('http://user%20name:pa%40ss%3Aword@www.ladybird.org', undefined)
protocol => 'http:'
username => 'user%20name'
password => 'pa%40ss%3Aword'
host => 'www.ladybird.org'
hostname => 'www.ladybird.org'
port => ''
pathname => '/'
search => ''
hash => ''

View File

@ -0,0 +1,9 @@
<a id="username-and-password" href="http://user%20name:pa%40ss%3Aword@www.ladybird.org"></a>
<script src="../include.js"></script>
<script>
test(() => {
const anchorElement = document.getElementById('username-and-password');
println(anchorElement.username);
println(anchorElement.password);
})
</script>

View File

@ -23,6 +23,7 @@
{ input: '/hello', base: 'file://friends/' },
{ input: '//d:/..', base: 'file:///C:/a/b' },
{ input: 'file://a%C2%ADb/p' },
{ input: 'http://user%20name:pa%40ss%3Aword@www.ladybird.org' },
];
for (url of urls) {

View File

@ -271,12 +271,12 @@ Optional<Header> HttpRequest::get_http_basic_authentication_header(URL::URL cons
if (!url.includes_credentials())
return {};
StringBuilder builder;
builder.append(url.username().release_value_but_fixme_should_propagate_errors());
builder.append(URL::percent_decode(url.username()));
builder.append(':');
builder.append(url.password().release_value_but_fixme_should_propagate_errors());
builder.append(URL::percent_decode(url.password()));
// FIXME: change to TRY() and make method fallible
auto token = MUST(encode_base64(MUST(builder.to_string()).bytes()));
auto token = MUST(encode_base64(builder.string_view().bytes()));
builder.clear();
builder.append("Basic "sv);
builder.append(token);

View File

@ -36,16 +36,6 @@ URL URL::complete_url(StringView relative_url) const
return Parser::basic_parse(relative_url, *this);
}
ErrorOr<String> URL::username() const
{
return String::from_byte_string(percent_decode(m_data->username));
}
ErrorOr<String> URL::password() const
{
return String::from_byte_string(percent_decode(m_data->password));
}
ByteString URL::path_segment_at_index(size_t index) const
{
VERIFY(index < path_segment_count());

View File

@ -126,8 +126,8 @@ public:
bool is_valid() const { return m_data->valid; }
String const& scheme() const { return m_data->scheme; }
ErrorOr<String> username() const;
ErrorOr<String> password() const;
String const& username() const { return m_data->username; }
String const& password() const { return m_data->password; }
Host const& host() const { return m_data->host; }
ErrorOr<String> serialized_host() const;
ByteString basename() const;
@ -180,9 +180,6 @@ public:
return equals(other, ExcludeFragment::No);
}
String const& raw_username() const { return m_data->username; }
String const& raw_password() const { return m_data->password; }
Optional<BlobURLEntry> const& blob_url_entry() const { return m_data->blob_url_entry; }
void set_blob_url_entry(Optional<BlobURLEntry> entry) { m_data->blob_url_entry = move(entry); }

View File

@ -246,12 +246,10 @@ WebIDL::ExceptionOr<void> DOMURL::set_protocol(String const& protocol)
}
// https://url.spec.whatwg.org/#dom-url-username
WebIDL::ExceptionOr<String> DOMURL::username() const
String const& DOMURL::username() const
{
auto& vm = realm().vm();
// The username getter steps are to return thiss URLs username.
return TRY_OR_THROW_OOM(vm, m_url.username());
return m_url.username();
}
// https://url.spec.whatwg.org/#ref-for-dom-url-username%E2%91%A0
@ -266,12 +264,10 @@ void DOMURL::set_username(String const& username)
}
// https://url.spec.whatwg.org/#dom-url-password
WebIDL::ExceptionOr<String> DOMURL::password() const
String const& DOMURL::password() const
{
auto& vm = realm().vm();
// The password getter steps are to return thiss URLs password.
return TRY_OR_THROW_OOM(vm, m_url.password());
return m_url.password();
}
// https://url.spec.whatwg.org/#ref-for-dom-url-password%E2%91%A0

View File

@ -40,10 +40,10 @@ public:
WebIDL::ExceptionOr<String> protocol() const;
WebIDL::ExceptionOr<void> set_protocol(String const&);
WebIDL::ExceptionOr<String> username() const;
String const& username() const;
void set_username(String const&);
WebIDL::ExceptionOr<String> password() const;
String const& password() const;
void set_password(String const&);
WebIDL::ExceptionOr<String> host() const;

View File

@ -1810,7 +1810,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<PendingResponse>> http_network_or_cache_fet
// true, set authorizationValue to httpRequests current URL, converted to an `Authorization` value.
else if (http_request->current_url().includes_credentials() && is_authentication_fetch == IsAuthenticationFetch::Yes) {
auto const& url = http_request->current_url();
auto payload = MUST(String::formatted("{}:{}", MUST(url.username()), MUST(url.password())));
auto payload = MUST(String::formatted("{}:{}", URL::percent_decode(url.username()), URL::percent_decode(url.password())));
authorization_value = TRY_OR_THROW_OOM(vm, encode_base64(payload.bytes()));
}

View File

@ -41,8 +41,8 @@ bool url_matches_about_blank(URL::URL const& url)
// A URL matches about:blank if its scheme is "about", its path contains a single string "blank", its username and password are the empty string, and its host is null.
return url.scheme() == "about"sv
&& url.serialize_path() == "blank"sv
&& url.raw_username().is_empty()
&& url.raw_password().is_empty()
&& url.username().is_empty()
&& url.password().is_empty()
&& url.host().has<Empty>();
}
@ -53,8 +53,8 @@ bool url_matches_about_srcdoc(URL::URL const& url)
return url.scheme() == "about"sv
&& url.serialize_path() == "srcdoc"sv
&& !url.query().has_value()
&& url.raw_username().is_empty()
&& url.raw_password().is_empty()
&& url.username().is_empty()
&& url.password().is_empty()
&& url.host().has<Empty>();
}

View File

@ -97,7 +97,7 @@ String HTMLHyperlinkElementUtils::username() const
return String {};
// 3. Return this element's url's username.
return m_url->username().release_value();
return m_url->username();
}
// https://html.spec.whatwg.org/multipage/links.html#dom-hyperlink-username
@ -134,7 +134,7 @@ String HTMLHyperlinkElementUtils::password() const
return String {};
// 4. Return url's password.
return url->password().release_value();
return url->password();
}
// https://html.spec.whatwg.org/multipage/links.html#dom-hyperlink-password

View File

@ -135,8 +135,8 @@ bool can_have_its_url_rewritten(DOM::Document const& document, URL::URL const& t
// 2. If targetURL and documentURL differ in their scheme, username, password, host, or port components,
// then return false.
if (target_url.scheme() != document_url.scheme()
|| target_url.raw_username() != document_url.raw_username()
|| target_url.raw_password() != document_url.raw_password()
|| target_url.username() != document_url.username()
|| target_url.password() != document_url.password()
|| target_url.host() != document_url.host()
|| target_url.port() != document_url.port())
return false;