Skip to content

Commit

Permalink
Everywhere: Use HTTP::HeaderMap for request headers
Browse files Browse the repository at this point in the history
No longer just for response headers! The same type is obviously useful
and ergonomic when making requests as well.

(cherry picked from commit 260c5c50ad19f19d0d4c30984e512f56c055ecff)

Updated various SerenityOS components to make it build.
  • Loading branch information
awesomekling authored and alimpfard committed Jun 10, 2024
1 parent bec8d73 commit 8994dcb
Show file tree
Hide file tree
Showing 30 changed files with 56 additions and 69 deletions.
10 changes: 5 additions & 5 deletions Ladybird/Qt/RequestManagerQt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ void RequestManagerQt::reply_finished(QNetworkReply* reply)
request->did_finish();
}

RefPtr<Web::ResourceLoaderConnectorRequest> RequestManagerQt::start_request(ByteString const& method, URL::URL const& url, HashMap<ByteString, ByteString> const& request_headers, ReadonlyBytes request_body, Core::ProxyData const& proxy)
RefPtr<Web::ResourceLoaderConnectorRequest> RequestManagerQt::start_request(ByteString const& method, URL::URL const& url, HTTP::HeaderMap const& request_headers, ReadonlyBytes request_body, Core::ProxyData const& proxy)
{
if (!url.scheme().bytes_as_string_view().is_one_of_ignoring_ascii_case("http"sv, "https"sv)) {
return nullptr;
Expand All @@ -39,7 +39,7 @@ RefPtr<Web::ResourceLoaderConnectorRequest> RequestManagerQt::start_request(Byte
return request;
}

ErrorOr<NonnullRefPtr<RequestManagerQt::Request>> RequestManagerQt::Request::create(QNetworkAccessManager& qnam, ByteString const& method, URL::URL const& url, HashMap<ByteString, ByteString> const& request_headers, ReadonlyBytes request_body, Core::ProxyData const&)
ErrorOr<NonnullRefPtr<RequestManagerQt::Request>> RequestManagerQt::Request::create(QNetworkAccessManager& qnam, ByteString const& method, URL::URL const& url, HTTP::HeaderMap const& request_headers, ReadonlyBytes request_body, Core::ProxyData const&)
{
QNetworkRequest request { QString(url.to_byte_string().characters()) };
request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::ManualRedirectPolicy);
Expand All @@ -51,13 +51,13 @@ ErrorOr<NonnullRefPtr<RequestManagerQt::Request>> RequestManagerQt::Request::cre

QNetworkReply* reply = nullptr;

for (auto& it : request_headers) {
for (auto const& it : request_headers.headers()) {
// FIXME: We currently strip the Accept-Encoding header on outgoing requests from LibWeb
// since otherwise it'll ask for compression without Qt being aware of it.
// This is very hackish and I'm sure we can do it in concert with Qt somehow.
if (it.key == "Accept-Encoding")
if (it.name == "Accept-Encoding")
continue;
request.setRawHeader(QByteArray(it.key.characters()), QByteArray(it.value.characters()));
request.setRawHeader(QByteArray(it.name.characters()), QByteArray(it.value.characters()));
}

if (method.equals_ignoring_ascii_case("head"sv)) {
Expand Down
4 changes: 2 additions & 2 deletions Ladybird/Qt/RequestManagerQt.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class RequestManagerQt
virtual void prefetch_dns(URL::URL const&) override { }
virtual void preconnect(URL::URL const&) override { }

virtual RefPtr<Web::ResourceLoaderConnectorRequest> start_request(ByteString const& method, URL::URL const&, HashMap<ByteString, ByteString> const& request_headers, ReadonlyBytes request_body, Core::ProxyData const&) override;
virtual RefPtr<Web::ResourceLoaderConnectorRequest> start_request(ByteString const& method, URL::URL const&, HTTP::HeaderMap const& request_headers, ReadonlyBytes request_body, Core::ProxyData const&) override;
virtual RefPtr<Web::WebSockets::WebSocketClientSocket> websocket_connect(const URL::URL&, ByteString const& origin, Vector<ByteString> const& protocols) override;

private slots:
Expand All @@ -39,7 +39,7 @@ private slots:
class Request
: public Web::ResourceLoaderConnectorRequest {
public:
static ErrorOr<NonnullRefPtr<Request>> create(QNetworkAccessManager& qnam, ByteString const& method, URL::URL const& url, HashMap<ByteString, ByteString> const& request_headers, ReadonlyBytes request_body, Core::ProxyData const&);
static ErrorOr<NonnullRefPtr<Request>> create(QNetworkAccessManager& qnam, ByteString const& method, URL::URL const& url, HTTP::HeaderMap const& request_headers, ReadonlyBytes request_body, Core::ProxyData const&);

virtual ~Request() override;

Expand Down
2 changes: 1 addition & 1 deletion Userland/Applications/Maps/MapWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ void MapWidget::process_tile_queue()
auto tile_key = m_tile_queue.dequeue();

// Start HTTP GET request to load image
HashMap<ByteString, ByteString> headers;
HTTP::HeaderMap headers;
headers.set("User-Agent", "SerenityOS Maps");
headers.set("Accept", "image/png");
URL::URL url(MUST(String::formatted(m_tile_provider.value_or(m_default_tile_provider), tile_key.zoom, tile_key.x, tile_key.y)));
Expand Down
2 changes: 1 addition & 1 deletion Userland/Applications/Maps/SearchPanel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void SearchPanel::search(StringView query)
m_start_container->set_visible(false);

// Start HTTP GET request to load people.json
HashMap<ByteString, ByteString> headers;
HTTP::HeaderMap headers;
headers.set("User-Agent", "SerenityOS Maps");
headers.set("Accept", "application/json");
URL::URL url(MUST(String::formatted("https://nominatim.openstreetmap.org/search?q={}&format=json", URL::percent_encode(query, URL::PercentEncodeSet::Query))));
Expand Down
2 changes: 1 addition & 1 deletion Userland/Applications/Maps/UsersMapWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ UsersMapWidget::UsersMapWidget(Options const& options)
void UsersMapWidget::get_users()
{
// Start HTTP GET request to load people.json
HashMap<ByteString, ByteString> headers;
HTTP::HeaderMap headers;
headers.set("User-Agent", "SerenityOS Maps");
headers.set("Accept", "application/json");
URL::URL url("https://usermap.serenityos.org/people.json");
Expand Down
21 changes: 9 additions & 12 deletions Userland/Libraries/LibHTTP/HttpRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,11 @@ ErrorOr<ByteBuffer> HttpRequest::to_raw_request() const
TRY(builder.try_appendff(":{}", *m_url.port()));
TRY(builder.try_append("\r\n"sv));
// Start headers.
bool has_content_length = false;
for (auto& header : m_headers) {
if (header.name.equals_ignoring_ascii_case("Content-Length"sv))
has_content_length = true;
TRY(builder.try_append(header.name));
bool has_content_length = m_headers.contains("Content-Length"sv);
for (auto const& [name, value] : m_headers.headers()) {
TRY(builder.try_append(name));
TRY(builder.try_append(": "sv));
TRY(builder.try_append(header.value));
TRY(builder.try_append(value));
TRY(builder.try_append("\r\n"sv));
}
if (!m_body.is_empty() || method() == Method::POST) {
Expand Down Expand Up @@ -118,7 +116,7 @@ ErrorOr<HttpRequest, HttpRequest::ParseError> HttpRequest::from_raw_request(Read
ByteString method;
ByteString resource;
ByteString protocol;
Vector<Header> headers;
HeaderMap headers;
Header current_header;
ByteBuffer body;

Expand Down Expand Up @@ -185,7 +183,7 @@ ErrorOr<HttpRequest, HttpRequest::ParseError> HttpRequest::from_raw_request(Read
if (current_header.name.equals_ignoring_ascii_case("Content-Length"sv))
content_length = current_header.value.to_number<unsigned>();

headers.append(move(current_header));
headers.set(move(current_header.name), move(current_header.value));
break;
}
buffer.append(consume());
Expand Down Expand Up @@ -263,13 +261,12 @@ ErrorOr<HttpRequest, HttpRequest::ParseError> HttpRequest::from_raw_request(Read
return request;
}

void HttpRequest::set_headers(HashMap<ByteString, ByteString> const& headers)
void HttpRequest::set_headers(HTTP::HeaderMap headers)
{
for (auto& it : headers)
m_headers.append({ it.key, it.value });
m_headers = move(headers);
}

Optional<HttpRequest::Header> HttpRequest::get_http_basic_authentication_header(URL::URL const& url)
Optional<Header> HttpRequest::get_http_basic_authentication_header(URL::URL const& url)
{
if (!url.includes_credentials())
return {};
Expand Down
12 changes: 4 additions & 8 deletions Userland/Libraries/LibHTTP/HttpRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <AK/Optional.h>
#include <AK/Vector.h>
#include <LibCore/Forward.h>
#include <LibHTTP/HeaderMap.h>
#include <LibURL/URL.h>

namespace HTTP {
Expand Down Expand Up @@ -55,11 +56,6 @@ class HttpRequest {
PUT,
};

struct Header {
ByteString name;
ByteString value;
};

struct BasicAuthenticationCredentials {
ByteString username;
ByteString password;
Expand All @@ -69,7 +65,7 @@ class HttpRequest {
~HttpRequest() = default;

ByteString const& resource() const { return m_resource; }
Vector<Header> const& headers() const { return m_headers; }
HeaderMap const& headers() const { return m_headers; }

URL::URL const& url() const { return m_url; }
void set_url(URL::URL const& url) { m_url = url; }
Expand All @@ -83,7 +79,7 @@ class HttpRequest {
StringView method_name() const;
ErrorOr<ByteBuffer> to_raw_request() const;

void set_headers(HashMap<ByteString, ByteString> const&);
void set_headers(HeaderMap);

static ErrorOr<HttpRequest, HttpRequest::ParseError> from_raw_request(ReadonlyBytes);
static Optional<Header> get_http_basic_authentication_header(URL::URL const&);
Expand All @@ -93,7 +89,7 @@ class HttpRequest {
URL::URL m_url;
ByteString m_resource;
Method m_method { GET };
Vector<Header> m_headers;
HeaderMap m_headers;
ByteBuffer m_body;
};

Expand Down
11 changes: 2 additions & 9 deletions Userland/Libraries/LibProtocol/RequestClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,16 @@ void RequestClient::ensure_connection(URL::URL const& url, ::RequestServer::Cach
async_ensure_connection(url, cache_level);
}

template<typename RequestHashMapTraits>
RefPtr<Request> RequestClient::start_request(ByteString const& method, URL::URL const& url, HashMap<ByteString, ByteString, RequestHashMapTraits> const& request_headers, ReadonlyBytes request_body, Core::ProxyData const& proxy_data)
RefPtr<Request> RequestClient::start_request(ByteString const& method, URL::URL const& url, HTTP::HeaderMap const& request_headers, ReadonlyBytes request_body, Core::ProxyData const& proxy_data)
{
auto headers_or_error = request_headers.template clone<Traits<ByteString>>();
if (headers_or_error.is_error())
return nullptr;
auto body_result = ByteBuffer::copy(request_body);
if (body_result.is_error())
return nullptr;

static i32 s_next_request_id = 0;
auto request_id = s_next_request_id++;

IPCProxy::async_start_request(request_id, method, url, headers_or_error.release_value(), body_result.release_value(), proxy_data);
IPCProxy::async_start_request(request_id, method, url, request_headers, body_result.release_value(), proxy_data);
auto request = Request::create_from_id({}, *this, request_id);
m_requests.set(request_id, request);
return request;
Expand Down Expand Up @@ -146,6 +142,3 @@ void RequestClient::websocket_certificate_requested(i32 connection_id)
}

}

template RefPtr<Protocol::Request> Protocol::RequestClient::start_request(ByteString const& method, URL::URL const&, HashMap<ByteString, ByteString> const& request_headers, ReadonlyBytes request_body, Core::ProxyData const&);
template RefPtr<Protocol::Request> Protocol::RequestClient::start_request(ByteString const& method, URL::URL const&, HashMap<ByteString, ByteString, CaseInsensitiveStringTraits> const& request_headers, ReadonlyBytes request_body, Core::ProxyData const&);
3 changes: 1 addition & 2 deletions Userland/Libraries/LibProtocol/RequestClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ class RequestClient final
public:
explicit RequestClient(NonnullOwnPtr<Core::LocalSocket>);

template<typename RequestHashMapTraits = Traits<ByteString>>
RefPtr<Request> start_request(ByteString const& method, URL::URL const&, HashMap<ByteString, ByteString, RequestHashMapTraits> const& request_headers = {}, ReadonlyBytes request_body = {}, Core::ProxyData const& = {});
RefPtr<Request> start_request(ByteString const& method, URL::URL const&, HTTP::HeaderMap const& request_headers = {}, ReadonlyBytes request_body = {}, Core::ProxyData const& = {});

RefPtr<WebSocket> websocket_connect(const URL::URL&, ByteString const& origin = {}, Vector<ByteString> const& protocols = {}, Vector<ByteString> const& extensions = {}, HashMap<ByteString, ByteString> const& request_headers = {});

Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ RefPtr<ResourceLoaderConnectorRequest> ResourceLoader::start_network_request(Loa
{
auto proxy = ProxyMappings::the().proxy_for_url(request.url());

HashMap<ByteString, ByteString> headers;
HTTP::HeaderMap headers;
headers.set("User-Agent", m_user_agent.to_byte_string());
headers.set("Accept-Encoding", "gzip, deflate, br");

Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibWeb/Loader/ResourceLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class ResourceLoaderConnector : public RefCounted<ResourceLoaderConnector> {
virtual void prefetch_dns(URL::URL const&) = 0;
virtual void preconnect(URL::URL const&) = 0;

virtual RefPtr<ResourceLoaderConnectorRequest> start_request(ByteString const& method, URL::URL const&, HashMap<ByteString, ByteString> const& request_headers = {}, ReadonlyBytes request_body = {}, Core::ProxyData const& = {}) = 0;
virtual RefPtr<ResourceLoaderConnectorRequest> start_request(ByteString const& method, URL::URL const&, HTTP::HeaderMap const& request_headers = {}, ReadonlyBytes request_body = {}, Core::ProxyData const& = {}) = 0;
virtual RefPtr<Web::WebSockets::WebSocketClientSocket> websocket_connect(const URL::URL&, ByteString const& origin, Vector<ByteString> const& protocols) = 0;

protected:
Expand Down
4 changes: 2 additions & 2 deletions Userland/Libraries/LibWeb/WebDriver/Client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ ErrorOr<JsonValue, Client::WrappedError> Client::read_body_as_json()
// FIXME: Check the Content-Type is actually application/json.
size_t content_length = 0;

for (auto const& header : m_request->headers()) {
for (auto const& header : m_request->headers().headers()) {
if (header.name.equals_ignoring_ascii_case("Content-Length"sv)) {
content_length = header.value.to_number<size_t>(TrimWhitespace::Yes).value_or(0);
break;
Expand Down Expand Up @@ -281,7 +281,7 @@ ErrorOr<void, Client::WrappedError> Client::handle_request(JsonValue body)
ErrorOr<void, Client::WrappedError> Client::send_success_response(JsonValue result)
{
bool keep_alive = false;
if (auto it = m_request->headers().find_if([](auto& header) { return header.name.equals_ignoring_ascii_case("Connection"sv); }); !it.is_end())
if (auto it = m_request->headers().headers().find_if([](auto& header) { return header.name.equals_ignoring_ascii_case("Connection"sv); }); !it.is_end())
keep_alive = it->value.trim_whitespace().equals_ignoring_ascii_case("keep-alive"sv);

result = make_success_response(move(result));
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibWebView/RequestServerAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ RequestServerAdapter::RequestServerAdapter(NonnullRefPtr<Protocol::RequestClient

RequestServerAdapter::~RequestServerAdapter() = default;

RefPtr<Web::ResourceLoaderConnectorRequest> RequestServerAdapter::start_request(ByteString const& method, URL::URL const& url, HashMap<ByteString, ByteString> const& headers, ReadonlyBytes body, Core::ProxyData const& proxy)
RefPtr<Web::ResourceLoaderConnectorRequest> RequestServerAdapter::start_request(ByteString const& method, URL::URL const& url, HTTP::HeaderMap const& headers, ReadonlyBytes body, Core::ProxyData const& proxy)
{
auto protocol_request = m_protocol_client->start_request(method, url, headers, body, proxy);
if (!protocol_request)
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibWebView/RequestServerAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class RequestServerAdapter : public Web::ResourceLoaderConnector {
virtual void prefetch_dns(URL::URL const& url) override;
virtual void preconnect(URL::URL const& url) override;

virtual RefPtr<Web::ResourceLoaderConnectorRequest> start_request(ByteString const& method, URL::URL const&, HashMap<ByteString, ByteString> const& request_headers = {}, ReadonlyBytes request_body = {}, Core::ProxyData const& = {}) override;
virtual RefPtr<Web::ResourceLoaderConnectorRequest> start_request(ByteString const& method, URL::URL const&, HTTP::HeaderMap const& request_headers = {}, ReadonlyBytes request_body = {}, Core::ProxyData const& = {}) override;
virtual RefPtr<Web::WebSockets::WebSocketClientSocket> websocket_connect(const URL::URL&, ByteString const& origin, Vector<ByteString> const& protocols) override;

private:
Expand Down
2 changes: 1 addition & 1 deletion Userland/Services/RequestServer/ConnectionFromClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ Messages::RequestServer::IsSupportedProtocolResponse ConnectionFromClient::is_su
return supported;
}

void ConnectionFromClient::start_request(i32 request_id, ByteString const& method, URL::URL const& url, HashMap<ByteString, ByteString> const& request_headers, ByteBuffer const& request_body, Core::ProxyData const& proxy_data)
void ConnectionFromClient::start_request(i32 request_id, ByteString const& method, URL::URL const& url, HTTP::HeaderMap const& request_headers, ByteBuffer const& request_body, Core::ProxyData const& proxy_data)
{
if (!url.is_valid()) {
dbgln("StartRequest: Invalid URL requested: '{}'", url);
Expand Down
4 changes: 2 additions & 2 deletions Userland/Services/RequestServer/ConnectionFromClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class ConnectionFromClient final

virtual Messages::RequestServer::ConnectNewClientResponse connect_new_client() override;
virtual Messages::RequestServer::IsSupportedProtocolResponse is_supported_protocol(ByteString const&) override;
virtual void start_request(i32 request_id, ByteString const&, URL::URL const&, HashMap<ByteString, ByteString> const&, ByteBuffer const&, Core::ProxyData const&) override;
virtual void start_request(i32 request_id, ByteString const&, URL::URL const&, HTTP::HeaderMap const&, ByteBuffer const&, Core::ProxyData const&) override;
virtual Messages::RequestServer::StopRequestResponse stop_request(i32) override;
virtual Messages::RequestServer::SetCertificateResponse set_certificate(i32, ByteString const&, ByteString const&) override;
virtual void ensure_connection(URL::URL const& url, ::RequestServer::CacheLevel const& cache_level) override;
Expand All @@ -53,7 +53,7 @@ class ConnectionFromClient final
i32 request_id;
ByteString method;
URL::URL url;
HashMap<ByteString, ByteString> request_headers;
HTTP::HeaderMap request_headers;
ByteBuffer request_body;
Core::ProxyData proxy_data;
};
Expand Down
2 changes: 1 addition & 1 deletion Userland/Services/RequestServer/GeminiProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ GeminiProtocol::GeminiProtocol()
{
}

OwnPtr<Request> GeminiProtocol::start_request(i32 request_id, ConnectionFromClient& client, ByteString const&, const URL::URL& url, HashMap<ByteString, ByteString> const&, ReadonlyBytes, Core::ProxyData proxy_data)
OwnPtr<Request> GeminiProtocol::start_request(i32 request_id, ConnectionFromClient& client, ByteString const&, const URL::URL& url, HTTP::HeaderMap const&, ReadonlyBytes, Core::ProxyData proxy_data)
{
Gemini::GeminiRequest request;
request.set_url(url);
Expand Down
2 changes: 1 addition & 1 deletion Userland/Services/RequestServer/GeminiProtocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class GeminiProtocol final : public Protocol {
private:
GeminiProtocol();

virtual OwnPtr<Request> start_request(i32, ConnectionFromClient&, ByteString const& method, const URL::URL&, HashMap<ByteString, ByteString> const&, ReadonlyBytes body, Core::ProxyData proxy_data = {}) override;
virtual OwnPtr<Request> start_request(i32, ConnectionFromClient&, ByteString const& method, const URL::URL&, HTTP::HeaderMap const&, ReadonlyBytes body, Core::ProxyData proxy_data = {}) override;
};

}
2 changes: 1 addition & 1 deletion Userland/Services/RequestServer/GeminiRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ GeminiRequest::GeminiRequest(ConnectionFromClient& client, NonnullRefPtr<Gemini:
if (auto* response = m_job->response()) {
set_downloaded_size(MUST(m_job->response_length()));
if (!response->meta().is_empty()) {
HashMap<ByteString, ByteString, CaseInsensitiveStringTraits> headers;
HTTP::HeaderMap headers;
headers.set("meta", response->meta());
// Note: We're setting content-type to meta only on status==SUCCESS
// we should perhaps have a better mechanism for this, since we
Expand Down
2 changes: 1 addition & 1 deletion Userland/Services/RequestServer/HttpCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void init(TSelf* self, TJob job)
}

template<typename TBadgedProtocol, typename TPipeResult>
OwnPtr<Request> start_request(TBadgedProtocol&& protocol, i32 request_id, ConnectionFromClient& client, ByteString const& method, const URL::URL& url, HashMap<ByteString, ByteString> const& headers, ReadonlyBytes body, TPipeResult&& pipe_result, Core::ProxyData proxy_data = {})
OwnPtr<Request> start_request(TBadgedProtocol&& protocol, i32 request_id, ConnectionFromClient& client, ByteString const& method, URL::URL const& url, HTTP::HeaderMap const& headers, ReadonlyBytes body, TPipeResult&& pipe_result, Core::ProxyData proxy_data = {})
{
using TJob = typename TBadgedProtocol::Type::JobType;
using TRequest = typename TBadgedProtocol::Type::RequestType;
Expand Down
Loading

0 comments on commit 8994dcb

Please sign in to comment.