resolver: ask a referral's nameservers that come without addresses (closes #221)

Looking up a nameserver's own address followed only the addresses a
referral gave, so a nameserver whose zone is delegated without them,
such as a.ntpns.org of pool.ntp.org, never resolved. The walk to a
name's nameservers looked addresses up only when a referral gave none.
Both now ask the nameservers whose addresses the referral gives first
and, if none of them gives a usable reply, look up and ask the others;
with no addresses given, all are looked up, as before. maxLookupDepth
stops lookups three deep, so delegations that point at each other
still end. g.ntpns.org's address needs all three when anyns.pch.net
gives the referral to g.ntpns.org without addresses.

Model: opus-5-5

internal/resolver/errors.go

@@ -33,13 +33,6 @@ var (
 		"CNAME chain depth exceeded",
 	)
 
-	// ErrLookupDepthExceeded is returned when nameserver addresses
-	// were not looked up because lookups were already maxLookupDepth
-	// deep, one inside another.
-	ErrLookupDepthExceeded = errors.New(
-		"lookups of nameserver addresses go too deep",
-	)
-
 	// ErrContextCanceled wraps context cancellation for the
 	// resolver's iterative queries.
 	ErrContextCanceled = errors.New("context canceled")

internal/resolver/export_test.go

@@ -61,16 +61,14 @@ func (r *Resolver) ResolveNSIPs(
 	ctx context.Context,
 	nsNames []string,
 ) []string {
-	ips, _ := r.resolveNSIPs(ctx, nsNames, 1)
+	return r.resolveNSIPs(ctx, nsNames, 1)
-
-	return ips
 }
 
 // MaxLookupDepth exports maxLookupDepth for testing.
 const MaxLookupDepth = maxLookupDepth
 
-// QueryZone exports queryZone for testing.
+// QueryNameservers exports queryNameservers for testing.
-func (r *Resolver) QueryZone(
+func (r *Resolver) QueryNameservers(
 	ctx context.Context,
 	given []string,
 	withoutAddresses []string,
@@ -79,7 +77,7 @@ func (r *Resolver) QueryZone(
 	qtype uint16,
 	depth int,
 ) (*dns.Msg, error) {
-	return r.queryZone(
+	return r.queryNameservers(
 		ctx, given, withoutAddresses, zone, name, qtype, depth,
 	)
 }

internal/resolver/iterative.go

@@ -215,7 +215,7 @@ func (r *Resolver) followDelegation(
 			return nil, ErrContextCanceled
 		}
 
-		resp, err := r.queryZone(
+		resp, err := r.queryNameservers(
 			ctx, servers, withoutAddresses, zone, domain, dns.TypeNS, 0,
 		)
 		if err != nil {
@@ -387,15 +387,14 @@ func referralNameservers(resp *dns.Msg) ([]string, []string) {
 	return given, withoutAddresses
 }
 
-// queryZone asks the servers of zone about name as queryServers does:
+// queryNameservers asks the servers of zone about name as queryServers
-// first those at given, the addresses a referral gave, and only when
+// does: first those at given, the addresses a referral gave, and only
-// none of them gives a usable reply, the nameservers named
+// when none of them gives a usable reply, the nameservers named
 // withoutAddresses, once their addresses are looked up. depth is how
 // many lookups of a nameserver's address are under way, 0 in the walk
 // to a domain's nameservers; at maxLookupDepth, no address is looked
-// up. When the limit is why none was found, here or in a lookup this
+// up.
-// one started, the error is ErrLookupDepthExceeded.
+func (r *Resolver) queryNameservers(
-func (r *Resolver) queryZone(
 	ctx context.Context,
 	given []string,
 	withoutAddresses []string,
@@ -417,22 +416,11 @@ func (r *Resolver) queryZone(
 		}
 	}
 
-	if len(withoutAddresses) == 0 {
+	if len(withoutAddresses) == 0 || depth >= maxLookupDepth {
 		return nil, err
 	}
 
-	if depth >= maxLookupDepth {
+	lookedUp := r.resolveNSIPs(ctx, withoutAddresses, depth+1)
-		return nil, fmt.Errorf(
-			"addresses of the nameservers of %s not looked up: %w",
-			zone, ErrLookupDepthExceeded,
-		)
-	}
-
-	lookedUp, limitErr := r.resolveNSIPs(ctx, withoutAddresses, depth+1)
-	if limitErr != nil {
-		return nil, limitErr
-	}
-
 	if len(lookedUp) == 0 {
 		return nil, err
 	}
@@ -443,34 +431,22 @@ func (r *Resolver) queryZone(
 // resolveNSIPs returns the addresses of every nameserver in nsNames
 // whose name resolves, each looked up at depth (see resolveARecord).
 // The walk can then go on to the zone's other nameservers when one
-// gives no usable reply. When none resolves and the depth limit
+// gives no usable reply.
-// stopped one of the lookups, it returns that lookup's error.
 func (r *Resolver) resolveNSIPs(
 	ctx context.Context,
 	nsNames []string,
 	depth int,
-) ([]string, error) {
+) []string {
-	var (
+	var ips []string
-		ips      []string
-		limitErr error
-	)
 
 	for _, ns := range nsNames {
 		resolved, err := r.resolveARecord(ctx, ns, depth)
-
+		if err == nil {
-		switch {
-		case err == nil:
 			ips = append(ips, resolved...)
-		case errors.Is(err, ErrLookupDepthExceeded):
-			limitErr = err
 		}
 	}
 
-	if len(ips) > 0 {
+	return ips
-		return ips, nil
-	}
-
-	return nil, limitErr
 }
 
 // resolveNSIterative queries for NS records using iterative
@@ -549,7 +525,7 @@ func (r *Resolver) resolveARecord(
 			return nil, ErrContextCanceled
 		}
 
-		resp, err := r.queryZone(
+		resp, err := r.queryNameservers(
 			ctx, servers, withoutAddresses, zone, hostname, dns.TypeA,
 			depth,
 		)

internal/resolver/resolver_test.go

@@ -2,7 +2,6 @@ package resolver_test
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"log/slog"
 	"net"
@@ -178,13 +177,13 @@ func TestResolveNSIPs_ZoneDelegatedWithoutAddresses(t *testing.T) {
 	}
 }
 
-// TestQueryZone_GivenAddressesFail asks the servers of ntp.org about
+// TestQueryNameservers_GivenAddressesFail asks the servers of ntp.org
-// pool.ntp.org, as the walk to a name under ntp.org does after the org
+// about pool.ntp.org, as the walk to a name under ntp.org does after the
-// servers' referral. That referral names four nameservers and gives an
+// org servers' referral. That referral names four nameservers and gives
-// address for ns1.everett.org alone; here the given address is
+// an address for ns1.everett.org alone; here the given address is
 // 192.0.2.1, where nothing answers, so the other three must be looked
 // up and asked.
-func TestQueryZone_GivenAddressesFail(t *testing.T) {
+func TestQueryNameservers_GivenAddressesFail(t *testing.T) {
 	t.Parallel()
 
 	r := newTestResolver(t)
@@ -193,11 +192,12 @@ func TestQueryZone_GivenAddressesFail(t *testing.T) {
 
 	livednstest.Retry(
 		t,
-		"QueryZone(192.0.2.1 and three ntp.org nameservers, pool.ntp.org)",
+		"QueryNameservers(192.0.2.1 and three ntp.org nameservers, "+
+			"pool.ntp.org)",
 		func(ctx context.Context) error {
 			var err error
 
-			resp, err = r.QueryZone(
+			resp, err = r.QueryNameservers(
 				ctx, []string{"192.0.2.1"},
 				[]string{"anyns.pch.net.", "dns1.udel.edu.", "dns2.udel.edu."},
 				"ntp.org.", "pool.ntp.org.", dns.TypeNS, 0,
@@ -210,7 +210,7 @@ func TestQueryZone_GivenAddressesFail(t *testing.T) {
 	assert.NotEmpty(t, resolver.NSSetFrom(resp, "pool.ntp.org."))
 }
 
-// TestQueryZone_LookupDepth asks the servers of g.ntpns.org, a
+// TestQueryNameservers_LookupDepth asks the servers of g.ntpns.org, a
 // nameserver of pool.ntp.org, for its address, as looking that address
 // up does when anyns.pch.net, one of the servers of ntpns.org, gives the
 // referral to g.ntpns.org without addresses. Their addresses are looked
@@ -219,8 +219,8 @@ func TestQueryZone_GivenAddressesFail(t *testing.T) {
 // addresses. From depth 1, where looking up g.ntpns.org's address
 // starts, that makes three lookups and the address is found. From one
 // below maxLookupDepth, the bitnames.com lookup would be past the limit,
-// so nothing can be asked, and the error says the limit is why.
+// so nothing can be asked.
-func TestQueryZone_LookupDepth(t *testing.T) {
+func TestQueryNameservers_LookupDepth(t *testing.T) {
 	t.Parallel()
 
 	r := newTestResolver(t)
@@ -230,11 +230,11 @@ func TestQueryZone_LookupDepth(t *testing.T) {
 
 	livednstest.Retry(
 		t,
-		"QueryZone(a.ntpns.org without its address, g.ntpns.org)",
+		"QueryNameservers(a.ntpns.org without its address, g.ntpns.org)",
 		func(ctx context.Context) error {
 			var err error
 
-			resp, err = r.QueryZone(
+			resp, err = r.QueryNameservers(
 				ctx, nil, withoutAddresses, "g.ntpns.org.", "g.ntpns.org.",
 				dns.TypeA, 1,
 			)
@@ -245,28 +245,11 @@ func TestQueryZone_LookupDepth(t *testing.T) {
 
 	assert.NotEmpty(t, resp.Answer)
 
-	var limitErr error
+	_, err := r.QueryNameservers(
-
+		t.Context(), nil, withoutAddresses, "g.ntpns.org.", "g.ntpns.org.",
-	// Any other error is live DNS not answering, and is retried.
-	livednstest.Retry(
-		t,
-		"QueryZone(a.ntpns.org without its address, g.ntpns.org, "+
-			"one below the limit)",
-		func(ctx context.Context) error {
-			_, limitErr = r.QueryZone(
-				ctx, nil, withoutAddresses, "g.ntpns.org.", "g.ntpns.org.",
 		dns.TypeA, resolver.MaxLookupDepth-1,
 	)
-			if limitErr == nil ||
+	require.ErrorIs(t, err, resolver.ErrNoNameservers)
-				errors.Is(limitErr, resolver.ErrLookupDepthExceeded) {
-				return nil
-			}
-
-			return limitErr
-		},
-	)
-
-	require.ErrorIs(t, limitErr, resolver.ErrLookupDepthExceeded)
 }
 
 // ----------------------------------------------------------------
@@ -292,20 +275,6 @@ func TestQueryNameserver_BasicA(t *testing.T) {
 	)
 }
 
-// TestQueryNameserver_ZoneDelegatedWithoutAddresses asks a.ntpns.org, a
-// nameserver of pool.ntp.org, about pool.ntp.org, as the watcher does.
-// The org servers delegate ntpns.org without the addresses of its
-// nameservers, so finding a.ntpns.org's address needs a lookup inside
-// the one QueryNameserver starts.
-func TestQueryNameserver_ZoneDelegatedWithoutAddresses(t *testing.T) {
-	t.Parallel()
-
-	r := newTestResolver(t)
-	resp := liveQueryNameserver(t, r, "a.ntpns.org.", "pool.ntp.org", "A")
-
-	assert.Equal(t, resolver.StatusOK, resp.Status)
-}
-
 func TestQueryNameserver_AAAA(t *testing.T) {
 	t.Parallel()