fix(dns): floor the resolver timeout to stop indefinite hangs

opts.Timeout <= 0 reached retryabledns as a literal zero, and
retryabledns applies no default of its own, so a black-hole or
non-responsive resolver blocked the query forever. floor it to
defaultDNSTimeout before building the client, mirroring the tcp
executor's existing floor.

Author: TBX3D

internal/modules/dns.go

@@ -28,6 +28,12 @@ import (
 // timeout before giving up.
 const dnsMaxRetries = 3
 
+// defaultDNSTimeout bounds a query when the caller passes no timeout.
+// retryabledns applies no default of its own: a zero Options.Timeout reaches
+// the underlying dns.Client as a literal zero, which blocks forever against a
+// non-responsive resolver.
+const defaultDNSTimeout = 3 * time.Second
+
 // defaultDNSResolvers is the bundled pool: fast public anycast servers.
 var defaultDNSResolvers = []string{"1.1.1.1:53", "8.8.8.8:53", "9.9.9.9:53"}
 
@@ -59,19 +65,22 @@ type dnsResolver interface {
 }
 
 // newDNSResolver builds a resolver over the given pool (falling back to the
-// bundled default when it is empty) with the given timeout. It is a package var
-// so tests can supply a fake without touching the network.
+// bundled default when it is empty) with the given timeout, flooring a
+// non-positive timeout to the default so a caller can't request an
+// effectively unbounded resolve. It is a package var so tests can supply a
+// fake without touching the network.
 var newDNSResolver = func(resolvers []string, timeout time.Duration) (dnsResolver, error) {
 	pool := resolvers
 	if len(pool) == 0 {
 		pool = defaultDNSResolvers
 	}
+	if timeout <= 0 {
+		timeout = defaultDNSTimeout
+	}
 	opts := retryabledns.Options{
 		BaseResolvers: pool,
 		MaxRetries:    dnsMaxRetries,
-	}
-	if timeout > 0 {
-		opts.Timeout = timeout
+		Timeout:       timeout,
 	}
 	client, err := retryabledns.NewWithOptions(opts)
 	if err != nil {

internal/modules/dns_test.go

@@ -16,6 +16,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -422,3 +423,55 @@ func TestNewDNSResolverBuildsClient(t *testing.T) {
 	}
 	r.Close()
 }
+
+// TestNewDNSResolverFloorsZeroTimeout pins the timeout floor: opts.Timeout <= 0
+// must not reach retryabledns as a literal zero, since retryabledns applies no
+// default of its own and a zero-timeout dns.Client blocks forever on a
+// non-responsive resolver. A black-hole UDP listener (accepts the query,
+// never replies) stands in for that non-responsive resolver.
+func TestNewDNSResolverFloorsZeroTimeout(t *testing.T) {
+	pc, err := net.ListenPacket("udp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	defer pc.Close()
+	go func() {
+		buf := make([]byte, 512)
+		for {
+			if _, _, err := pc.ReadFrom(buf); err != nil {
+				return
+			}
+			// never reply: this is the black hole.
+		}
+	}()
+
+	origResolvers := defaultDNSResolvers
+	defaultDNSResolvers = []string{pc.LocalAddr().String()}
+	t.Cleanup(func() { defaultDNSResolvers = origResolvers })
+
+	r, err := newDNSResolver(nil, 0)
+	if err != nil {
+		t.Fatalf("newDNSResolver: %v", err)
+	}
+
+	done := make(chan error, 1)
+	start := time.Now()
+	go func() {
+		_, err := r.Query("example.com", dns.TypeA)
+		done <- err
+	}()
+
+	// the executor retries dnsMaxRetries times, each capped at the floored
+	// timeout, so the bound is a multiple of it, not the timeout itself.
+	bound := time.Duration(dnsMaxRetries) * defaultDNSTimeout
+	select {
+	case err := <-done:
+		elapsed := time.Since(start)
+		t.Logf("query against a black-hole resolver returned after %v: %v", elapsed, err)
+		if elapsed > bound+2*time.Second {
+			t.Errorf("query took %v, want it bounded by ~%v (floored timeout x retries)", elapsed, bound)
+		}
+	case <-time.After(bound + 5*time.Second):
+		t.Fatal("query against a black-hole resolver did not return in bounded time; a zero timeout is hanging forever")
+	}
+}