diff --git a/Userland/Services/LookupServer/ClientConnection.cpp b/Userland/Services/LookupServer/ClientConnection.cpp index 71f58d66fb..273ca96873 100644 --- a/Userland/Services/LookupServer/ClientConnection.cpp +++ b/Userland/Services/LookupServer/ClientConnection.cpp @@ -30,9 +30,12 @@ void ClientConnection::die() Messages::LookupServer::LookupNameResponse ClientConnection::lookup_name(String const& name) { - auto answers = LookupServer::the().lookup(name, DNSRecordType::A); - if (answers.is_empty()) - return { 1, Vector() }; + auto maybe_answers = LookupServer::the().lookup(name, DNSRecordType::A); + if (maybe_answers.is_error()) { + dbgln("LookupServer: Failed to lookup PTR record: {}", maybe_answers.error()); + } + + auto answers = maybe_answers.release_value(); Vector addresses; for (auto& answer : answers) { addresses.append(answer.record_data()); @@ -50,7 +53,13 @@ Messages::LookupServer::LookupAddressResponse ClientConnection::lookup_address(S ip_address[2], ip_address[1], ip_address[0]); - auto answers = LookupServer::the().lookup(name, DNSRecordType::PTR); + + auto maybe_answers = LookupServer::the().lookup(name, DNSRecordType::PTR); + if (maybe_answers.is_error()) { + dbgln("LookupServer: Failed to lookup PTR record: {}", maybe_answers.error()); + } + + auto answers = maybe_answers.release_value(); if (answers.is_empty()) return { 1, String() }; return { 0, answers[0].record_data() }; diff --git a/Userland/Services/LookupServer/DNSServer.cpp b/Userland/Services/LookupServer/DNSServer.cpp index 5c664f36a8..7d3846f0d0 100644 --- a/Userland/Services/LookupServer/DNSServer.cpp +++ b/Userland/Services/LookupServer/DNSServer.cpp @@ -16,24 +16,27 @@ DNSServer::DNSServer(Object* parent) { bind(IPv4Address(), 53); on_ready_to_receive = [this]() { - handle_client(); + auto result = handle_client(); + if (result.is_error()) { + dbgln("DNSServer: Failed to handle client: {}", result.error()); + } }; } -void DNSServer::handle_client() +ErrorOr DNSServer::handle_client() { sockaddr_in client_address; auto buffer = receive(1024, client_address); auto optional_request = DNSPacket::from_raw_packet(buffer.data(), buffer.size()); if (!optional_request.has_value()) { dbgln("Got an invalid DNS packet"); - return; + return {}; } auto& request = optional_request.value(); if (!request.is_query()) { dbgln("It's not a request"); - return; + return {}; } LookupServer& lookup_server = LookupServer::the(); @@ -46,7 +49,7 @@ void DNSServer::handle_client() if (question.class_code() != DNSRecordClass::IN) continue; response.add_question(question); - auto answers = lookup_server.lookup(question.name(), question.record_type()); + auto answers = TRY(lookup_server.lookup(question.name(), question.record_type())); for (auto& answer : answers) { response.add_answer(answer); } @@ -59,8 +62,8 @@ void DNSServer::handle_client() buffer = response.to_byte_buffer(); - // FIXME: We should be handling errors here. - [[maybe_unused]] auto result = send(buffer, client_address); + TRY(send(buffer, client_address)); + return {}; } } diff --git a/Userland/Services/LookupServer/DNSServer.h b/Userland/Services/LookupServer/DNSServer.h index 1a4c940263..54e1fe9f8f 100644 --- a/Userland/Services/LookupServer/DNSServer.h +++ b/Userland/Services/LookupServer/DNSServer.h @@ -16,7 +16,7 @@ class DNSServer : public Core::UDPServer { private: explicit DNSServer(Object* parent = nullptr); - void handle_client(); + ErrorOr handle_client(); }; } diff --git a/Userland/Services/LookupServer/LookupServer.cpp b/Userland/Services/LookupServer/LookupServer.cpp index 16c12b5f15..676988f8cc 100644 --- a/Userland/Services/LookupServer/LookupServer.cpp +++ b/Userland/Services/LookupServer/LookupServer.cpp @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include #include #include @@ -131,7 +131,7 @@ static String get_hostname() return buffer; } -Vector LookupServer::lookup(const DNSName& name, DNSRecordType record_type) +ErrorOr> LookupServer::lookup(const DNSName& name, DNSRecordType record_type) { dbgln_if(LOOKUPSERVER_DEBUG, "Got request for '{}'", name.as_string()); @@ -196,7 +196,7 @@ Vector LookupServer::lookup(const DNSName& name, DNSRecordType record int retries = 3; Vector upstream_answers; do { - upstream_answers = lookup(name, nameserver, did_get_response, record_type); + upstream_answers = TRY(lookup(name, nameserver, did_get_response, record_type)); if (did_get_response) break; } while (--retries); @@ -215,13 +215,13 @@ Vector LookupServer::lookup(const DNSName& name, DNSRecordType record // Sixth, fail. if (answers.is_empty()) { dbgln("Tried all nameservers but never got a response :("); - return {}; + return Vector {}; } return answers; } -Vector LookupServer::lookup(const DNSName& name, const String& nameserver, bool& did_get_response, DNSRecordType record_type, ShouldRandomizeCase should_randomize_case) +ErrorOr> LookupServer::lookup(const DNSName& name, const String& nameserver, bool& did_get_response, DNSRecordType record_type, ShouldRandomizeCase should_randomize_case) { DNSPacket request; request.set_is_query(); @@ -233,41 +233,27 @@ Vector LookupServer::lookup(const DNSName& name, const String& namese auto buffer = request.to_byte_buffer(); - auto udp_socket = Core::UDPSocket::construct(); - udp_socket->set_blocking(true); + auto udp_socket = TRY(Core::Stream::UDPSocket::connect(nameserver, 53, Time::from_seconds(1))); + TRY(udp_socket->set_blocking(true)); - struct timeval timeout { - 1, 0 - }; - - int rc = setsockopt(udp_socket->fd(), SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout)); - if (rc < 0) { - perror("setsockopt(SOL_SOCKET, SO_RCVTIMEO)"); - return {}; - } - - if (!udp_socket->connect(nameserver, 53)) - return {}; - - if (!udp_socket->write(buffer)) - return {}; + TRY(udp_socket->write(buffer)); u8 response_buffer[4096]; - int nrecv = udp_socket->read(response_buffer, sizeof(response_buffer)); - if (nrecv == 0) - return {}; + int nrecv = TRY(udp_socket->read({ response_buffer, sizeof(response_buffer) })); + if (udp_socket->is_eof()) + return Vector {}; did_get_response = true; auto o_response = DNSPacket::from_raw_packet(response_buffer, nrecv); if (!o_response.has_value()) - return {}; + return Vector {}; auto& response = o_response.value(); if (response.id() != request.id()) { dbgln("LookupServer: ID mismatch ({} vs {}) :(", response.id(), request.id()); - return {}; + return Vector {}; } if (response.code() == DNSPacket::Code::REFUSED) { @@ -275,12 +261,12 @@ Vector LookupServer::lookup(const DNSName& name, const String& namese // Retry with 0x20 case randomization turned off. return lookup(name, nameserver, did_get_response, record_type, ShouldRandomizeCase::No); } - return {}; + return Vector {}; } if (response.question_count() != request.question_count()) { dbgln("LookupServer: Question count ({} vs {}) :(", response.question_count(), request.question_count()); - return {}; + return Vector {}; } // Verify the questions in our request and in their response match exactly, including case. @@ -294,13 +280,13 @@ Vector LookupServer::lookup(const DNSName& name, const String& namese dbgln("Request and response questions do not match"); dbgln(" Request: name=_{}_, type={}, class={}", request_question.name().as_string(), response_question.record_type(), response_question.class_code()); dbgln(" Response: name=_{}_, type={}, class={}", response_question.name().as_string(), response_question.record_type(), response_question.class_code()); - return {}; + return Vector {}; } } if (response.answer_count() < 1) { dbgln("LookupServer: No answers :("); - return {}; + return Vector {}; } Vector answers; diff --git a/Userland/Services/LookupServer/LookupServer.h b/Userland/Services/LookupServer/LookupServer.h index f94b3ad10c..829d0e5c95 100644 --- a/Userland/Services/LookupServer/LookupServer.h +++ b/Userland/Services/LookupServer/LookupServer.h @@ -24,7 +24,7 @@ class LookupServer final : public Core::Object { public: static LookupServer& the(); - Vector lookup(const DNSName& name, DNSRecordType record_type); + ErrorOr> lookup(const DNSName& name, DNSRecordType record_type); private: LookupServer(); @@ -32,7 +32,7 @@ private: void load_etc_hosts(); void put_in_cache(const DNSAnswer&); - Vector lookup(const DNSName& hostname, const String& nameserver, bool& did_get_response, DNSRecordType record_type, ShouldRandomizeCase = ShouldRandomizeCase::Yes); + ErrorOr> lookup(const DNSName& hostname, const String& nameserver, bool& did_get_response, DNSRecordType record_type, ShouldRandomizeCase = ShouldRandomizeCase::Yes); OwnPtr> m_server; RefPtr m_dns_server;