net: fail fast for DNS rcode success with no answers of requested type

DNS responses which do not contain answers of the requested type return
errNoSuchHost, the same error as rcode name error. Prior to
golang.org/cl/37879, both cases resulted in no additional name servers
being consulted for the question. That CL changed the behavior for both
cases. Issue #25336 was filed about the rcode name error case and
golang.org/cl/113815 fixed it. This CL fixes the no answers of requested
type case as well.

Fixes #27525

Change-Id: I52fadedcd195f16adf62646b76bea2ab3b15d117
Reviewed-on: https://go-review.googlesource.com/133675
Run-TryBot: Ian Gudger <igudger@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

1 files changed, 88 insertions, 55 deletions

diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go
index bb014b903a..9e4ebcc7bb 100644
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -1427,28 +1427,35 @@ func TestDNSGoroutineRace(t *testing.T) {
 	}
 }
 
+func lookupWithFake(fake fakeDNSServer, name string, typ dnsmessage.Type) error {
+	r := Resolver{PreferGo: true, Dial: fake.DialContext}
+
+	resolvConf.mu.RLock()
+	conf := resolvConf.dnsConfig
+	resolvConf.mu.RUnlock()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	_, _, err := r.tryOneName(ctx, conf, name, typ)
+	return err
+}
+
 // Issue 8434: verify that Temporary returns true on an error when rcode
 // is SERVFAIL
 func TestIssue8434(t *testing.T) {
-	msg := dnsmessage.Message{
-		Header: dnsmessage.Header{
-			RCode: dnsmessage.RCodeServerFailure,
+	err := lookupWithFake(fakeDNSServer{
+		rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
+			return dnsmessage.Message{
+				Header: dnsmessage.Header{
+					ID:       q.ID,
+					Response: true,
+					RCode:    dnsmessage.RCodeServerFailure,
+				},
+				Questions: q.Questions,
+			}, nil
 		},
-	}
-	b, err := msg.Pack()
-	if err != nil {
-		t.Fatal("Pack failed:", err)
-	}
-	var p dnsmessage.Parser
-	h, err := p.Start(b)
-	if err != nil {
-		t.Fatal("Start failed:", err)
-	}
-	if err := p.SkipAllQuestions(); err != nil {
-		t.Fatal("SkipAllQuestions failed:", err)
-	}
-
-	err = checkHeader(&p, h, "golang.org", "foo:53")
+	}, "golang.org.", dnsmessage.TypeALL)
 	if err == nil {
 		t.Fatal("expected an error")
 	}
@@ -1464,50 +1471,76 @@ func TestIssue8434(t *testing.T) {
 	}
 }
 
-// Issue 12778: verify that NXDOMAIN without RA bit errors as
-// "no such host" and not "server misbehaving"
+// TestNoSuchHost verifies that tryOneName works correctly when the domain does
+// not exist.
+//
+// Issue 12778: verify that NXDOMAIN without RA bit errors as "no such host"
+// and not "server misbehaving"
 //
 // Issue 25336: verify that NXDOMAIN errors fail fast.
-func TestIssue12778(t *testing.T) {
-	lookups := 0
-	fake := fakeDNSServer{
-		rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
-			lookups++
-			return dnsmessage.Message{
-				Header: dnsmessage.Header{
-					ID:                 q.ID,
-					Response:           true,
-					RCode:              dnsmessage.RCodeNameError,
-					RecursionAvailable: false,
-				},
-				Questions: q.Questions,
-			}, nil
+//
+// Issue 27525: verify that empty answers fail fast.
+func TestNoSuchHost(t *testing.T) {
+	tests := []struct {
+		name string
+		f    func(string, string, dnsmessage.Message, time.Time) (dnsmessage.Message, error)
+	}{
+		{
+			"NXDOMAIN",
+			func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
+				return dnsmessage.Message{
+					Header: dnsmessage.Header{
+						ID:                 q.ID,
+						Response:           true,
+						RCode:              dnsmessage.RCodeNameError,
+						RecursionAvailable: false,
+					},
+					Questions: q.Questions,
+				}, nil
+			},
+		},
+		{
+			"no answers",
+			func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
+				return dnsmessage.Message{
+					Header: dnsmessage.Header{
+						ID:                 q.ID,
+						Response:           true,
+						RCode:              dnsmessage.RCodeSuccess,
+						RecursionAvailable: false,
+						Authoritative:      true,
+					},
+					Questions: q.Questions,
+				}, nil
+			},
 		},
 	}
-	r := Resolver{PreferGo: true, Dial: fake.DialContext}
-
-	resolvConf.mu.RLock()
-	conf := resolvConf.dnsConfig
-	resolvConf.mu.RUnlock()
 
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	_, _, err := r.tryOneName(ctx, conf, ".", dnsmessage.TypeALL)
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			lookups := 0
+			err := lookupWithFake(fakeDNSServer{
+				rh: func(n, s string, q dnsmessage.Message, d time.Time) (dnsmessage.Message, error) {
+					lookups++
+					return test.f(n, s, q, d)
+				},
+			}, ".", dnsmessage.TypeALL)
 
-	if lookups != 1 {
-		t.Errorf("got %d lookups, wanted 1", lookups)
-	}
+			if lookups != 1 {
+				t.Errorf("got %d lookups, wanted 1", lookups)
+			}
 
-	if err == nil {
-		t.Fatal("expected an error")
-	}
-	de, ok := err.(*DNSError)
-	if !ok {
-		t.Fatalf("err = %#v; wanted a *net.DNSError", err)
-	}
-	if de.Err != errNoSuchHost.Error() {
-		t.Fatalf("Err = %#v; wanted %q", de.Err, errNoSuchHost.Error())
+			if err == nil {
+				t.Fatal("expected an error")
+			}
+			de, ok := err.(*DNSError)
+			if !ok {
+				t.Fatalf("err = %#v; wanted a *net.DNSError", err)
+			}
+			if de.Err != errNoSuchHost.Error() {
+				t.Fatalf("Err = %#v; wanted %q", de.Err, errNoSuchHost.Error())
+			}
+		})
 	}
 }