Provide fallback

examples/custom-config.php

@@ -1,12 +1,13 @@
 <?php
 
-require __DIR__ . "/_bootstrap.php";
+require __DIR__."/_bootstrap.php";
 
 use Amp\Dns;
 use Amp\DoH;
 use Amp\Loop;
 use Amp\Promise;
 
+// Used only by the subresolver for resolving the DoH nameserver URL
 $customConfigLoader = new class implements Dns\ConfigLoader {
     public function loadConfig(): Promise
     {
@@ -22,7 +23,16 @@ $customConfigLoader = new class implements Dns\ConfigLoader {
 };
 
 // Set default resolver to DNS-over-https resolver
-$DohConfig = new DoH\DoHConfig([new DoH\Nameserver('https://mozilla.cloudflare-dns.com/dns-query')], null, null, $customConfigLoader);
+$DohConfig = new DoH\DoHConfig(
+    [
+        new DoH\Nameserver('https://daniil.it/dns-query'),
+        new DoH\Nameserver('https://mozilla.nonexistant-dns.com/dns-query'),
+        new DoH\Nameserver('https://mozilla.cloudflare-dns.com/dns-query'), // Will fallback to this
+    ],
+    null,
+    null,
+    $customConfigLoader
+);
 Dns\resolver(new DoH\Rfc8484StubResolver($DohConfig));
 
 Loop::run(function () {

lib/DoHException.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Amp\DoH;
+
+/**
+ * Throw when DoH resolution fails.
+ */
+class DoHException extends \Exception
+{
+}

lib/Internal/HttpsSocket.php

@@ -2,10 +2,9 @@
 
 namespace Amp\DoH\Internal;
 
-use Amp;
 use Amp\Artax\Client;
 use Amp\Artax\Request;
-use Amp\Dns\DnsException;
+use Amp\DoH\DoHException;
 use Amp\DoH\JsonDecoderFactory;
 use Amp\DoH\Nameserver;
 use Amp\DoH\QueryEncoderFactory;
@@ -84,7 +83,7 @@ final class HttpsSocket extends Socket
         return call(function () use ($response, $id) {
             $response = yield $response;
             if ($response->getStatus() !== 200) {
-                throw new DnsException("HTTP result !== 200: ".$response->getReason());
+                throw new DoHException("HTTP result !== 200: ".$response->getStatus()." ".$response->getReason(), $response->getStatus());
             }
             $response = yield $response->getBody();
 

lib/Internal/Socket.php

@@ -9,6 +9,7 @@ use Amp\ByteStream\StreamException;
 use Amp\Deferred;
 use Amp\Dns\DnsException;
 use Amp\Dns\TimeoutException;
+use Amp\DoH\DoHException;
 use Amp\DoH\Nameserver;
 use Amp\Promise;
 use LibDNS\Messages\Message;
@@ -142,7 +143,7 @@ abstract class Socket
             return;
         }
 
-        if (!$exception instanceof DnsException) {
+        if (!$exception instanceof DnsException && !$exception instanceof DoHException) {
             $message = "Unexpected error during resolution: ".$exception->getMessage();
             $exception = new DnsException($message, 0, $exception);
         }

lib/Rfc8484StubResolver.php

@@ -259,7 +259,23 @@ final class Rfc8484StubResolver implements Resolver
                     $attemptDescription[] = $nameserver;
 
                     /** @var Message $response */
-                    $response = yield $socket->ask($question, $this->config->getTimeout());
+                    try {
+                        $response = yield $socket->ask($question, $this->config->getTimeout());
+                    } catch (DoHException $e) {
+                        // Defer call, because it might interfere with the unreference() call in Internal\Socket otherwise
+
+                        $i = ++$attempt % \count($nameservers);
+                        $nameserver = $nameservers[$i];
+                        $socket = $this->getSocket($nameserver);
+                        continue;
+                    } catch (NoRecordException $e) {
+                        // Defer call, because it might interfere with the unreference() call in Internal\Socket otherwise
+
+                        $i = ++$attempt % \count($nameservers);
+                        $nameserver = $nameservers[$i];
+                        $socket = $this->getSocket($nameserver);
+                        continue;
+                    }
                     $this->assertAcceptableResponse($response);
 
                     if ($response->isTruncated()) {

test/IntegrationTest.php

@@ -17,11 +17,13 @@ class IntegrationTest extends TestCase
      * @group internet
      * @dataProvider provideServersAndHostnames
      */
-    public function testResolve($hostname, $nameserver)
+    public function testResolve($hostname, $nameservers)
     {
-        $nameserver = new Nameserver(...$nameserver);
+        foreach ($nameservers as &$nameserver) {
-        Loop::run(function () use ($hostname, $nameserver) {
+            $nameserver = new Nameserver(...$nameserver);
-            $DohConfig = new DoH\DoHConfig([$nameserver]);
+        }
+        Loop::run(function () use ($hostname, $nameservers) {
+            $DohConfig = new DoH\DoHConfig($nameservers);
             Dns\resolver(new DoH\Rfc8484StubResolver($DohConfig));
             $result = yield Dns\resolve($hostname);
 
@@ -39,11 +41,13 @@ class IntegrationTest extends TestCase
      * @group internet
      * @dataProvider provideServersAndHostnames
      */
-    public function testWorksAfterConfigReload($hostname, $nameserver)
+    public function testWorksAfterConfigReload($hostname, $nameservers)
     {
-        $nameserver = new Nameserver(...$nameserver);
+        foreach ($nameservers as &$nameserver) {
-        Loop::run(function () use ($hostname, $nameserver) {
+            $nameserver = new Nameserver(...$nameserver);
-            $DohConfig = new DoH\DoHConfig([$nameserver]);
+        }
+        Loop::run(function () use ($hostname, $nameservers) {
+            $DohConfig = new DoH\DoHConfig($nameservers);
             Dns\resolver(new DoH\Rfc8484StubResolver($DohConfig));
 
             yield Dns\resolve($hostname);
@@ -56,11 +60,13 @@ class IntegrationTest extends TestCase
      * @group internet
      * @dataProvider provideServers
      */
-    public function testResolveIPv4only($nameserver)
+    public function testResolveIPv4only($nameservers)
     {
-        $nameserver = new Nameserver(...$nameserver);
+        foreach ($nameservers as &$nameserver) {
-        Loop::run(function () use ($nameserver) {
+            $nameserver = new Nameserver(...$nameserver);
-            $DohConfig = new DoH\DoHConfig([$nameserver]);
+        }
+        Loop::run(function () use ($nameservers) {
+            $DohConfig = new DoH\DoHConfig($nameservers);
             Dns\resolver(new DoH\Rfc8484StubResolver($DohConfig));
 
             $records = yield Dns\resolve("google.com", Record::A);
@@ -81,11 +87,13 @@ class IntegrationTest extends TestCase
      * @group internet
      * @dataProvider provideServers
      */
-    public function testResolveIPv6only($nameserver)
+    public function testResolveIPv6only($nameservers)
     {
-        $nameserver = new Nameserver(...$nameserver);
+        foreach ($nameservers as &$nameserver) {
-        Loop::run(function () use ($nameserver) {
+            $nameserver = new Nameserver(...$nameserver);
-            $DohConfig = new DoH\DoHConfig([$nameserver]);
+        }
+        Loop::run(function () use ($nameservers) {
+            $DohConfig = new DoH\DoHConfig($nameservers);
             Dns\resolver(new DoH\Rfc8484StubResolver($DohConfig));
 
             $records = yield Dns\resolve("google.com", Record::AAAA);
@@ -106,11 +114,13 @@ class IntegrationTest extends TestCase
      * @group internet
      * @dataProvider provideServers
      */
-    public function testPtrLookup($nameserver)
+    public function testPtrLookup($nameservers)
     {
-        $nameserver = new Nameserver(...$nameserver);
+        foreach ($nameservers as &$nameserver) {
-        Loop::run(function () use ($nameserver) {
+            $nameserver = new Nameserver(...$nameserver);
-            $DohConfig = new DoH\DoHConfig([$nameserver]);
+        }
+        Loop::run(function () use ($nameservers) {
+            $DohConfig = new DoH\DoHConfig($nameservers);
             Dns\resolver(new DoH\Rfc8484StubResolver($DohConfig));
 
             $result = yield Dns\query("8.8.4.4", Record::PTR);
@@ -171,12 +181,24 @@ class IntegrationTest extends TestCase
 
     public function provideServers()
     {
-        return [
+        $nameservers = [
-            [['https://mozilla.cloudflare-dns.com/dns-query']],
+            ['https://mozilla.cloudflare-dns.com/dns-query'],
-            [['https://mozilla.cloudflare-dns.com/dns-query', Nameserver::RFC8484_POST]],
+            ['https://mozilla.cloudflare-dns.com/dns-query', Nameserver::RFC8484_POST],
-            [['https://mozilla.cloudflare-dns.com/dns-query', Nameserver::RFC8484_GET]],
+            ['https://mozilla.cloudflare-dns.com/dns-query', Nameserver::RFC8484_GET],
-            [['https://mozilla.cloudflare-dns.com/dns-query', Nameserver::GOOGLE_JSON]],
+            ['https://mozilla.cloudflare-dns.com/dns-query', Nameserver::GOOGLE_JSON],
-            [['https://google.com/resolve', Nameserver::GOOGLE_JSON, ["Host" => "dns.google.com"]]],
+            ['https://google.com/resolve', Nameserver::GOOGLE_JSON, ["Host" => "dns.google.com"]],
         ];
+        $result = [];
+        for ($start = 0; $start < count($nameservers); $start++) {
+            $temp = [];
+            for ($i = 0; $i < count($nameservers); $i++) {
+                $i = ($start + $i) % count($nameservers);
+
+                $temp[] = $nameservers[$i];
+            }
+            $result[] = [$temp];
+        }
+
+        return $result;
     }
 }

test/Rfc8484StubResolverTest.php

@@ -66,4 +66,21 @@ class Rfc8484StubResolverTest extends TestCase
             new Rfc8484StubResolver($DohConfig);
         });
     }
+
+    public function testInvalidNameserverFallback()
+    {
+        Loop::run(function () {
+            $DohConfig = new DoH\DoHConfig(
+                [
+                    new DoH\Nameserver('https://google.com/wrong-uri'),
+                    new DoH\Nameserver('https://google.com/wrong-uri'),
+                    new DoH\Nameserver('https://nonexistant-dns.com/dns-query'),
+                    new DoH\Nameserver('https://mozilla.cloudflare-dns.com/dns-query'),
+                ]
+            );
+            $resolver = new Rfc8484StubResolver($DohConfig);
+            $this->assertInstanceOf(Rfc8484StubResolver::class, $resolver);
+            yield $resolver->resolve('google.com');
+        });
+    }
 }