From d4ee439d3725c031a890afb99eecbae1a9f9547b Mon Sep 17 00:00:00 2001 From: georglauterbach <44545919+georglauterbach@users.noreply.github.com> Date: Sun, 13 Oct 2024 08:21:03 +0000 Subject: [PATCH 1/2] fix: do not query CNAME if A succeeded already Signed-off-by: georglauterbach <44545919+georglauterbach@users.noreply.github.com> --- lib/private/Http/Client/DnsPinMiddleware.php | 6 +++++- tests/lib/Http/Client/DnsPinMiddlewareTest.php | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/private/Http/Client/DnsPinMiddleware.php b/lib/private/Http/Client/DnsPinMiddleware.php index aecccc6ce9772..73310091a41c9 100644 --- a/lib/private/Http/Client/DnsPinMiddleware.php +++ b/lib/private/Http/Client/DnsPinMiddleware.php @@ -74,15 +74,19 @@ private function dnsResolve(string $target, int $recursionCount) : array { $soaDnsEntry = $this->soaRecord($target); $dnsNegativeTtl = $soaDnsEntry['minimum-ttl'] ?? null; + $canHaveCnameRecord = true; $dnsTypes = [DNS_A, DNS_AAAA, DNS_CNAME]; foreach ($dnsTypes as $dnsType) { + if ($canHaveCnameRecord === false && $dnsType === DNS_CNAME) { + continue; + } + if ($this->negativeDnsCache->isNegativeCached($target, $dnsType)) { continue; } $dnsResponses = $this->dnsGetRecord($target, $dnsType); - $canHaveCnameRecord = true; if ($dnsResponses !== false && count($dnsResponses) > 0) { foreach ($dnsResponses as $dnsResponse) { if (isset($dnsResponse['ip'])) { diff --git a/tests/lib/Http/Client/DnsPinMiddlewareTest.php b/tests/lib/Http/Client/DnsPinMiddlewareTest.php index 2fa868e6963d4..1414d58bd975e 100644 --- a/tests/lib/Http/Client/DnsPinMiddlewareTest.php +++ b/tests/lib/Http/Client/DnsPinMiddlewareTest.php @@ -554,10 +554,11 @@ static function (RequestInterface $request, array $options) { ['nextcloud' => ['allow_local_address' => false]] ); - $this->assertCount(4, $dnsQueries); + $this->assertCount(3, $dnsQueries); $this->assertContains('example.com' . DNS_SOA, $dnsQueries); $this->assertContains('subsubdomain.subdomain.example.com' . DNS_A, $dnsQueries); $this->assertContains('subsubdomain.subdomain.example.com' . DNS_AAAA, $dnsQueries); - $this->assertContains('subsubdomain.subdomain.example.com' . DNS_CNAME, $dnsQueries); + // CNAME should not be queried if A or AAAA succeeded already + $this->assertNotContains('subsubdomain.subdomain.example.com' . DNS_CNAME, $dnsQueries); } } From 72a891d26003fb87d6e4dcf16193dd80860a3435 Mon Sep 17 00:00:00 2001 From: georglauterbach <44545919+georglauterbach@users.noreply.github.com> Date: Thu, 7 Nov 2024 20:30:16 +0100 Subject: [PATCH 2/2] fix: remove superflous line Signed-off-by: georglauterbach <44545919+georglauterbach@users.noreply.github.com> --- lib/private/Http/Client/DnsPinMiddleware.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/private/Http/Client/DnsPinMiddleware.php b/lib/private/Http/Client/DnsPinMiddleware.php index 73310091a41c9..5ad120898849d 100644 --- a/lib/private/Http/Client/DnsPinMiddleware.php +++ b/lib/private/Http/Client/DnsPinMiddleware.php @@ -97,7 +97,6 @@ private function dnsResolve(string $target, int $recursionCount) : array { $canHaveCnameRecord = false; } elseif (isset($dnsResponse['target']) && $canHaveCnameRecord) { $targetIps = array_merge($targetIps, $this->dnsResolve($dnsResponse['target'], $recursionCount)); - $canHaveCnameRecord = true; } } } elseif ($dnsNegativeTtl !== null) {