LookupServer: Convert to Core::Stream::UDPSocket

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);
+    auto maybe_answers = LookupServer::the().lookup(name, DNSRecordType::A);
-    if (answers.is_empty())
+    if (maybe_answers.is_error()) {
-        return { 1, Vector<String>() };
+        dbgln("LookupServer: Failed to lookup PTR record: {}", maybe_answers.error());
+    }
+
+    auto answers = maybe_answers.release_value();
     Vector<String> 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() };

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<void> 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.
+    TRY(send(buffer, client_address));
-    [[maybe_unused]] auto result = send(buffer, client_address);
+    return {};
 }
 
 }

Userland/Services/LookupServer/DNSServer.h

@@ -16,7 +16,7 @@ class DNSServer : public Core::UDPServer {
 private:
     explicit DNSServer(Object* parent = nullptr);
 
-    void handle_client();
+    ErrorOr<void> handle_client();
 };
 
 }

Userland/Services/LookupServer/LookupServer.cpp

@@ -15,7 +15,7 @@
 #include <LibCore/ConfigFile.h>
 #include <LibCore/File.h>
 #include <LibCore/LocalServer.h>
-#include <LibCore/UDPSocket.h>
+#include <LibCore/Stream.h>
 #include <stdio.h>
 #include <time.h>
 #include <unistd.h>
@@ -131,7 +131,7 @@ static String get_hostname()
     return buffer;
 }
 
-Vector<DNSAnswer> LookupServer::lookup(const DNSName& name, DNSRecordType record_type)
+ErrorOr<Vector<DNSAnswer>> LookupServer::lookup(const DNSName& name, DNSRecordType record_type)
 {
     dbgln_if(LOOKUPSERVER_DEBUG, "Got request for '{}'", name.as_string());
 
@@ -196,7 +196,7 @@ Vector<DNSAnswer> LookupServer::lookup(const DNSName& name, DNSRecordType record
         int retries = 3;
         Vector<DNSAnswer> 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<DNSAnswer> 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<DNSAnswer> {};
     }
 
     return answers;
 }
 
-Vector<DNSAnswer> LookupServer::lookup(const DNSName& name, const String& nameserver, bool& did_get_response, DNSRecordType record_type, ShouldRandomizeCase should_randomize_case)
+ErrorOr<Vector<DNSAnswer>> 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<DNSAnswer> LookupServer::lookup(const DNSName& name, const String& namese
 
     auto buffer = request.to_byte_buffer();
 
-    auto udp_socket = Core::UDPSocket::construct();
+    auto udp_socket = TRY(Core::Stream::UDPSocket::connect(nameserver, 53, Time::from_seconds(1)));
-    udp_socket->set_blocking(true);
+    TRY(udp_socket->set_blocking(true));
 
-    struct timeval timeout {
+    TRY(udp_socket->write(buffer));
-        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 {};
 
     u8 response_buffer[4096];
-    int nrecv = udp_socket->read(response_buffer, sizeof(response_buffer));
+    int nrecv = TRY(udp_socket->read({ response_buffer, sizeof(response_buffer) }));
-    if (nrecv == 0)
+    if (udp_socket->is_eof())
-        return {};
+        return Vector<DNSAnswer> {};
 
     did_get_response = true;
 
     auto o_response = DNSPacket::from_raw_packet(response_buffer, nrecv);
     if (!o_response.has_value())
-        return {};
+        return Vector<DNSAnswer> {};
 
     auto& response = o_response.value();
 
     if (response.id() != request.id()) {
         dbgln("LookupServer: ID mismatch ({} vs {}) :(", response.id(), request.id());
-        return {};
+        return Vector<DNSAnswer> {};
     }
 
     if (response.code() == DNSPacket::Code::REFUSED) {
@@ -275,12 +261,12 @@ Vector<DNSAnswer> 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<DNSAnswer> {};
     }
 
     if (response.question_count() != request.question_count()) {
         dbgln("LookupServer: Question count ({} vs {}) :(", response.question_count(), request.question_count());
-        return {};
+        return Vector<DNSAnswer> {};
     }
 
     // Verify the questions in our request and in their response match exactly, including case.
@@ -294,13 +280,13 @@ Vector<DNSAnswer> 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<DNSAnswer> {};
         }
     }
 
     if (response.answer_count() < 1) {
         dbgln("LookupServer: No answers :(");
-        return {};
+        return Vector<DNSAnswer> {};
     }
 
     Vector<DNSAnswer, 8> answers;

Userland/Services/LookupServer/LookupServer.h

@@ -24,7 +24,7 @@ class LookupServer final : public Core::Object {
 
 public:
     static LookupServer& the();
-    Vector<DNSAnswer> lookup(const DNSName& name, DNSRecordType record_type);
+    ErrorOr<Vector<DNSAnswer>> 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<DNSAnswer> lookup(const DNSName& hostname, const String& nameserver, bool& did_get_response, DNSRecordType record_type, ShouldRandomizeCase = ShouldRandomizeCase::Yes);
+    ErrorOr<Vector<DNSAnswer>> lookup(const DNSName& hostname, const String& nameserver, bool& did_get_response, DNSRecordType record_type, ShouldRandomizeCase = ShouldRandomizeCase::Yes);
 
     OwnPtr<IPC::MultiServer<ClientConnection>> m_server;
     RefPtr<DNSServer> m_dns_server;