net: prefer error for original name on lookups

With certain names and search domain configurations the
returned error would be one encountered while querying a
generated name instead of the original name. This caused
confusion when a manual check of the same name produced
different results.

Now prefer errors encountered for the original name.

Also makes the low-level DNS connection plumbing swappable
in tests enabling tighter control over responses without
relying on the network.

Fixes #12712
Updates #13295

Change-Id: I780d628a762006bb11899caf20b5f97b462a717f
Reviewed-on: https://go-review.googlesource.com/16953
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go
index 319011f5f6edf1364dc86fcb53f28fa712c6e5f5..15a4081835d7632a296ffcd0d280573b60b05cd4 100644 (file)
--- a/src/net/dnsclient_unix.go
+++ b/src/net/dnsclient_unix.go
@@ -24,9 +24,16 @@ import (
        "time"
 )
 
+// A dnsDialer provides dialing suitable for DNS queries.
+type dnsDialer interface {
+       dialDNS(string, string) (dnsConn, error)
+}
+
 // A dnsConn represents a DNS transport endpoint.
 type dnsConn interface {
-       Conn
+       io.Closer
+
+       SetDeadline(time.Time) error
 
        // readDNSResponse reads a DNS response message from the DNS
        // transport endpoint and returns the received DNS response
@@ -121,7 +128,7 @@ func (d *Dialer) dialDNS(network, server string) (dnsConn, error) {
 
 // exchange sends a query on the connection and hopes for a response.
 func exchange(server, name string, qtype uint16, timeout time.Duration) (*dnsMsg, error) {
-       d := Dialer{Timeout: timeout}
+       d := testHookDNSDialer(timeout)
        out := dnsMsg{
                dnsMsgHdr: dnsMsgHdr{
                        recursion_desired: true,
@@ -440,7 +447,8 @@ func goLookupIPOrder(name string, order hostLookupOrder) (addrs []IPAddr, err er
        conf := resolvConf.dnsConfig
        resolvConf.mu.RUnlock()
        type racer struct {
-               rrs []dnsRR
+               fqdn string
+               rrs  []dnsRR
                error
        }
        lane := make(chan racer, 1)
@@ -450,13 +458,16 @@ func goLookupIPOrder(name string, order hostLookupOrder) (addrs []IPAddr, err er
                for _, qtype := range qtypes {
                        go func(qtype uint16) {
                                _, rrs, err := tryOneName(conf, fqdn, qtype)
-                               lane <- racer{rrs, err}
+                               lane <- racer{fqdn, rrs, err}
                        }(qtype)
                }
                for range qtypes {
                        racer := <-lane
                        if racer.error != nil {
-                               lastErr = racer.error
+                               // Prefer error for original name.
+                               if lastErr == nil || racer.fqdn == name+"." {
+                                       lastErr = racer.error
+                               }
                                continue
                        }
                        addrs = append(addrs, addrRecordList(racer.rrs)...)

diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go
index 66ca4cf8ab0b1aff99844959823b89e88decaa58..95c14df52e51184734c56ce8f974794ad9a6d344 100644 (file)
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -424,6 +424,57 @@ func TestGoLookupIPOrderFallbackToFile(t *testing.T) {
        defer conf.teardown()
 }
 
+// Issue 12712.
+// When using search domains, return the error encountered
+// querying the original name instead of an error encountered
+// querying a generated name.
+func TestErrorForOriginalNameWhenSearching(t *testing.T) {
+       const fqdn = "doesnotexist.domain"
+
+       origTestHookDNSDialer := testHookDNSDialer
+       defer func() { testHookDNSDialer = origTestHookDNSDialer }()
+
+       conf, err := newResolvConfTest()
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer conf.teardown()
+
+       if err := conf.writeAndUpdate([]string{"search servfail"}); err != nil {
+               t.Fatal(err)
+       }
+
+       d := &fakeDNSConn{}
+       testHookDNSDialer = func(time.Duration) dnsDialer { return d }
+
+       d.rh = func(q *dnsMsg) (*dnsMsg, error) {
+               r := &dnsMsg{
+                       dnsMsgHdr: dnsMsgHdr{
+                               id: q.id,
+                       },
+               }
+
+               switch q.question[0].Name {
+               case fqdn + ".servfail.":
+                       r.rcode = dnsRcodeServerFailure
+               default:
+                       r.rcode = dnsRcodeNameError
+               }
+
+               return r, nil
+       }
+
+       _, err = goLookupIP(fqdn)
+       if err == nil {
+               t.Fatal("expected an error")
+       }
+
+       want := &DNSError{Name: fqdn, Err: errNoSuchHost.Error()}
+       if err, ok := err.(*DNSError); !ok || err.Name != want.Name || err.Err != want.Err {
+               t.Errorf("got %v; want %v", err, want)
+       }
+}
+
 func BenchmarkGoLookupIP(b *testing.B) {
        testHookUninstaller.Do(uninstallTestHooks)
 
@@ -461,3 +512,31 @@ func BenchmarkGoLookupIPWithBrokenNameServer(b *testing.B) {
                goLookupIP("www.example.com")
        }
 }
+
+type fakeDNSConn struct {
+       // last query
+       q *dnsMsg
+       // reply handler
+       rh func(*dnsMsg) (*dnsMsg, error)
+}
+
+func (f *fakeDNSConn) dialDNS(n, s string) (dnsConn, error) {
+       return f, nil
+}
+
+func (f *fakeDNSConn) Close() error {
+       return nil
+}
+
+func (f *fakeDNSConn) SetDeadline(time.Time) error {
+       return nil
+}
+
+func (f *fakeDNSConn) writeDNSQuery(q *dnsMsg) error {
+       f.q = q
+       return nil
+}
+
+func (f *fakeDNSConn) readDNSResponse() (*dnsMsg, error) {
+       return f.rh(f.q)
+}

diff --git a/src/net/hook.go b/src/net/hook.go
index 9ab34c0e36faf6d0451e9c4869c1eca7bb2411fe..81e061f372b74a132fc89b2a1586828a6881928a 100644 (file)
--- a/src/net/hook.go
+++ b/src/net/hook.go
@@ -4,8 +4,11 @@
 
 package net
 
+import "time"
+
 var (
        testHookDialTCP      = dialTCP
+       testHookDNSDialer    = func(d time.Duration) dnsDialer { return &Dialer{Timeout: d} }
        testHookHostsPath    = "/etc/hosts"
        testHookLookupIP     = func(fn func(string) ([]IPAddr, error), host string) ([]IPAddr, error) { return fn(host) }
        testHookSetKeepAlive = func() {}