From 7450da55562081f32c971bdb6e571802a3c0a6bd Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Wed, 22 Oct 2025 19:26:17 -0400 Subject: [PATCH] RequestServer: Move Resolver (and related structures) to its own file In an upcoming commit to handle requests as a state machine, we will need access to Resolver from outside of ConnectionFromClient.. --- Services/RequestServer/CMakeLists.txt | 1 + .../RequestServer/ConnectionFromClient.cpp | 97 ++++++------------- Services/RequestServer/ConnectionFromClient.h | 12 +-- Services/RequestServer/Forward.h | 3 + Services/RequestServer/Resolver.cpp | 80 +++++++++++++++ Services/RequestServer/Resolver.h | 45 +++++++++ Services/RequestServer/main.cpp | 4 +- 7 files changed, 160 insertions(+), 82 deletions(-) create mode 100644 Services/RequestServer/Resolver.cpp create mode 100644 Services/RequestServer/Resolver.h diff --git a/Services/RequestServer/CMakeLists.txt b/Services/RequestServer/CMakeLists.txt index dc23582c62..5f39f76acc 100644 --- a/Services/RequestServer/CMakeLists.txt +++ b/Services/RequestServer/CMakeLists.txt @@ -8,6 +8,7 @@ set(SOURCES Cache/DiskCache.cpp Cache/Utilities.cpp ConnectionFromClient.cpp + Resolver.cpp WebSocketImplCurl.cpp ) diff --git a/Services/RequestServer/ConnectionFromClient.cpp b/Services/RequestServer/ConnectionFromClient.cpp index 7692db462b..c552ed603a 100644 --- a/Services/RequestServer/ConnectionFromClient.cpp +++ b/Services/RequestServer/ConnectionFromClient.cpp @@ -4,8 +4,6 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include "WebSocketImplCurl.h" - #include #include #include @@ -16,13 +14,13 @@ #include #include #include -#include #include #include #include #include #include -#include +#include +#include #ifdef AK_OS_WINDOWS // needed because curl.h includes winsock2.h @@ -33,59 +31,13 @@ namespace RequestServer { -ByteString g_default_certificate_path; static HashMap> s_connections; static IDAllocator s_client_ids; + static long s_connect_timeout_seconds = 90L; -static struct { - Optional server_address; - Optional server_hostname; - u16 port; - bool use_dns_over_tls = true; - bool validate_dnssec_locally = false; -} g_dns_info; Optional g_disk_cache; -static WeakPtr s_resolver {}; -static NonnullRefPtr default_resolver() -{ - if (auto resolver = s_resolver.strong_ref()) - return *resolver; - auto resolver = make_ref_counted([] -> ErrorOr { - if (!g_dns_info.server_address.has_value()) { - if (!g_dns_info.server_hostname.has_value()) - return Error::from_string_literal("No DNS server configured"); - - auto resolved = TRY(default_resolver()->dns.lookup(*g_dns_info.server_hostname)->await()); - if (!resolved->has_cached_addresses()) - return Error::from_string_literal("Failed to resolve DNS server hostname"); - auto address = resolved->cached_addresses().first().visit([](auto& addr) -> Core::SocketAddress { return { addr, g_dns_info.port }; }); - g_dns_info.server_address = address; - } - - if (g_dns_info.use_dns_over_tls) { - TLS::Options options; - - if (!g_default_certificate_path.is_empty()) - options.root_certificates_path = g_default_certificate_path; - - return DNS::Resolver::SocketResult { - MaybeOwned(TRY(TLS::TLSv12::connect(*g_dns_info.server_address, *g_dns_info.server_hostname, move(options)))), - DNS::Resolver::ConnectionMode::TCP, - }; - } - - return DNS::Resolver::SocketResult { - MaybeOwned(TRY(Core::BufferedUDPSocket::create(TRY(Core::UDPSocket::connect(*g_dns_info.server_address))))), - DNS::Resolver::ConnectionMode::UDP, - }; - }); - - s_resolver = resolver; - return resolver; -} - ByteString build_curl_resolve_list(DNS::LookupResult const& dns_result, StringView host, u16 port) { StringBuilder resolve_opt_builder; @@ -385,7 +337,7 @@ int ConnectionFromClient::on_timeout_callback(void*, long timeout_ms, void* user ConnectionFromClient::ConnectionFromClient(NonnullOwnPtr transport) : IPC::ConnectionFromClient(*this, move(transport), s_client_ids.allocate()) - , m_resolver(default_resolver()) + , m_resolver(Resolver::default_resolver()) { s_connections.set(client_id(), *this); @@ -492,7 +444,9 @@ Messages::RequestServer::IsSupportedProtocolResponse ConnectionFromClient::is_su void ConnectionFromClient::set_dns_server(ByteString host_or_address, u16 port, bool use_tls, bool validate_dnssec_locally) { - if (host_or_address == g_dns_info.server_hostname && port == g_dns_info.port && use_tls == g_dns_info.use_dns_over_tls && validate_dnssec_locally == g_dns_info.validate_dnssec_locally) + auto& dns_info = DNSInfo::the(); + + if (host_or_address == dns_info.server_hostname && port == dns_info.port && use_tls == dns_info.use_dns_over_tls && validate_dnssec_locally == dns_info.validate_dnssec_locally) return; auto result = [&] -> ErrorOr { @@ -502,27 +456,29 @@ void ConnectionFromClient::set_dns_server(ByteString host_or_address, u16 port, else if (auto v6 = IPv6Address::from_string(host_or_address); v6.has_value()) addr = { v6.value(), port }; else - TRY(default_resolver()->dns.lookup(host_or_address)->await())->cached_addresses().first().visit([&](auto& address) { addr = { address, port }; }); + TRY(m_resolver->dns.lookup(host_or_address)->await())->cached_addresses().first().visit([&](auto& address) { addr = { address, port }; }); - g_dns_info.server_address = addr; - g_dns_info.server_hostname = host_or_address; - g_dns_info.port = port; - g_dns_info.use_dns_over_tls = use_tls; - g_dns_info.validate_dnssec_locally = validate_dnssec_locally; + dns_info.server_address = addr; + dns_info.server_hostname = host_or_address; + dns_info.port = port; + dns_info.use_dns_over_tls = use_tls; + dns_info.validate_dnssec_locally = validate_dnssec_locally; return {}; }(); if (result.is_error()) dbgln("Failed to set DNS server: {}", result.error()); else - default_resolver()->dns.reset_connection(); + m_resolver->dns.reset_connection(); } void ConnectionFromClient::set_use_system_dns() { - g_dns_info.server_hostname = {}; - g_dns_info.server_address = {}; - default_resolver()->dns.reset_connection(); + auto& dns_info = DNSInfo::the(); + dns_info.server_hostname = {}; + dns_info.server_address = {}; + + m_resolver->dns.reset_connection(); } #ifdef AK_OS_WINDOWS @@ -576,8 +532,9 @@ void ConnectionFromClient::start_request(i32 request_id, ByteString method, URL: void ConnectionFromClient::issue_network_request(i32 request_id, ByteString method, URL::URL url, HTTP::HeaderMap request_headers, ByteBuffer request_body, Core::ProxyData proxy_data, Optional resume_request) { auto host = url.serialized_host().to_byte_string(); + auto const& dns_info = DNSInfo::the(); - m_resolver->dns.lookup(host, DNS::Messages::Class::IN, { DNS::Messages::ResourceType::A, DNS::Messages::ResourceType::AAAA }, { .validate_dnssec_locally = g_dns_info.validate_dnssec_locally }) + m_resolver->dns.lookup(host, DNS::Messages::Class::IN, { DNS::Messages::ResourceType::A, DNS::Messages::ResourceType::AAAA }, { .validate_dnssec_locally = dns_info.validate_dnssec_locally }) ->when_rejected([this, request_id, resume_request](auto const& error) { dbgln("StartRequest: DNS lookup failed: {}", error); // FIXME: Implement timing info for DNS lookup failure. @@ -632,8 +589,8 @@ void ConnectionFromClient::issue_network_request(i32 request_id, ByteString meth set_option(CURLOPT_PRIVATE, request.ptr()); - if (!g_default_certificate_path.is_empty()) - set_option(CURLOPT_CAINFO, g_default_certificate_path.characters()); + if (auto const& path = default_certificate_path(); !path.is_empty()) + set_option(CURLOPT_CAINFO, path.characters()); set_option(CURLOPT_ACCEPT_ENCODING, ""); // empty string lets curl define the accepted encodings set_option(CURLOPT_URL, url.to_string().to_byte_string().characters()); @@ -927,7 +884,9 @@ void ConnectionFromClient::ensure_connection(URL::URL url, ::RequestServer::Cach } if (cache_level == CacheLevel::ResolveOnly) { - [[maybe_unused]] auto promise = m_resolver->dns.lookup(url.serialized_host().to_byte_string(), DNS::Messages::Class::IN, { DNS::Messages::ResourceType::A, DNS::Messages::ResourceType::AAAA }, { .validate_dnssec_locally = g_dns_info.validate_dnssec_locally }); + auto const& dns_info = DNSInfo::the(); + + [[maybe_unused]] auto promise = m_resolver->dns.lookup(url.serialized_host().to_byte_string(), DNS::Messages::Class::IN, { DNS::Messages::ResourceType::A, DNS::Messages::ResourceType::AAAA }, { .validate_dnssec_locally = dns_info.validate_dnssec_locally }); if constexpr (REQUESTSERVER_DEBUG) { Core::ElapsedTimer timer; timer.start(); @@ -969,8 +928,8 @@ void ConnectionFromClient::websocket_connect(i64 websocket_id, URL::URL url, Byt connection_info.set_headers(move(additional_request_headers)); connection_info.set_dns_result(move(dns_result)); - if (!g_default_certificate_path.is_empty()) - connection_info.set_root_certificates_path(g_default_certificate_path); + if (auto const& path = default_certificate_path(); !path.is_empty()) + connection_info.set_root_certificates_path(path); auto impl = WebSocketImplCurl::create(m_curl_multi); auto connection = WebSocket::WebSocket::create(move(connection_info), move(impl)); diff --git a/Services/RequestServer/ConnectionFromClient.h b/Services/RequestServer/ConnectionFromClient.h index 1fdabb46c5..ff32d50170 100644 --- a/Services/RequestServer/ConnectionFromClient.h +++ b/Services/RequestServer/ConnectionFromClient.h @@ -7,24 +7,14 @@ #pragma once #include -#include #include #include +#include #include #include namespace RequestServer { -struct Resolver : public RefCounted - , Weakable { - Resolver(Function()> create_socket) - : dns(move(create_socket)) - { - } - - DNS::Resolver dns; -}; - class ConnectionFromClient final : public IPC::ConnectionFromClient { C_OBJECT(ConnectionFromClient); diff --git a/Services/RequestServer/Forward.h b/Services/RequestServer/Forward.h index e78918ece1..e8aa15884a 100644 --- a/Services/RequestServer/Forward.h +++ b/Services/RequestServer/Forward.h @@ -14,4 +14,7 @@ class CacheEntryWriter; class CacheIndex; class DiskCache; +struct DNSInfo; +struct Resolver; + } diff --git a/Services/RequestServer/Resolver.cpp b/Services/RequestServer/Resolver.cpp new file mode 100644 index 0000000000..053f1b8ce3 --- /dev/null +++ b/Services/RequestServer/Resolver.cpp @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2024, Ali Mohammad Pur + * Copyright (c) 2025, Tim Flynn + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include + +namespace RequestServer { + +static ByteString g_default_certificate_path; + +ByteString const& default_certificate_path() +{ + return g_default_certificate_path; +} + +void set_default_certificate_path(ByteString default_certificate_path) +{ + g_default_certificate_path = move(default_certificate_path); +} + +DNSInfo& DNSInfo::the() +{ + static DNSInfo g_dns_info; + return g_dns_info; +} + +NonnullRefPtr Resolver::default_resolver() +{ + static WeakPtr g_resolver {}; + + if (auto resolver = g_resolver.strong_ref()) + return *resolver; + + auto resolver = adopt_ref(*new Resolver([] -> ErrorOr { + auto& dns_info = DNSInfo::the(); + + if (!dns_info.server_address.has_value()) { + if (!dns_info.server_hostname.has_value()) + return Error::from_string_literal("No DNS server configured"); + + auto resolved = TRY(default_resolver()->dns.lookup(*dns_info.server_hostname)->await()); + if (!resolved->has_cached_addresses()) + return Error::from_string_literal("Failed to resolve DNS server hostname"); + + auto address = resolved->cached_addresses().first().visit([&](auto& addr) -> Core::SocketAddress { return { addr, dns_info.port }; }); + dns_info.server_address = address; + } + + if (dns_info.use_dns_over_tls) { + TLS::Options options; + + if (!g_default_certificate_path.is_empty()) + options.root_certificates_path = g_default_certificate_path; + + return DNS::Resolver::SocketResult { + MaybeOwned(TRY(TLS::TLSv12::connect(*dns_info.server_address, *dns_info.server_hostname, move(options)))), + DNS::Resolver::ConnectionMode::TCP, + }; + } + + return DNS::Resolver::SocketResult { + MaybeOwned(TRY(Core::BufferedUDPSocket::create(TRY(Core::UDPSocket::connect(*dns_info.server_address))))), + DNS::Resolver::ConnectionMode::UDP, + }; + })); + + g_resolver = resolver; + return resolver; +} + +Resolver::Resolver(Function()> create_socket) + : dns(move(create_socket)) +{ +} + +} diff --git a/Services/RequestServer/Resolver.h b/Services/RequestServer/Resolver.h new file mode 100644 index 0000000000..dddf55af8b --- /dev/null +++ b/Services/RequestServer/Resolver.h @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2024, Ali Mohammad Pur + * Copyright (c) 2025, Tim Flynn + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include +#include +#include + +namespace RequestServer { + +struct DNSInfo { + static DNSInfo& the(); + + Optional server_address; + Optional server_hostname; + u16 port { 0 }; + bool use_dns_over_tls { true }; + bool validate_dnssec_locally { false }; + +private: + DNSInfo() = default; +}; + +struct Resolver + : public RefCounted + , public Weakable { + static NonnullRefPtr default_resolver(); + + DNS::Resolver dns; + +private: + explicit Resolver(Function()> create_socket); +}; + +ByteString const& default_certificate_path(); +void set_default_certificate_path(ByteString); + +} diff --git a/Services/RequestServer/main.cpp b/Services/RequestServer/main.cpp index 67561184da..5ccc71b46f 100644 --- a/Services/RequestServer/main.cpp +++ b/Services/RequestServer/main.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #if defined(AK_OS_MACOS) # include @@ -23,7 +24,6 @@ namespace RequestServer { -extern ByteString g_default_certificate_path; extern Optional g_disk_cache; } @@ -49,7 +49,7 @@ ErrorOr ladybird_main(Main::Arguments arguments) // FIXME: Update RequestServer to support multiple custom root certificates. if (!certificates.is_empty()) - RequestServer::g_default_certificate_path = certificates.first(); + RequestServer::set_default_certificate_path(certificates.first()); Core::EventLoop event_loop;