LookupServer: Minor overhaul

- Break out request building into a DNSRequest class.
- Break out response parsing into a DNSResponse class.

A DNSRequest contains one or more DNSQuestion objects.
A DNSResponse contains all the DNSQuestions asked, and a DNSAnswer
object for each answer.

Servers/LookupServer/DNSAnswer.h

@@ -0,0 +1,29 @@
+#pragma once
+
+#include <AK/String.h>
+#include <AK/Types.h>
+
+class DNSAnswer {
+public:
+    DNSAnswer(const String& name, u16 type, u16 class_code, u32 ttl, const String& record_data)
+        : m_name(name)
+        , m_type(type)
+        , m_class_code(class_code)
+        , m_ttl(ttl)
+        , m_record_data(record_data)
+    {
+    }
+
+    const String& name() const { return m_name; }
+    u16 type() const { return m_type; }
+    u16 class_code() const { return m_class_code; }
+    u32 ttl() const { return m_ttl; }
+    const String& record_data() const { return m_record_data; }
+
+private:
+    String m_name;
+    u16 m_type { 0 };
+    u16 m_class_code { 0 };
+    u32 m_ttl { 0 };
+    String m_record_data;
+};

Servers/LookupServer/DNSQuestion.h

@@ -0,0 +1,23 @@
+#pragma once
+
+#include <AK/String.h>
+#include <AK/Types.h>
+
+class DNSQuestion {
+public:
+    DNSQuestion(const String& name, u16 record_type, u16 class_code)
+        : m_name(name)
+        , m_record_type(record_type)
+        , m_class_code(class_code)
+    {
+    }
+
+    u16 record_type() const { return m_record_type; }
+    u16 class_code() const { return m_class_code; }
+    const String& name() const { return m_name; }
+
+private:
+    String m_name;
+    u16 m_record_type { 0 };
+    u16 m_class_code { 0 };
+};

Servers/LookupServer/DNSRecord.h

@@ -1,54 +0,0 @@
-/*
- * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright notice, this
- *    list of conditions and the following disclaimer.
- *
- * 2. Redistributions in binary form must reproduce the above copyright notice,
- *    this list of conditions and the following disclaimer in the documentation
- *    and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
- * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
- * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#pragma once
-
-#include <AK/NetworkOrdered.h>
-#include <AK/Types.h>
-
-class [[gnu::packed]] DNSRecord
-{
-public:
-    DNSRecord() {}
-
-    u16 name() const { return m_name; }
-    u16 type() const { return m_type; }
-    u16 record_class() const { return m_class; }
-    u32 ttl() const { return m_ttl; }
-    u16 data_length() const { return m_data_length; }
-
-    void* data() { return this + 1; }
-    const void* data() const { return this + 1; }
-
-private:
-    NetworkOrdered<u16> m_name;
-    NetworkOrdered<u16> m_type;
-    NetworkOrdered<u16> m_class;
-    NetworkOrdered<u32> m_ttl;
-    NetworkOrdered<u16> m_data_length;
-};
-
-static_assert(sizeof(DNSRecord) == 12);

Servers/LookupServer/DNSRequest.cpp

@@ -0,0 +1,49 @@
+#include "DNSRequest.h"
+#include "DNSPacket.h"
+#include <AK/BufferStream.h>
+#include <arpa/inet.h>
+
+#define C_IN 1
+
+static u16 s_next_id = 0;
+
+DNSRequest::DNSRequest()
+    : m_id(s_next_id++)
+{
+}
+
+void DNSRequest::add_question(const String& name, u16 record_type)
+{
+    ASSERT(m_questions.size() <= UINT16_MAX);
+    m_questions.empend(name, record_type, C_IN);
+}
+
+ByteBuffer DNSRequest::to_byte_buffer() const
+{
+    DNSPacket request_header;
+    request_header.set_id(m_id);
+    request_header.set_is_query();
+    request_header.set_opcode(0);
+    request_header.set_truncated(false);
+    request_header.set_recursion_desired(true);
+    request_header.set_question_count(m_questions.size());
+
+    auto buffer = ByteBuffer::create_uninitialized(m_questions.size() * 4096);
+    BufferStream stream(buffer);
+
+    stream << ByteBuffer::wrap(&request_header, sizeof(request_header));
+
+    for (auto& question : m_questions) {
+        auto parts = question.name().split('.');
+        for (auto& part : parts) {
+            stream << (u8)part.length();
+            stream << part;
+        }
+        stream << '\0';
+        stream << htons(question.record_type());
+        stream << htons(question.class_code());
+    }
+    stream.snip();
+
+    return buffer;
+}

Servers/LookupServer/DNSRequest.h

@@ -0,0 +1,33 @@
+#pragma once
+
+#include "DNSQuestion.h"
+#include <AK/ByteBuffer.h>
+#include <AK/String.h>
+#include <AK/Types.h>
+
+#define T_A 1
+#define T_NS 2
+#define T_CNAME 5
+#define T_SOA 6
+#define T_PTR 12
+#define T_MX 15
+
+class DNSRequest {
+public:
+    DNSRequest();
+
+    void add_question(const String& name, u16 record_type);
+
+    u16 question_count() const
+    {
+        ASSERT(m_questions.size() < UINT16_MAX);
+        return m_questions.size();
+    }
+
+    u16 id() const { return m_id; }
+    ByteBuffer to_byte_buffer() const;
+
+private:
+    u16 m_id { 0 };
+    Vector<DNSQuestion> m_questions;
+};

Servers/LookupServer/DNSResponse.cpp

@@ -0,0 +1,118 @@
+#include "DNSResponse.h"
+#include "DNSPacket.h"
+#include "DNSRequest.h"
+#include <AK/IPv4Address.h>
+#include <AK/StringBuilder.h>
+
+static String parse_dns_name(const u8* data, size_t& offset, size_t max_offset, size_t recursion_level = 0);
+
+class [[gnu::packed]] DNSRecordWithoutName
+{
+public:
+    DNSRecordWithoutName() {}
+
+    u16 type() const { return m_type; }
+    u16 record_class() const { return m_class; }
+    u32 ttl() const { return m_ttl; }
+    u16 data_length() const { return m_data_length; }
+
+    void* data() { return this + 1; }
+    const void* data() const { return this + 1; }
+
+private:
+    NetworkOrdered<u16> m_type;
+    NetworkOrdered<u16> m_class;
+    NetworkOrdered<u32> m_ttl;
+    NetworkOrdered<u16> m_data_length;
+};
+
+static_assert(sizeof(DNSRecordWithoutName) == 10);
+
+
+Optional<DNSResponse> DNSResponse::from_raw_response(const u8* raw_data, size_t raw_size)
+{
+    if (raw_size < sizeof(DNSPacket)) {
+        dbg() << "DNS response not large enough (" << raw_size << " out of " << sizeof(DNSPacket) << ") to be a DNS packet.";
+        return {};
+    }
+
+    auto& response_header = *(const DNSPacket*)(raw_data);
+    dbgprintf("Got response (ID: %u)\n", response_header.id());
+    dbgprintf("  Question count: %u\n", response_header.question_count());
+    dbgprintf("  Answer count: %u\n", response_header.answer_count());
+    dbgprintf(" Authority count: %u\n", response_header.authority_count());
+    dbgprintf("Additional count: %u\n", response_header.additional_count());
+
+    DNSResponse response;
+    response.m_id = response_header.id();
+
+    size_t offset = sizeof(DNSPacket);
+
+    for (u16 i = 0; i < response_header.question_count(); ++i) {
+        auto name = parse_dns_name(raw_data, offset, raw_size);
+        struct RawDNSAnswerQuestion {
+            NetworkOrdered<u16> record_type;
+            NetworkOrdered<u16> class_code;
+        };
+        auto& record_and_class = *(const RawDNSAnswerQuestion*)&raw_data[offset];
+        response.m_questions.empend(name, record_and_class.record_type, record_and_class.class_code);
+        auto& question = response.m_questions.last();
+        offset += 4;
+        dbg() << "Question #" << i << ": _" << question.name() << "_ type: " << question.record_type() << ", class: " << question.class_code();
+    }
+
+    for (u16 i = 0; i < response_header.answer_count(); ++i) {
+        auto name = parse_dns_name(raw_data, offset, raw_size);
+
+        auto& record = *(const DNSRecordWithoutName*)(&raw_data[offset]);
+
+        String data;
+
+        offset += sizeof(DNSRecordWithoutName);
+        if (record.type() == T_PTR) {
+            size_t dummy_offset = offset;
+            data = parse_dns_name(raw_data, dummy_offset, raw_size);
+        } else if (record.type() == T_A) {
+            auto ipv4_address = IPv4Address((const u8*)record.data());
+            data = ipv4_address.to_string();
+        } else {
+            // FIXME: Parse some other record types perhaps?
+            dbg() << "  data=(unimplemented record type " << record.type() << ")";
+        }
+        dbg() << "Answer   #" << i << ": type=" << record.type() << ", ttl=" << record.ttl() << ", length=" << record.data_length() << ", data=_" << data << "_";
+        response.m_answers.empend(name, record.type(), record.record_class(), record.ttl(), data);
+        offset += record.data_length();
+    }
+
+    return response;
+}
+
+String parse_dns_name(const u8* data, size_t& offset, size_t max_offset, size_t recursion_level)
+{
+    if (recursion_level > 4)
+        return {};
+    Vector<char, 128> buf;
+    while (offset < max_offset) {
+        u8 ch = data[offset];
+        if (ch == '\0') {
+            ++offset;
+            break;
+        }
+        if ((ch & 0xc0) == 0xc0) {
+            if ((offset + 1) >= max_offset)
+                return {};
+            size_t dummy = (ch & 0x3f) << 8 | data[offset + 1];
+            offset += 2;
+            StringBuilder builder;
+            builder.append(buf.data(), buf.size());
+            auto okay = parse_dns_name(data, dummy, max_offset, recursion_level + 1);
+            builder.append(okay);
+            return builder.to_string();
+        }
+        for (size_t i = 0; i < ch; ++i)
+            buf.append(data[offset + i + 1]);
+        buf.append('.');
+        offset += ch + 1;
+    }
+    return String::copy(buf);
+}

Servers/LookupServer/DNSResponse.h

@@ -0,0 +1,35 @@
+#pragma once
+
+#include "DNSAnswer.h"
+#include "DNSQuestion.h"
+#include <AK/Types.h>
+#include <AK/Vector.h>
+#include <AK/Optional.h>
+
+class DNSResponse {
+public:
+    static Optional<DNSResponse> from_raw_response(const u8*, size_t);
+
+    u16 id() const { return m_id; }
+    const Vector<DNSQuestion>& questions() const { return m_questions; }
+    const Vector<DNSAnswer>& answers() const { return m_answers; }
+
+    u16 question_count() const
+    {
+        ASSERT(m_questions.size() <= UINT16_MAX);
+        return m_questions.size();
+    }
+
+    u16 answer_count() const
+    {
+        ASSERT(m_answers.size() <= UINT16_MAX);
+        return m_answers.size();
+    }
+
+private:
+    DNSResponse() {}
+
+    u16 m_id { 0 };
+    Vector<DNSQuestion> m_questions;
+    Vector<DNSAnswer> m_answers;
+};

Servers/LookupServer/LookupServer.cpp

@@ -25,10 +25,8 @@
  */
 
 #include "LookupServer.h"
-#include "DNSPacket.h"
-#include "DNSRecord.h"
-#include <AK/BufferStream.h>
-#include <AK/ByteBuffer.h>
+#include "DNSRequest.h"
+#include "DNSResponse.h"
 #include <AK/HashMap.h>
 #include <AK/String.h>
 #include <AK/StringBuilder.h>
@@ -38,7 +36,6 @@
 #include <LibCore/CFile.h>
 #include <LibCore/CLocalServer.h>
 #include <LibCore/CLocalSocket.h>
-#include <LibCore/CSyscallUtils.h>
 #include <arpa/inet.h>
 #include <netinet/in.h>
 #include <stdio.h>
@@ -49,15 +46,6 @@
 #include <time.h>
 #include <unistd.h>
 
-#define T_A 1
-#define T_NS 2
-#define T_CNAME 5
-#define T_SOA 6
-#define T_PTR 12
-#define T_MX 15
-
-#define C_IN 1
-
 LookupServer::LookupServer()
 {
     auto config = CConfigFile::get_for_system("LookupServer");
@@ -173,29 +161,6 @@ void LookupServer::service_client(RefPtr<CLocalSocket> socket)
     }
 }
 
-String LookupServer::parse_dns_name(const u8* data, int& offset, int max_offset)
-{
-    Vector<char, 128> buf;
-    while (offset < max_offset) {
-        u8 ch = data[offset];
-        if (ch == '\0') {
-            ++offset;
-            break;
-        }
-        if ((ch & 0xc0) == 0xc0) {
-            ASSERT_NOT_REACHED();
-            // FIXME: Parse referential names.
-            offset += 2;
-        }
-        for (int i = 0; i < ch; ++i) {
-            buf.append(data[offset + i + 1]);
-        }
-        buf.append('.');
-        offset += ch + 1;
-    }
-    return String::copy(buf);
-}
-
 Vector<String> LookupServer::lookup(const String& hostname, bool& did_timeout, unsigned short record_type)
 {
     if (auto it = m_lookup_cache.find(hostname); it != m_lookup_cache.end()) {
@@ -206,52 +171,16 @@ Vector<String> LookupServer::lookup(const String& hostname, bool& did_timeout, u
         m_lookup_cache.remove(it);
     }
 
-    DNSPacket request_header;
-    request_header.set_id(get_next_id());
-    request_header.set_is_query();
-    request_header.set_opcode(0);
-    request_header.set_truncated(false);
-    request_header.set_recursion_desired(true);
-    request_header.set_question_count(1);
+    DNSRequest request;
+    request.add_question(hostname, record_type);
 
-    auto buffer = ByteBuffer::create_uninitialized(1024);
-    {
-        BufferStream stream(buffer);
-
-        stream << ByteBuffer::wrap(&request_header, sizeof(request_header));
-        auto parts = hostname.split('.');
-        for (auto& part : parts) {
-            stream << (u8)part.length();
-            stream << part;
-        }
-        stream << '\0';
-        stream << htons(record_type);
-        stream << htons(C_IN);
-        stream.snip();
-    }
-
-    int fd = socket(AF_INET, SOCK_DGRAM, 0);
-    if (fd < 0) {
-        perror("socket");
-        return {};
-    }
-
-    struct timeval timeout {
-        1, 0
-    };
-    int rc = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout));
-    if (rc < 0) {
-        perror("setsockopt");
-        close(fd);
-        return {};
-    }
+    auto buffer = request.to_byte_buffer();
 
     struct sockaddr_in dst_addr;
-    memset(&dst_addr, 0, sizeof(dst_addr));
 
-    dst_addr.sin_family = AF_INET;
-    dst_addr.sin_port = htons(53);
-    rc = inet_pton(AF_INET, m_dns_ip.characters(), &dst_addr.sin_addr);
+    int fd = make_dns_request_socket(dst_addr);
+    if (fd < 0)
+        return {};
 
     int nsent = sendto(fd, buffer.data(), buffer.size(), 0, (const struct sockaddr*)&dst_addr, sizeof(dst_addr));
     if (nsent < 0) {
@@ -277,63 +206,62 @@ Vector<String> LookupServer::lookup(const String& hostname, bool& did_timeout, u
 
     response_buffer[nrecv] = '\0';
 
-    if (nrecv < (int)sizeof(DNSPacket)) {
-        dbgprintf("LookupServer: Response not big enough (%d) to be a DNS packet :(\n", nrecv);
+    auto o_response = DNSResponse::from_raw_response(response_buffer, nrecv);
+    if (!o_response.has_value())
         return {};
-    }
 
-    auto& response_header = *(DNSPacket*)(response_buffer);
-    dbgprintf("Got response (ID: %u)\n", response_header.id());
-    dbgprintf("  Question count: %u\n", response_header.question_count());
-    dbgprintf("  Answer count: %u\n", response_header.answer_count());
-    dbgprintf(" Authority count: %u\n", response_header.authority_count());
-    dbgprintf("Additional count: %u\n", response_header.additional_count());
+    auto& response = o_response.value();
 
-    if (response_header.id() != request_header.id()) {
-        dbgprintf("LookupServer: ID mismatch (%u vs %u) :(\n", response_header.id(), request_header.id());
+    if (response.id() != request.id()) {
+        dbgprintf("LookupServer: ID mismatch (%u vs %u) :(\n", response.id(), request.id());
         return {};
     }
-    if (response_header.question_count() != 1) {
-        dbgprintf("LookupServer: Question count (%u vs %u) :(\n", response_header.question_count(), request_header.question_count());
+    if (response.question_count() != 1) {
+        dbgprintf("LookupServer: Question count (%u vs %u) :(\n", response.question_count(), request.question_count());
         return {};
     }
-    if (response_header.answer_count() < 1) {
-        dbgprintf("LookupServer: Not enough answers (%u) :(\n", response_header.answer_count());
+    if (response.answer_count() < 1) {
+        dbgprintf("LookupServer: Not enough answers (%u) :(\n", response.answer_count());
         return {};
     }
 
-    int offset = 0;
-    auto question = parse_dns_name((const u8*)response_header.payload(), offset, nrecv);
-    offset += 4;
-
     Vector<String> addresses;
-
-    for (u16 i = 0; i < response_header.answer_count(); ++i) {
-        auto& record = *(const DNSRecord*)(&((const u8*)response_header.payload())[offset]);
-        dbgprintf("LookupServer:     Answer #%u: (question: %s), type=%u, ttl=%u, length=%u, data=",
-            i,
-            question.characters(),
-            record.type(),
-            record.ttl(),
-            record.data_length());
-
-        offset += sizeof(DNSRecord) + record.data_length();
-        if (record.type() == T_PTR) {
-            int dummy = 0;
-            auto name = parse_dns_name((const u8*)record.data(), dummy, record.data_length());
-            dbgprintf("%s\n", name.characters());
-            addresses.append(name);
-        } else if (record.type() == T_A) {
-            auto ipv4_address = IPv4Address((const u8*)record.data());
-            dbgprintf("%s\n", ipv4_address.to_string().characters());
-            addresses.append(ipv4_address.to_string());
-        } else {
-            dbgprintf("(unimplemented)\n");
-            dbgprintf("LookupServer: FIXME: Handle record type %u\n", record.type());
-        }
-        // FIXME: Parse some other record types perhaps?
+    for (auto& answer : response.answers()) {
+        addresses.append(answer.record_data());
     }
 
     m_lookup_cache.set(hostname, { time(nullptr), record_type, addresses });
     return addresses;
 }
+
+int LookupServer::make_dns_request_socket(sockaddr_in& dst_addr)
+{
+    int fd = socket(AF_INET, SOCK_DGRAM, 0);
+    if (fd < 0) {
+        perror("socket");
+        return {};
+    }
+
+    struct timeval timeout {
+        1, 0
+    };
+    int rc = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout));
+    if (rc < 0) {
+        perror("setsockopt(SOL_SOCKET, SO_RCVTIMEO)");
+        close(fd);
+        return {};
+    }
+
+    memset(&dst_addr, 0, sizeof(dst_addr));
+
+    dst_addr.sin_family = AF_INET;
+    dst_addr.sin_port = htons(53);
+    rc = inet_pton(AF_INET, m_dns_ip.characters(), &dst_addr.sin_addr);
+    if (rc < 0) {
+        perror("inet_pton");
+        close(fd);
+        return rc;
+    }
+
+    return fd;
+}

Servers/LookupServer/LookupServer.h

@@ -30,6 +30,7 @@
 #include <AK/RefPtr.h>
 #include <AK/String.h>
 #include <LibCore/CObject.h>
+#include <netinet/in.h>
 
 class CLocalSocket;
 class CLocalServer;
@@ -43,10 +44,9 @@ public:
 private:
     void load_etc_hosts();
     void service_client(RefPtr<CLocalSocket>);
-    static String parse_dns_name(const u8* data, int& offset, int max_offset);
     Vector<String> lookup(const String& hostname, bool& did_timeout, unsigned short record_type);
 
-    u16 get_next_id() { return ++m_next_id; }
+    int make_dns_request_socket(sockaddr_in& dst_addr);
 
     struct CachedLookup {
         time_t timestamp { 0 };
@@ -55,7 +55,6 @@ private:
     };
 
     RefPtr<CLocalServer> m_local_server;
-    u16 m_next_id { 0 };
     String m_dns_ip;
     HashMap<String, String> m_dns_custom_hostnames;
     HashMap<String, CachedLookup> m_lookup_cache;

Servers/LookupServer/Makefile

@@ -1,5 +1,7 @@
 OBJS = \
     LookupServer.o \
+    DNSRequest.o \
+    DNSResponse.o \
     main.o
 
 PROGRAM = LookupServer