From 21fe86d235845626383fc321d1125d4a2c6db98a Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Sat, 12 Aug 2023 19:28:19 +1200 Subject: [PATCH] AK: Port URL::m_query from DeprecatedString to String --- AK/URL.cpp | 18 +++---------- AK/URL.h | 6 ++--- AK/URLParser.cpp | 13 +++++---- Tests/AK/TestURL.cpp | 27 +++++++++---------- Userland/Libraries/LibHTTP/HttpRequest.cpp | 6 ++--- .../Libraries/LibWeb/HTML/HTMLFormElement.cpp | 14 +++++----- .../LibWeb/HTML/HTMLHyperlinkElementUtils.cpp | 4 +-- Userland/Libraries/LibWeb/HTML/Location.cpp | 2 +- .../Libraries/LibWeb/HTML/WorkerLocation.cpp | 4 +-- Userland/Libraries/LibWeb/URL/URL.cpp | 14 +++++----- Userland/Libraries/LibWeb/URL/URL.h | 2 +- .../Libraries/LibWebSocket/ConnectionInfo.cpp | 7 ++--- Userland/Services/LaunchServer/Launcher.cpp | 19 ++++++------- Userland/Utilities/bt.cpp | 2 +- 14 files changed, 63 insertions(+), 75 deletions(-) diff --git a/AK/URL.cpp b/AK/URL.cpp index eedde740342..b2382ef6b19 100644 --- a/AK/URL.cpp +++ b/AK/URL.cpp @@ -62,11 +62,6 @@ DeprecatedString URL::basename() const return percent_decode(last_segment); } -DeprecatedString URL::query() const -{ - return m_query; -} - DeprecatedString URL::fragment() const { return percent_decode(m_fragment); @@ -145,11 +140,6 @@ void URL::append_path(StringView path) m_paths.append(deprecated_string_percent_encode(path, PercentEncodeSet::Path)); } -void URL::set_query(StringView query) -{ - m_query = deprecated_string_percent_encode(query, is_special() ? PercentEncodeSet::SpecialQuery : PercentEncodeSet::Query); -} - void URL::set_fragment(StringView fragment) { m_fragment = deprecated_string_percent_encode(fragment, PercentEncodeSet::Fragment); @@ -346,9 +336,9 @@ DeprecatedString URL::serialize(ExcludeFragment exclude_fragment) const } // 5. If url’s query is non-null, append U+003F (?), followed by url’s query, to output. - if (!m_query.is_null()) { + if (m_query.has_value()) { output.append('?'); - output.append(m_query); + output.append(*m_query); } // 6. If exclude fragment is false and url’s fragment is non-null, then append U+0023 (#), followed by url’s fragment, to output. @@ -391,9 +381,9 @@ DeprecatedString URL::serialize_for_display() const } } - if (!m_query.is_null()) { + if (m_query.has_value()) { builder.append('?'); - builder.append(m_query); + builder.append(*m_query); } if (!m_fragment.is_null()) { diff --git a/AK/URL.h b/AK/URL.h index 00e535ef18a..2dab5cf6784 100644 --- a/AK/URL.h +++ b/AK/URL.h @@ -82,7 +82,7 @@ public: Host const& host() const { return m_host; } ErrorOr serialized_host() const; DeprecatedString basename() const; - DeprecatedString query() const; + Optional const& query() const { return m_query; } // NOTE: fragment() is percent-decoded, raw_fragment() is not. DeprecatedString fragment() const; DeprecatedString raw_fragment() const; @@ -103,7 +103,7 @@ public: void set_host(Host); void set_port(Optional); void set_paths(Vector const&); - void set_query(StringView); + void set_query(Optional query) { m_query = move(query); } void set_fragment(StringView fragment); void set_cannot_be_a_base_url(bool value) { m_cannot_be_a_base_url = value; } void append_path(StringView); @@ -182,7 +182,7 @@ private: Vector m_paths; // A URL’s query is either null or an ASCII string. It is initially null. - DeprecatedString m_query; + Optional m_query; // A URL’s fragment is either null or an ASCII string that can be used for further processing on the resource the URL’s other components identify. It is initially null. DeprecatedString m_fragment; diff --git a/AK/URLParser.cpp b/AK/URLParser.cpp index 8a90521d5d9..d6b3ede51dd 100644 --- a/AK/URLParser.cpp +++ b/AK/URLParser.cpp @@ -994,7 +994,7 @@ URL URLParser::basic_parse(StringView raw_input, Optional const& base_url, // 2. If c is U+003F (?), then set url’s query to the empty string, and state to query state. if (code_point == '?') { - url->m_query = ""; + url->m_query = String {}; state = State::Query; } // 3. Otherwise, if c is U+0023 (#), set url’s fragment to the empty string and state to fragment state. @@ -1304,7 +1304,7 @@ URL URLParser::basic_parse(StringView raw_input, Optional const& base_url, // 2. If c is U+003F (?), then set url’s query to the empty string and state to query state. if (code_point == '?') { - url->m_query = ""; + url->m_query = String {}; state = State::Query; } // 3. Otherwise, if c is U+0023 (#), set url’s fragment to the empty string and state to fragment state. @@ -1445,7 +1445,7 @@ URL URLParser::basic_parse(StringView raw_input, Optional const& base_url, } // 2. Otherwise, if state override is not given and c is U+003F (?), set url’s query to the empty string and state to query state. else if (!state_override.has_value() && code_point == '?') { - url->m_query = ""; + url->m_query = String {}; state = State::Query; } // 3. Otherwise, if state override is not given and c is U+0023 (#), set url’s fragment to the empty string and state to fragment state. @@ -1514,7 +1514,7 @@ URL URLParser::basic_parse(StringView raw_input, Optional const& base_url, // 6. If c is U+003F (?), then set url’s query to the empty string and state to query state. if (code_point == '?') { - url->m_query = ""; + url->m_query = String {}; state = State::Query; } // 7. If c is U+0023 (#), then set url’s fragment to the empty string and state to fragment state. @@ -1543,7 +1543,7 @@ URL URLParser::basic_parse(StringView raw_input, Optional const& base_url, // 1. If c is U+003F (?), then set url’s query to the empty string and state to query state. if (code_point == '?') { url->m_paths[0] = buffer.string_view(); - url->m_query = ""; + url->m_query = String {}; buffer.clear(); state = State::Query; } @@ -1584,14 +1584,13 @@ URL URLParser::basic_parse(StringView raw_input, Optional const& base_url, // * c is the EOF code point if ((!state_override.has_value() && code_point == '#') || code_point == end_of_file) { - VERIFY(url->m_query == ""); // then: // 1. Let queryPercentEncodeSet be the special-query percent-encode set if url is special; otherwise the query percent-encode set. auto query_percent_encode_set = url->is_special() ? URL::PercentEncodeSet::SpecialQuery : URL::PercentEncodeSet::Query; // 2. Percent-encode after encoding, with encoding, buffer, and queryPercentEncodeSet, and append the result to url’s query. - url->m_query = percent_encode_after_encoding(buffer.string_view(), query_percent_encode_set); + url->m_query = String::from_deprecated_string(percent_encode_after_encoding(buffer.string_view(), query_percent_encode_set)).release_value_but_fixme_should_propagate_errors(); // 3. Set buffer to the empty string. buffer.clear(); diff --git a/Tests/AK/TestURL.cpp b/Tests/AK/TestURL.cpp index 5a391c99ea2..4c327d58847 100644 --- a/Tests/AK/TestURL.cpp +++ b/Tests/AK/TestURL.cpp @@ -24,7 +24,7 @@ TEST_CASE(basic) EXPECT_EQ(MUST(url.serialized_host()), "www.serenityos.org"); EXPECT_EQ(url.port_or_default(), 80); EXPECT_EQ(url.serialize_path(), "/"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); } { @@ -34,7 +34,7 @@ TEST_CASE(basic) EXPECT_EQ(MUST(url.serialized_host()), "www.serenityos.org"); EXPECT_EQ(url.port_or_default(), 443); EXPECT_EQ(url.serialize_path(), "/index.html"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); } { @@ -44,7 +44,7 @@ TEST_CASE(basic) EXPECT_EQ(MUST(url.serialized_host()), "www.serenityos.org1"); EXPECT_EQ(url.port_or_default(), 443); EXPECT_EQ(url.serialize_path(), "/index.html"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); } { @@ -54,7 +54,7 @@ TEST_CASE(basic) EXPECT_EQ(MUST(url.serialized_host()), "localhost"); EXPECT_EQ(url.port_or_default(), 1234); EXPECT_EQ(url.serialize_path(), "/~anon/test/page.html"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); } { @@ -84,7 +84,7 @@ TEST_CASE(basic) EXPECT_EQ(MUST(url.serialized_host()), "www.serenityos.org"); EXPECT_EQ(url.port_or_default(), 80); EXPECT_EQ(url.serialize_path(), "/index.html"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT_EQ(url.fragment(), "fragment"); } { @@ -130,7 +130,7 @@ TEST_CASE(file_url_with_hostname) EXPECT_EQ(url.port_or_default(), 0); EXPECT_EQ(url.serialize_path(), "/my/file"); EXPECT_EQ(url.serialize(), "file://courage/my/file"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); } @@ -160,7 +160,7 @@ TEST_CASE(file_url_with_encoded_characters) EXPECT(url.is_valid()); EXPECT_EQ(url.scheme(), "file"); EXPECT_EQ(url.serialize_path(), "/my/file/test#file.txt"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); } @@ -170,7 +170,7 @@ TEST_CASE(file_url_with_fragment) EXPECT(url.is_valid()); EXPECT_EQ(url.scheme(), "file"); EXPECT_EQ(url.serialize_path(), "/my/file"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT_EQ(url.fragment(), "fragment"); } @@ -205,7 +205,7 @@ TEST_CASE(about_url) EXPECT_EQ(url.scheme(), "about"); EXPECT(url.host().has()); EXPECT_EQ(url.serialize_path(), "blank"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); EXPECT_EQ(url.serialize(), "about:blank"); } @@ -219,7 +219,7 @@ TEST_CASE(mailto_url) EXPECT_EQ(url.port_or_default(), 0); EXPECT_EQ(url.path_segment_count(), 1u); EXPECT_EQ(url.path_segment_at_index(0), "mail@example.com"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); EXPECT_EQ(url.serialize(), "mailto:mail@example.com"); } @@ -379,7 +379,7 @@ TEST_CASE(create_with_file_scheme) EXPECT_EQ(url.path_segment_at_index(1), "anon"); EXPECT_EQ(url.path_segment_at_index(2), "README.md"); EXPECT_EQ(url.serialize_path(), "/home/anon/README.md"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); url = URL::create_with_file_scheme("/home/anon/"); @@ -402,8 +402,7 @@ TEST_CASE(complete_url) EXPECT_EQ(url.scheme(), "http"); EXPECT_EQ(MUST(url.serialized_host()), "serenityos.org"); EXPECT_EQ(url.serialize_path(), "/test.html"); - EXPECT(url.query().is_null()); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT_EQ(url.cannot_be_a_base_url(), false); EXPECT(base_url.complete_url("../index.html#fragment"sv).equals(base_url)); @@ -435,7 +434,7 @@ TEST_CASE(unicode) URL url { "http://example.com/_ünicöde_téxt_©"sv }; EXPECT(url.is_valid()); EXPECT_EQ(url.serialize_path(), "/_ünicöde_téxt_©"); - EXPECT(url.query().is_null()); + EXPECT(!url.query().has_value()); EXPECT(url.fragment().is_null()); } diff --git a/Userland/Libraries/LibHTTP/HttpRequest.cpp b/Userland/Libraries/LibHTTP/HttpRequest.cpp index 8af802ed06d..60d8dd05d9e 100644 --- a/Userland/Libraries/LibHTTP/HttpRequest.cpp +++ b/Userland/Libraries/LibHTTP/HttpRequest.cpp @@ -53,9 +53,9 @@ ErrorOr HttpRequest::to_raw_request() const auto path = m_url.serialize_path(); VERIFY(!path.is_empty()); TRY(builder.try_append(URL::percent_encode(path, URL::PercentEncodeSet::EncodeURI))); - if (!m_url.query().is_empty()) { + if (m_url.query().has_value()) { TRY(builder.try_append('?')); - TRY(builder.try_append(m_url.query())); + TRY(builder.try_append(*m_url.query())); } TRY(builder.try_append(" HTTP/1.1\r\nHost: "sv)); TRY(builder.try_append(TRY(m_url.serialized_host()))); @@ -230,7 +230,7 @@ ErrorOr HttpRequest::from_raw_request(Read if (url_parts.size() == 2) { request.m_resource = url_parts[0]; request.m_url.set_paths({ url_parts[0] }); - request.m_url.set_query(url_parts[1]); + request.m_url.set_query(String::from_deprecated_string(url_parts[1]).release_value_but_fixme_should_propagate_errors()); } else { request.m_resource = resource; request.m_url.set_paths({ resource }); diff --git a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp index b72ca9446e5..033509500fd 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp @@ -616,7 +616,7 @@ ErrorOr HTMLFormElement::mutate_action_url(AK::URL parsed_action, Vector HTMLFormElement::mail_with_headers(AK::URL parsed_action, Vector HTMLFormElement::mail_as_body(AK::URL parsed_action, Vectoris_empty()) TRY(query_builder.try_append('&')); // 5. Append "body=" to parsed action's query. @@ -768,7 +768,7 @@ ErrorOr HTMLFormElement::mail_as_body(AK::URL parsed_action, Vectorquery().is_null() || m_url->query().is_empty()) + if (!m_url.has_value() || m_url->query().has_value() || m_url->query()->is_empty()) return DeprecatedString::empty(); // 4. Return "?", followed by url's query. @@ -358,7 +358,7 @@ void HTMLHyperlinkElementUtils::set_search(DeprecatedString search) // 2. Set url's query to the empty string. auto url_copy = m_url; // We copy the URL here to follow other browser's behavior of reverting the search change if the parse failed. - url_copy->set_query(DeprecatedString::empty()); + url_copy->set_query(String {}); // 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::basic_parse(input, {}, move(url_copy), URLParser::State::Query); diff --git a/Userland/Libraries/LibWeb/HTML/Location.cpp b/Userland/Libraries/LibWeb/HTML/Location.cpp index de13e639178..278159a5c0a 100644 --- a/Userland/Libraries/LibWeb/HTML/Location.cpp +++ b/Userland/Libraries/LibWeb/HTML/Location.cpp @@ -253,7 +253,7 @@ WebIDL::ExceptionOr Location::search() const auto url = this->url(); // 2. If this's url's query is either null or the empty string, return the empty string. - if (url.query().is_empty()) + if (!url.query().has_value() || url.query()->is_empty()) return String {}; // 3. Return "?", followed by this's url's query. diff --git a/Userland/Libraries/LibWeb/HTML/WorkerLocation.cpp b/Userland/Libraries/LibWeb/HTML/WorkerLocation.cpp index 9f012f4d79e..bc534a3984a 100644 --- a/Userland/Libraries/LibWeb/HTML/WorkerLocation.cpp +++ b/Userland/Libraries/LibWeb/HTML/WorkerLocation.cpp @@ -106,11 +106,11 @@ WebIDL::ExceptionOr WorkerLocation::search() const auto const& query = m_global_scope->url().query(); // 2. If query is either null or the empty string, return the empty string. - if (query.is_empty()) + if (!query.has_value() || query->is_empty()) return String {}; // 3. Return "?", followed by query. - return TRY_OR_THROW_OOM(vm, String::formatted("?{}", query.view())); + return TRY_OR_THROW_OOM(vm, String::formatted("?{}", *query)); } // https://html.spec.whatwg.org/multipage/workers.html#dom-workerlocation-hash diff --git a/Userland/Libraries/LibWeb/URL/URL.cpp b/Userland/Libraries/LibWeb/URL/URL.cpp index 7e68f93dca5..370826c6ef8 100644 --- a/Userland/Libraries/LibWeb/URL/URL.cpp +++ b/Userland/Libraries/LibWeb/URL/URL.cpp @@ -50,8 +50,6 @@ static Optional parse_api_url(String const& url, Optional const // https://url.spec.whatwg.org/#dom-url-url WebIDL::ExceptionOr> URL::construct_impl(JS::Realm& realm, String const& url, Optional const& base) { - auto& vm = realm.vm(); - // 1. Let parsedURL be the result of running the API URL parser on url with base, if given. auto parsed_url = parse_api_url(url, base); @@ -60,7 +58,7 @@ WebIDL::ExceptionOr> URL::construct_impl(JS::Realm& realm, return WebIDL::SimpleException { WebIDL::SimpleExceptionType::TypeError, "Invalid URL"sv }; // 3. Let query be parsedURL’s query, if that is non-null, and the empty string otherwise. - auto query = parsed_url->query().is_null() ? String {} : TRY_OR_THROW_OOM(vm, String::from_deprecated_string(parsed_url->query())); + auto query = parsed_url->query().value_or(String {}); // 4. Set this’s URL to parsedURL. // 5. Set this’s query object to a new URLSearchParams object. @@ -182,8 +180,8 @@ WebIDL::ExceptionOr URL::set_href(String const& href) auto query = m_url.query(); // 6. If query is non-null, then set this’s query object’s list to the result of parsing query. - if (!query.is_null()) - m_query->m_list = TRY_OR_THROW_OOM(vm, url_decode(query)); + if (query.has_value()) + m_query->m_list = TRY_OR_THROW_OOM(vm, url_decode(*query)); return {}; } @@ -382,11 +380,11 @@ WebIDL::ExceptionOr URL::search() const auto& vm = realm().vm(); // 1. If this’s URL’s query is either null or the empty string, then return the empty string. - if (m_url.query().is_null() || m_url.query().is_empty()) + if (!m_url.query().has_value() || m_url.query()->is_empty()) return String {}; // 2. Return U+003F (?), followed by this’s URL’s query. - return TRY_OR_THROW_OOM(vm, String::formatted("?{}", m_url.query())); + return TRY_OR_THROW_OOM(vm, String::formatted("?{}", *m_url.query())); } // https://url.spec.whatwg.org/#ref-for-dom-url-search%E2%91%A0 @@ -417,7 +415,7 @@ WebIDL::ExceptionOr URL::set_search(String const& search) // 4. Set url’s query to the empty string. auto url_copy = url; // We copy the URL here to follow other browser's behavior of reverting the search change if the parse failed. - url_copy.set_query(DeprecatedString::empty()); + url_copy.set_query(String {}); // 5. Basic URL parse input with url as url and query state as state override. auto result_url = URLParser::basic_parse(input, {}, move(url_copy), URLParser::State::Query); diff --git a/Userland/Libraries/LibWeb/URL/URL.h b/Userland/Libraries/LibWeb/URL/URL.h index b236a85ad17..ca67ebc0d5e 100644 --- a/Userland/Libraries/LibWeb/URL/URL.h +++ b/Userland/Libraries/LibWeb/URL/URL.h @@ -65,7 +65,7 @@ public: WebIDL::ExceptionOr to_json() const; - void set_query(Badge, StringView query) { m_url.set_query(query); } + void set_query(Badge, Optional query) { m_url.set_query(move(query)); } private: URL(JS::Realm&, AK::URL, JS::NonnullGCPtr query); diff --git a/Userland/Libraries/LibWebSocket/ConnectionInfo.cpp b/Userland/Libraries/LibWebSocket/ConnectionInfo.cpp index d6bc7036a40..cd3a29d73c4 100644 --- a/Userland/Libraries/LibWebSocket/ConnectionInfo.cpp +++ b/Userland/Libraries/LibWebSocket/ConnectionInfo.cpp @@ -32,10 +32,11 @@ DeprecatedString ConnectionInfo::resource_name() const // The path component builder.append(path); // "?" if the query component is non-empty - if (!m_url.query().is_empty()) + if (m_url.query().has_value() && !m_url.query()->is_empty()) { builder.append('?'); - // the query component - builder.append(m_url.query()); + // the query component + builder.append(*m_url.query()); + } return builder.to_deprecated_string(); } diff --git a/Userland/Services/LaunchServer/Launcher.cpp b/Userland/Services/LaunchServer/Launcher.cpp index 34a96eb472a..f3720d84a03 100644 --- a/Userland/Services/LaunchServer/Launcher.cpp +++ b/Userland/Services/LaunchServer/Launcher.cpp @@ -393,15 +393,16 @@ bool Launcher::open_file_url(const URL& url) Vector additional_parameters; DeprecatedString filepath = url.serialize_path(); - auto parameters = url.query().split('&'); - for (auto const& parameter : parameters) { - auto pair = parameter.split('='); - if (pair.size() == 2 && pair[0] == "line_number") { - auto line = pair[1].to_int(); - if (line.has_value()) - // TextEditor uses file:line:col to open a file at a specific line number - filepath = DeprecatedString::formatted("{}:{}", filepath, line.value()); - } + if (url.query().has_value()) { + url.query()->bytes_as_string_view().for_each_split_view('&', SplitBehavior::Nothing, [&](auto parameter) { + auto pair = parameter.split_view('='); + if (pair.size() == 2 && pair[0] == "line_number") { + auto line = pair[1].to_int(); + if (line.has_value()) + // TextEditor uses file:line:col to open a file at a specific line number + filepath = DeprecatedString::formatted("{}:{}", filepath, line.value()); + } + }); } additional_parameters.append(filepath); diff --git a/Userland/Utilities/bt.cpp b/Userland/Utilities/bt.cpp index d16453629c9..543feea17ec 100644 --- a/Userland/Utilities/bt.cpp +++ b/Userland/Utilities/bt.cpp @@ -58,7 +58,7 @@ ErrorOr serenity_main(Main::Arguments arguments) if (access(full_path.characters(), F_OK) == 0) { linked = true; auto url = URL::create_with_file_scheme(full_path, {}, hostname); - url.set_query(DeprecatedString::formatted("line_number={}", source_position.line_number)); + url.set_query(TRY(String::formatted("line_number={}", source_position.line_number))); out("\033]8;;{}\033\\", url.serialize()); }