From 2440112d5360d2de614a26cd436d368609f46879 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sun, 25 Oct 2020 15:07:31 +0000 Subject: [PATCH] LookupServer: Replace unused 'did_timeout' with 'did_get_response' did_timeout is a bool& parameter of lookup() that used to be set when the UDP connection timed out, but this behaviour was broken in e335d73. I replaced it with a more useful 'did_get_response' parameter that is being set after the UDP socket connected, sent its request and got a response - it might still be bogus data but we know the communication was successful. --- Services/LookupServer/LookupServer.cpp | 19 ++++++++++--------- Services/LookupServer/LookupServer.h | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Services/LookupServer/LookupServer.cpp b/Services/LookupServer/LookupServer.cpp index 855a2c1dce..65e737eb4d 100644 --- a/Services/LookupServer/LookupServer.cpp +++ b/Services/LookupServer/LookupServer.cpp @@ -120,19 +120,18 @@ void LookupServer::service_client(RefPtr socket) if (auto known_host = m_etc_hosts.get(hostname); known_host.has_value()) { responses.append(known_host.value()); } else if (!hostname.is_empty()) { - bool did_timeout; + bool did_get_response = false; int retries = 3; do { - did_timeout = false; if (lookup_type == 'L') - responses = lookup(hostname, did_timeout, T_A); + responses = lookup(hostname, did_get_response, T_A); else if (lookup_type == 'R') - responses = lookup(hostname, did_timeout, T_PTR); - if (!did_timeout) + responses = lookup(hostname, did_get_response, T_PTR); + if (did_get_response) break; } while (--retries); - if (did_timeout) { - fprintf(stderr, "LookupServer: Out of retries :(\n"); + if (!did_get_response) { + fprintf(stderr, "LookupServer: Never got a response but out of retries :(\n"); return; } } @@ -153,7 +152,7 @@ void LookupServer::service_client(RefPtr socket) } } -Vector LookupServer::lookup(const String& hostname, bool& did_timeout, unsigned short record_type, ShouldRandomizeCase should_randomize_case) +Vector LookupServer::lookup(const String& hostname, bool& did_get_response, unsigned short record_type, ShouldRandomizeCase should_randomize_case) { if (auto it = m_lookup_cache.find(hostname); it != m_lookup_cache.end()) { auto& cached_lookup = it->value; @@ -199,6 +198,8 @@ Vector LookupServer::lookup(const String& hostname, bool& did_timeout, u if (nrecv == 0) return {}; + did_get_response = true; + auto o_response = DNSResponse::from_raw_response(response_buffer, nrecv); if (!o_response.has_value()) return {}; @@ -213,7 +214,7 @@ Vector LookupServer::lookup(const String& hostname, bool& did_timeout, u if (response.code() == DNSResponse::Code::REFUSED) { if (should_randomize_case == ShouldRandomizeCase::Yes) { // Retry with 0x20 case randomization turned off. - return lookup(hostname, did_timeout, record_type, ShouldRandomizeCase::No); + return lookup(hostname, did_get_response, record_type, ShouldRandomizeCase::No); } return {}; } diff --git a/Services/LookupServer/LookupServer.h b/Services/LookupServer/LookupServer.h index 4666e0904b..0cf346f6f1 100644 --- a/Services/LookupServer/LookupServer.h +++ b/Services/LookupServer/LookupServer.h @@ -42,7 +42,7 @@ public: private: void load_etc_hosts(); void service_client(RefPtr); - Vector lookup(const String& hostname, bool& did_timeout, unsigned short record_type, ShouldRandomizeCase = ShouldRandomizeCase::Yes); + Vector lookup(const String& hostname, bool& did_get_response, unsigned short record_type, ShouldRandomizeCase = ShouldRandomizeCase::Yes); struct CachedLookup { DNSQuestion question;