AK+Everywhere: Use Optional for URLParser::parse's base_url parameter

networkException · linusg · commit 9915fa72fbb1 · 2023-04-11T16:28:20.000+02:00
diff --git a/AK/URL.cpp b/AK/URL.cpp
@@ -44,7 +44,7 @@ URL URL::complete_url(StringView relative_url) const
     if (!is_valid())
         return {};
 
-    return URLParser::parse(relative_url, this);
+    return URLParser::parse(relative_url, *this);
 }
 
 void URL::set_scheme(DeprecatedString scheme)
diff --git a/AK/URLParser.cpp b/AK/URLParser.cpp
@@ -197,11 +197,11 @@ Optional<URL> URLParser::parse_data_url(StringView raw_input)
 // NOTE: Since the URL class's member variables contain percent decoded data, we have to deviate from the URL parser specification when setting
 //       some of those values. Because the specification leaves all values percent encoded in their URL data structure, we have to percent decode
 //       everything before setting the member variables.
-URL URLParser::parse(StringView raw_input, URL const* base_url, Optional<URL> url, Optional<State> state_override)
+URL URLParser::parse(StringView raw_input, Optional<URL> const& base_url, Optional<URL> url, Optional<State> state_override)
 {
     dbgln_if(URL_PARSER_DEBUG, "URLParser::parse: Parsing '{}'", raw_input);
     if (raw_input.is_empty())
-        return base_url ? *base_url : URL {};
+        return base_url.has_value() ? *base_url : URL {};
 
     if (raw_input.starts_with("data:"sv)) {
         auto maybe_url = parse_data_url(raw_input);
@@ -301,7 +301,7 @@ URL URLParser::parse(StringView raw_input, URL const* base_url, Optional<URL> ur
                     }
                     state = State::File;
                 } else if (url->is_special()) {
-                    if (base_url && base_url->m_scheme == url->m_scheme)
+                    if (base_url.has_value() && base_url->m_scheme == url->m_scheme)
                         state = State::SpecialRelativeOrAuthority;
                     else
                         state = State::SpecialAuthoritySlashes;
@@ -321,7 +321,7 @@ URL URLParser::parse(StringView raw_input, URL const* base_url, Optional<URL> ur
             }
             break;
         case State::NoScheme:
-            if (!base_url || (base_url->m_cannot_be_a_base_url && code_point != '#')) {
+            if (!base_url.has_value() || (base_url->m_cannot_be_a_base_url && code_point != '#')) {
                 report_validation_error();
                 return {};
             } else if (base_url->m_cannot_be_a_base_url && code_point == '#') {
@@ -527,7 +527,7 @@ URL URLParser::parse(StringView raw_input, URL const* base_url, Optional<URL> ur
                 if (code_point == '\\')
                     report_validation_error();
                 state = State::FileSlash;
-            } else if (base_url && base_url->m_scheme == "file") {
+            } else if (base_url.has_value() && base_url->m_scheme == "file") {
                 url->m_host = base_url->m_host;
                 url->m_paths = base_url->m_paths;
                 url->m_query = base_url->m_query;
@@ -557,7 +557,7 @@ URL URLParser::parse(StringView raw_input, URL const* base_url, Optional<URL> ur
                 if (code_point == '\\')
                     report_validation_error();
                 state = State::FileHost;
-            } else if (base_url && base_url->m_scheme == "file") {
+            } else if (base_url.has_value() && base_url->m_scheme == "file") {
                 url->m_host = base_url->m_host;
                 auto substring_from_pointer = input.substring_view(iterator - input.begin()).as_string();
                 if (!starts_with_windows_drive_letter(substring_from_pointer) && is_normalized_windows_drive_letter(base_url->m_paths[0]))
diff --git a/AK/URLParser.h b/AK/URLParser.h
@@ -55,7 +55,7 @@ class URLParser {
         VERIFY_NOT_REACHED();
     }
 
-    static URL parse(StringView input, URL const* base_url = nullptr, Optional<URL> url = {}, Optional<State> state_override = {});
+    static URL parse(StringView input, Optional<URL> const& base_url = {}, Optional<URL> url = {}, Optional<State> state_override = {});
 
 private:
     static Optional<URL> parse_data_url(StringView raw_input);
diff --git a/Tests/AK/TestURL.cpp b/Tests/AK/TestURL.cpp
@@ -411,7 +411,7 @@ TEST_CASE(complete_file_url_with_base)
 TEST_CASE(empty_url_with_base_url)
 {
     URL base_url { "https://foo.com/"sv };
-    URL parsed_url = URLParser::parse(""sv, &base_url);
+    URL parsed_url = URLParser::parse(""sv, base_url);
     EXPECT_EQ(parsed_url.is_valid(), true);
     EXPECT(base_url.equals(parsed_url));
 }
diff --git a/Userland/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Responses.cpp b/Userland/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Responses.cpp
@@ -113,8 +113,7 @@ ErrorOr<Optional<AK::URL>> Response::location_url(Optional<String> const& reques
         return Optional<AK::URL> {};
 
     // 3. If location is a header value, then set location to the result of parsing location with response’s URL.
-    auto base_url = *url();
-    auto location = AK::URLParser::parse(location_values.first(), &base_url);
+    auto location = AK::URLParser::parse(location_values.first(), url());
     if (!location.is_valid())
         return Error::from_string_view("Invalid 'Location' header URL"sv);
 
diff --git a/Userland/Libraries/LibWeb/Fetch/Request.cpp b/Userland/Libraries/LibWeb/Fetch/Request.cpp
@@ -121,7 +121,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<Request>> Request::construct_impl(JS::Realm
     // 5. If input is a string, then:
     if (input.has<String>()) {
         // 1. Let parsedURL be the result of parsing input with baseURL.
-        auto parsed_url = URLParser::parse(input.get<String>(), &base_url);
+        auto parsed_url = URLParser::parse(input.get<String>(), base_url);
 
         // 2. If parsedURL is failure, then throw a TypeError.
         if (!parsed_url.is_valid())
@@ -299,7 +299,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<Request>> Request::construct_impl(JS::Realm
         // 3. Otherwise:
         else {
             // 1. Let parsedReferrer be the result of parsing referrer with baseURL.
-            auto parsed_referrer = URLParser::parse(referrer, &base_url);
+            auto parsed_referrer = URLParser::parse(referrer, base_url);
 
             // 2. If parsedReferrer is failure, then throw a TypeError.
             if (!parsed_referrer.is_valid())
diff --git a/Userland/Libraries/LibWeb/Fetch/Response.cpp b/Userland/Libraries/LibWeb/Fetch/Response.cpp
@@ -174,7 +174,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<Response>> Response::redirect(JS::VM& vm, S
 
     // 1. Let parsedURL be the result of parsing url with current settings object’s API base URL.
     auto api_base_url = HTML::current_settings_object().api_base_url();
-    auto parsed_url = URLParser::parse(url, &api_base_url);
+    auto parsed_url = URLParser::parse(url, api_base_url);
 
     // 2. If parsedURL is failure, then throw a TypeError.
     if (!parsed_url.is_valid())
diff --git a/Userland/Libraries/LibWeb/HTML/HTMLHyperlinkElementUtils.cpp b/Userland/Libraries/LibWeb/HTML/HTMLHyperlinkElementUtils.cpp
@@ -80,7 +80,7 @@ void HTMLHyperlinkElementUtils::set_protocol(DeprecatedString protocol)
         return;
 
     // 3. Basic URL parse the given value, followed by ":", with this element's url as url and scheme start state as state override.
-    auto result_url = URLParser::parse(DeprecatedString::formatted("{}:", protocol), nullptr, m_url, URLParser::State::SchemeStart);
+    auto result_url = URLParser::parse(DeprecatedString::formatted("{}:", protocol), {}, m_url, URLParser::State::SchemeStart);
     if (result_url.is_valid())
         m_url = move(result_url);
 
@@ -194,7 +194,7 @@ void HTMLHyperlinkElementUtils::set_host(DeprecatedString host)
         return;
 
     // 4. Basic URL parse the given value, with url as url and host state as state override.
-    auto result_url = URLParser::parse(host, nullptr, url, URLParser::State::Host);
+    auto result_url = URLParser::parse(host, {}, url, URLParser::State::Host);
     if (result_url.is_valid())
         m_url = move(result_url);
 
@@ -227,7 +227,7 @@ void HTMLHyperlinkElementUtils::set_hostname(DeprecatedString hostname)
         return;
 
     // 4. Basic URL parse the given value, with url as url and hostname state as state override.
-    auto result_url = URLParser::parse(hostname, nullptr, m_url, URLParser::State::Hostname);
+    auto result_url = URLParser::parse(hostname, {}, m_url, URLParser::State::Hostname);
     if (result_url.is_valid())
         m_url = move(result_url);
 
@@ -269,7 +269,7 @@ void HTMLHyperlinkElementUtils::set_port(DeprecatedString port)
         m_url->set_port({});
     } else {
         // 5. Otherwise, basic URL parse the given value, with url as url and port state as state override.
-        auto result_url = URLParser::parse(port, nullptr, m_url, URLParser::State::Port);
+        auto result_url = URLParser::parse(port, {}, m_url, URLParser::State::Port);
         if (result_url.is_valid())
             m_url = move(result_url);
     }
@@ -313,7 +313,7 @@ void HTMLHyperlinkElementUtils::set_pathname(DeprecatedString pathname)
     url->set_paths({});
 
     // 5. Basic URL parse the given value, with url as url and path start state as state override.
-    auto result_url = URLParser::parse(pathname, nullptr, move(url), URLParser::State::PathStart);
+    auto result_url = URLParser::parse(pathname, {}, move(url), URLParser::State::PathStart);
     if (result_url.is_valid())
         m_url = move(result_url);
 
@@ -360,7 +360,7 @@ void HTMLHyperlinkElementUtils::set_search(DeprecatedString search)
         url_copy->set_query(DeprecatedString::empty());
 
         //    3. Basic URL parse input, with null, this element's node document's document's character encoding, url as url, and query state as state override.
-        auto result_url = URLParser::parse(input, nullptr, move(url_copy), URLParser::State::Query);
+        auto result_url = URLParser::parse(input, {}, move(url_copy), URLParser::State::Query);
         if (result_url.is_valid())
             m_url = move(result_url);
     }
@@ -408,7 +408,7 @@ void HTMLHyperlinkElementUtils::set_hash(DeprecatedString hash)
         url_copy->set_fragment(DeprecatedString::empty());
 
         //    3. Basic URL parse input, with url as url and fragment state as state override.
-        auto result_url = URLParser::parse(input, nullptr, move(url_copy), URLParser::State::Fragment);
+        auto result_url = URLParser::parse(input, {}, move(url_copy), URLParser::State::Fragment);
         if (result_url.is_valid())
             m_url = move(result_url);
     }
diff --git a/Userland/Libraries/LibWeb/HTML/Location.cpp b/Userland/Libraries/LibWeb/HTML/Location.cpp
@@ -312,7 +312,7 @@ WebIDL::ExceptionOr<void> Location::set_hash(String const& value)
     copy_url.set_fragment("");
 
     // 6. Basic URL parse input, with copyURL as url and fragment state as state override.
-    auto result_url = URLParser::parse(input, nullptr, copy_url, URLParser::State::Fragment);
+    auto result_url = URLParser::parse(input, {}, copy_url, URLParser::State::Fragment);
 
     // 7. If copyURL's fragment is this's url's fragment, then return.
     if (copy_url.fragment() == this->url().fragment())
diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp
@@ -159,7 +159,7 @@ WebIDL::ExceptionOr<Optional<AK::URL>> resolve_imports_match(DeprecatedString co
             VERIFY(resolution_result->serialize().ends_with("/"sv));
 
             // 5. Let url be the result of URL parsing afterPrefix with resolutionResult.
-            auto url = URLParser::parse(after_prefix, &*resolution_result);
+            auto url = URLParser::parse(after_prefix, *resolution_result);
 
             // 6. If url is failure, then throw a TypeError indicating that resolution of normalizedSpecifier was blocked since the afterPrefix portion
             //    could not be URL-parsed relative to the resolutionResult mapped to by the specifierKey prefix.
@@ -189,7 +189,7 @@ Optional<AK::URL> resolve_url_like_module_specifier(DeprecatedString const& spec
     // 1. If specifier starts with "/", "./", or "../", then:
     if (specifier.starts_with("/"sv) || specifier.starts_with("./"sv) || specifier.starts_with("../"sv)) {
         // 1. Let url be the result of URL parsing specifier with baseURL.
-        auto url = URLParser::parse(specifier, &base_url);
+        auto url = URLParser::parse(specifier, base_url);
 
         // 2. If url is failure, then return null.
         if (!url.is_valid())
diff --git a/Userland/Libraries/LibWeb/URL/URL.cpp b/Userland/Libraries/LibWeb/URL/URL.cpp
@@ -135,7 +135,7 @@ WebIDL::ExceptionOr<void> URL::set_protocol(String const& protocol)
     auto& vm = realm().vm();
 
     // basic URL parse the given value, followed by U+003A (:), with this’s URL as url and scheme start state as state override.
-    auto result_url = URLParser::parse(TRY_OR_THROW_OOM(vm, String::formatted("{}:", protocol)), nullptr, m_url, URLParser::State::SchemeStart);
+    auto result_url = URLParser::parse(TRY_OR_THROW_OOM(vm, String::formatted("{}:", protocol)), {}, m_url, URLParser::State::SchemeStart);
     if (result_url.is_valid())
         m_url = move(result_url);
     return {};
@@ -197,7 +197,7 @@ void URL::set_host(String const& host)
     if (m_url.cannot_be_a_base_url())
         return;
     // 2. Basic URL parse the given value with this’s URL as url and host state as state override.
-    auto result_url = URLParser::parse(host, nullptr, m_url, URLParser::State::Host);
+    auto result_url = URLParser::parse(host, {}, m_url, URLParser::State::Host);
     if (result_url.is_valid())
         m_url = move(result_url);
 }
@@ -219,7 +219,7 @@ void URL::set_hostname(String const& hostname)
     if (m_url.cannot_be_a_base_url())
         return;
     // 2. Basic URL parse the given value with this’s URL as url and hostname state as state override.
-    auto result_url = URLParser::parse(hostname, nullptr, m_url, URLParser::State::Hostname);
+    auto result_url = URLParser::parse(hostname, {}, m_url, URLParser::State::Hostname);
     if (result_url.is_valid())
         m_url = move(result_url);
 }
@@ -249,7 +249,7 @@ void URL::set_port(String const& port)
     }
 
     // 3. Otherwise, basic URL parse the given value with this’s URL as url and port state as state override.
-    auto result_url = URLParser::parse(port, nullptr, m_url, URLParser::State::Port);
+    auto result_url = URLParser::parse(port, {}, m_url, URLParser::State::Port);
     if (result_url.is_valid())
         m_url = move(result_url);
 }
@@ -273,7 +273,7 @@ void URL::set_pathname(String const& pathname)
     auto url = m_url; // We copy the URL here to follow other browser's behaviour of reverting the path change if the parse failed.
     url.set_paths({});
     // 3. Basic URL parse the given value with this’s URL as url and path start state as state override.
-    auto result_url = URLParser::parse(pathname, nullptr, move(url), URLParser::State::PathStart);
+    auto result_url = URLParser::parse(pathname, {}, move(url), URLParser::State::PathStart);
     if (result_url.is_valid())
         m_url = move(result_url);
 }
@@ -308,7 +308,7 @@ WebIDL::ExceptionOr<void> URL::set_search(String const& search)
     auto url_copy = url; // We copy the URL here to follow other browser's behaviour of reverting the search change if the parse failed.
     url_copy.set_query(DeprecatedString::empty());
     // 4. Basic URL parse input with url as url and query state as state override.
-    auto result_url = URLParser::parse(input, nullptr, move(url_copy), URLParser::State::Query);
+    auto result_url = URLParser::parse(input, {}, move(url_copy), URLParser::State::Query);
     if (result_url.is_valid()) {
         m_url = move(result_url);
         // 5. Set this’s query object’s list to the result of parsing input.
@@ -348,7 +348,7 @@ void URL::set_hash(String const& hash)
     auto url = m_url; // We copy the URL here to follow other browser's behaviour of reverting the hash change if the parse failed.
     url.set_fragment(DeprecatedString::empty());
     // 4. Basic URL parse input with this’s URL as url and fragment state as state override.
-    auto result_url = URLParser::parse(input, nullptr, move(url), URLParser::State::Fragment);
+    auto result_url = URLParser::parse(input, {}, move(url), URLParser::State::Fragment);
     if (result_url.is_valid())
         m_url = move(result_url);
 }
diff --git a/Userland/Utilities/xml.cpp b/Userland/Utilities/xml.cpp
@@ -364,7 +364,7 @@ static auto parse(StringView contents)
             .preserve_comments = true,
             .resolve_external_resource = [&](XML::SystemID const& system_id, Optional<XML::PublicID> const&) -> ErrorOr<DeprecatedString> {
                 auto base = URL::create_with_file_scheme(s_path);
-                auto url = URLParser::parse(system_id.system_literal, &base);
+                auto url = URLParser::parse(system_id.system_literal, base);
                 if (!url.is_valid())
                     return Error::from_string_literal("Invalid URL");
 

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ URL URL::complete_url(StringView relative_url) const`
`44`	`44`	`if (!is_valid())`
`45`	`45`	`return {};`
`46`	`46`
`47`		`- return URLParser::parse(relative_url, this);`
	`47`	`+ return URLParser::parse(relative_url, *this);`
`48`	`48`	`}`
`49`	`49`
`50`	`50`	`void URL::set_scheme(DeprecatedString scheme)`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ class URLParser {`
`55`	`55`	`VERIFY_NOT_REACHED();`
`56`	`56`	`}`
`57`	`57`
`58`		`- static URL parse(StringView input, URL const* base_url = nullptr, Optional<URL> url = {}, Optional<State> state_override = {});`
	`58`	`+ static URL parse(StringView input, Optional<URL> const& base_url = {}, Optional<URL> url = {}, Optional<State> state_override = {});`
`59`	`59`
`60`	`60`	`private:`
`61`	`61`	`static Optional<URL> parse_data_url(StringView raw_input);`
Original file line number	Diff line number	Diff line change
`@@ -411,7 +411,7 @@ TEST_CASE(complete_file_url_with_base)`
`411`	`411`	`TEST_CASE(empty_url_with_base_url)`
`412`	`412`	`{`
`413`	`413`	`URL base_url { "https://foo.com/"sv };`
`414`		`- URL parsed_url = URLParser::parse(""sv, &base_url);`
	`414`	`+ URL parsed_url = URLParser::parse(""sv, base_url);`
`415`	`415`	`EXPECT_EQ(parsed_url.is_valid(), true);`
`416`	`416`	`EXPECT(base_url.equals(parsed_url));`
`417`	`417`	`}`