[go] net: allow Resolver to use a custom dialer
Matt Harden (Gerrit)
Matt Harden has uploaded this change for review.
net: allow Resolver to use a custom dialer In some cases it is desirable to customize the way the DNS server is contacted, for instance to use a specific LocalAddr. While most operating-system level resolvers do not allow this, we have the opportunity to do so with the Go resolver. Most of the code was already in place to allow tests to override the dialer. This exposes that functionality, and as a side effect eliminates the need for a testing hook. Fixes #17404 Change-Id: I1c5e570f8edbcf630090f8ec6feb52e379e3e5c0 --- M src/net/dnsclient_unix.go M src/net/dnsclient_unix_test.go M src/net/lookup.go M src/net/lookup_unix.go 4 files changed, 120 insertions(+), 113 deletions(-)
diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go
index 4dd4e16..40d3fcc 100644
--- a/src/net/dnsclient_unix.go
+++ b/src/net/dnsclient_unix.go
@@ -26,11 +26,7 @@
)
// A dnsDialer provides dialing suitable for DNS queries.
-type dnsDialer interface {
- dialDNS(ctx context.Context, network, addr string) (dnsConn, error)
-}
-
-var testHookDNSDialer = func() dnsDialer { return &Dialer{} }
+type dnsDialer func(ctx context.Context, network, addr string) (dnsConn, error)
// A dnsConn represents a DNS transport endpoint.
type dnsConn interface {
@@ -116,33 +112,8 @@
return resp, nil
}
-func (d *Dialer) dialDNS(ctx context.Context, network, server string) (dnsConn, error) {
- switch network {
- case "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6":
- default:
- return nil, UnknownNetworkError(network)
- }
- // Calling Dial here is scary -- we have to be sure not to
- // dial a name that will require a DNS lookup, or Dial will
- // call back here to translate it. The DNS config parser has
- // already checked that all the cfg.servers are IP
- // addresses, which Dial will use without a DNS lookup.
- c, err := d.DialContext(ctx, network, server)
- if err != nil {
- return nil, mapErr(err)
- }
- switch network {
- case "tcp", "tcp4", "tcp6":
- return c.(*TCPConn), nil
- case "udp", "udp4", "udp6":
- return c.(*UDPConn), nil
- }
- panic("unreachable")
-}
-
// exchange sends a query on the connection and hopes for a response.
-func exchange(ctx context.Context, server, name string, qtype uint16, timeout time.Duration) (*dnsMsg, error) {
- d := testHookDNSDialer()
+func exchange(ctx context.Context, dial dnsDialer, server, name string, qtype uint16, timeout time.Duration) (*dnsMsg, error) {
out := dnsMsg{
dnsMsgHdr: dnsMsgHdr{
recursion_desired: true,
@@ -158,7 +129,7 @@
ctx, cancel := context.WithDeadline(ctx, time.Now().Add(timeout))
defer cancel()
- c, err := d.dialDNS(ctx, network, server)
+ c, err := dial(ctx, network, server)
if err != nil {
return nil, err
}
@@ -181,7 +152,7 @@
// Do a lookup for a single name, which must be rooted
// (otherwise answer will not find the answers).
-func tryOneName(ctx context.Context, cfg *dnsConfig, name string, qtype uint16) (string, []dnsRR, error) {
+func tryOneName(ctx context.Context, d dnsDialer, cfg *dnsConfig, name string, qtype uint16) (string, []dnsRR, error) {
var lastErr error
serverOffset := cfg.serverOffset()
sLen := uint32(len(cfg.servers))
@@ -190,7 +161,7 @@
for j := uint32(0); j < sLen; j++ {
server := cfg.servers[(serverOffset+j)%sLen]
- msg, err := exchange(ctx, server, name, qtype, cfg.timeout)
+ msg, err := exchange(ctx, d, server, name, qtype, cfg.timeout)
if err != nil {
lastErr = &DNSError{
Err: err.Error(),
@@ -314,7 +285,7 @@
<-conf.ch
}
-func lookup(ctx context.Context, name string, qtype uint16) (cname string, rrs []dnsRR, err error) {
+func lookup(ctx context.Context, d dnsDialer, name string, qtype uint16) (cname string, rrs []dnsRR, err error) {
if !isDomainName(name) {
// We used to use "invalid domain name" as the error,
// but that is a detail of the specific lookup mechanism.
@@ -328,7 +299,7 @@
conf := resolvConf.dnsConfig
resolvConf.mu.RUnlock()
for _, fqdn := range conf.nameList(name) {
- cname, rrs, err = tryOneName(ctx, conf, fqdn, qtype)
+ cname, rrs, err = tryOneName(ctx, d, conf, fqdn, qtype)
if err == nil {
break
}
@@ -432,11 +403,11 @@
// Normally we let cgo use the C library resolver instead of
// depending on our lookup code, so that Go and C get the same
// answers.
-func goLookupHost(ctx context.Context, name string) (addrs []string, err error) {
- return goLookupHostOrder(ctx, name, hostLookupFilesDNS)
+func goLookupHost(ctx context.Context, d dnsDialer, name string) (addrs []string, err error) {
+ return goLookupHostOrder(ctx, d, name, hostLookupFilesDNS)
}
-func goLookupHostOrder(ctx context.Context, name string, order hostLookupOrder) (addrs []string, err error) {
+func goLookupHostOrder(ctx context.Context, d dnsDialer, name string, order hostLookupOrder) (addrs []string, err error) {
if order == hostLookupFilesDNS || order == hostLookupFiles {
// Use entries from /etc/hosts if they match.
addrs = lookupStaticHost(name)
@@ -444,7 +415,7 @@
return
}
}
- ips, _, err := goLookupIPCNAMEOrder(ctx, name, order)
+ ips, _, err := goLookupIPCNAMEOrder(ctx, d, name, order)
if err != nil {
return
}
@@ -470,13 +441,13 @@
// goLookupIP is the native Go implementation of LookupIP.
// The libc versions are in cgo_*.go.
-func goLookupIP(ctx context.Context, host string) (addrs []IPAddr, err error) {
+func goLookupIP(ctx context.Context, d dnsDialer, host string) (addrs []IPAddr, err error) {
order := systemConf().hostLookupOrder(host)
- addrs, _, err = goLookupIPCNAMEOrder(ctx, host, order)
+ addrs, _, err = goLookupIPCNAMEOrder(ctx, d, host, order)
return
}
-func goLookupIPCNAMEOrder(ctx context.Context, name string, order hostLookupOrder) (addrs []IPAddr, cname string, err error) {
+func goLookupIPCNAMEOrder(ctx context.Context, d dnsDialer, name string, order hostLookupOrder) (addrs []IPAddr, cname string, err error) {
if order == hostLookupFilesDNS || order == hostLookupFiles {
addrs = goLookupIPFiles(name)
if len(addrs) > 0 || order == hostLookupFiles {
@@ -502,7 +473,7 @@
for _, fqdn := range conf.nameList(name) {
for _, qtype := range qtypes {
go func(qtype uint16) {
- cname, rrs, err := tryOneName(ctx, conf, fqdn, qtype)
+ cname, rrs, err := tryOneName(ctx, d, conf, fqdn, qtype)
lane <- racer{cname, rrs, err}
}(qtype)
}
@@ -543,9 +514,9 @@
}
// goLookupCNAME is the native Go (non-cgo) implementation of LookupCNAME.
-func goLookupCNAME(ctx context.Context, host string) (cname string, err error) {
+func goLookupCNAME(ctx context.Context, d dnsDialer, host string) (cname string, err error) {
order := systemConf().hostLookupOrder(host)
- _, cname, err = goLookupIPCNAMEOrder(ctx, host, order)
+ _, cname, err = goLookupIPCNAMEOrder(ctx, d, host, order)
return
}
@@ -554,7 +525,7 @@
// only if cgoLookupPTR is the stub in cgo_stub.go).
// Normally we let cgo use the C library resolver instead of depending
// on our lookup code, so that Go and C get the same answers.
-func goLookupPTR(ctx context.Context, addr string) ([]string, error) {
+func goLookupPTR(ctx context.Context, d dnsDialer, addr string) ([]string, error) {
names := lookupStaticAddr(addr)
if len(names) > 0 {
return names, nil
@@ -563,7 +534,7 @@
if err != nil {
return nil, err
}
- _, rrs, err := lookup(ctx, arpa, dnsTypePTR)
+ _, rrs, err := lookup(ctx, d, arpa, dnsTypePTR)
if err != nil {
return nil, err
}
diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go
index 4464804..325d1c7 100644
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -40,10 +40,11 @@
func TestDNSTransportFallback(t *testing.T) {
testenv.MustHaveExternalNetwork(t)
+ r := Resolver{}
for _, tt := range dnsTransportFallbackTests {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
- msg, err := exchange(ctx, tt.server, tt.name, tt.qtype, time.Second)
+ msg, err := exchange(ctx, r.dnsDialer(), tt.server, tt.name, tt.qtype, time.Second)
if err != nil {
t.Error(err)
continue
@@ -82,10 +83,11 @@
testenv.MustHaveExternalNetwork(t)
server := "8.8.8.8:53"
+ r := Resolver{}
for _, tt := range specialDomainNameTests {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
- msg, err := exchange(ctx, server, tt.name, tt.qtype, 3*time.Second)
+ msg, err := exchange(ctx, r.dnsDialer(), server, tt.name, tt.qtype, 3*time.Second)
if err != nil {
t.Error(err)
continue
@@ -142,7 +144,8 @@
// Issue 13705: don't try to resolve onion addresses, etc
func TestLookupTorOnion(t *testing.T) {
- addrs, err := goLookupIP(context.Background(), "foo.onion")
+ r := Resolver{PreferGo: true}
+ addrs, err := r.LookupIPAddr(context.Background(), "foo.onion")
if len(addrs) > 0 {
t.Errorf("unexpected addresses: %v", addrs)
}
@@ -246,6 +249,7 @@
}
defer conf.teardown()
+ r := Resolver{PreferGo: true}
for i, tt := range updateResolvConfTests {
if err := conf.writeAndUpdate(tt.lines); err != nil {
t.Error(err)
@@ -258,7 +262,7 @@
for j := 0; j < N; j++ {
go func(name string) {
defer wg.Done()
- ips, err := goLookupIP(context.Background(), name)
+ ips, err := r.LookupIPAddr(context.Background(), name)
if err != nil {
t.Error(err)
return
@@ -401,12 +405,13 @@
}
defer conf.teardown()
+ r := &Resolver{PreferGo: true}
for _, tt := range goLookupIPWithResolverConfigTests {
if err := conf.writeAndUpdate(tt.lines); err != nil {
t.Error(err)
continue
}
- addrs, err := goLookupIP(context.Background(), tt.name)
+ addrs, err := r.LookupIPAddr(context.Background(), tt.name)
if err != nil {
// This test uses external network connectivity.
// We need to take care with errors on both
@@ -452,18 +457,19 @@
defer func(orig string) { testHookHostsPath = orig }(testHookHostsPath)
testHookHostsPath = "testdata/hosts"
+ r := Resolver{PreferGo: true}
for _, order := range []hostLookupOrder{hostLookupFilesDNS, hostLookupDNSFiles} {
name := fmt.Sprintf("order %v", order)
// First ensure that we get an error when contacting a non-existent host.
- _, _, err := goLookupIPCNAMEOrder(context.Background(), "notarealhost", order)
+ _, _, err := goLookupIPCNAMEOrder(context.Background(), r.dnsDialer(), "notarealhost", order)
if err == nil {
t.Errorf("%s: expected error while looking up name not in hosts file", name)
continue
}
// Now check that we get an address when the name appears in the hosts file.
- addrs, _, err := goLookupIPCNAMEOrder(context.Background(), "thor", order) // entry is in "testdata/hosts"
+ addrs, _, err := goLookupIPCNAMEOrder(context.Background(), r.dnsDialer(), "thor", order) // entry is in "testdata/hosts"
if err != nil {
t.Errorf("%s: expected to successfully lookup host entry", name)
continue
@@ -486,9 +492,6 @@
func TestErrorForOriginalNameWhenSearching(t *testing.T) {
const fqdn = "doesnotexist.domain"
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -499,10 +502,7 @@
t.Fatal(err)
}
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
- d.rh = func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ d := fakeDNSDialer(func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
r := &dnsMsg{
dnsMsgHdr: dnsMsgHdr{
id: q.id,
@@ -517,9 +517,10 @@
}
return r, nil
- }
+ })
+ r := &Resolver{PreferGo: true, DNSDialer: d}
- _, err = goLookupIP(context.Background(), fqdn)
+ _, err = r.LookupIPAddr(context.Background(), fqdn)
if err == nil {
t.Fatal("expected an error")
}
@@ -532,9 +533,6 @@
// Issue 15434. If a name server gives a lame referral, continue to the next.
func TestIgnoreLameReferrals(t *testing.T) {
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -546,10 +544,7 @@
t.Fatal(err)
}
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
- d.rh = func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ d := fakeDNSDialer(func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
t.Log(s, q)
r := &dnsMsg{
dnsMsgHdr: dnsMsgHdr{
@@ -577,9 +572,10 @@
}
return r, nil
- }
+ })
+ r := &Resolver{PreferGo: true, DNSDialer: d}
- addrs, err := goLookupIP(context.Background(), "www.golang.org")
+ addrs, err := r.LookupIPAddr(context.Background(), "www.golang.org")
if err != nil {
t.Fatal(err)
}
@@ -597,8 +593,9 @@
testHookUninstaller.Do(uninstallTestHooks)
ctx := context.Background()
+ r := Resolver{PreferGo: true}
for i := 0; i < b.N; i++ {
- goLookupIP(ctx, "www.example.com")
+ r.LookupIPAddr(ctx, "www.example.com")
}
}
@@ -606,8 +603,9 @@
testHookUninstaller.Do(uninstallTestHooks)
ctx := context.Background()
+ r := Resolver{PreferGo: true}
for i := 0; i < b.N; i++ {
- goLookupIP(ctx, "some.nonexistent")
+ r.LookupIPAddr(ctx, "some.nonexistent")
}
}
@@ -629,18 +627,16 @@
}
ctx := context.Background()
+ r := Resolver{PreferGo: true}
for i := 0; i < b.N; i++ {
- goLookupIP(ctx, "www.example.com")
+ r.LookupIPAddr(ctx, "www.example.com")
}
}
-type fakeDNSDialer struct {
- // reply handler
- rh func(s string, q *dnsMsg, t time.Time) (*dnsMsg, error)
-}
-
-func (f *fakeDNSDialer) dialDNS(_ context.Context, n, s string) (dnsConn, error) {
- return &fakeDNSConn{f.rh, s, time.Time{}}, nil
+func fakeDNSDialer(rh func(s string, q *dnsMsg, t time.Time) (*dnsMsg, error)) func(context.Context, string, string) (Conn, error) {
+ return func(_ context.Context, n, s string) (Conn, error) {
+ return &fakeDNSConn{rh, s, time.Time{}}, nil
+ }
}
type fakeDNSConn struct {
@@ -649,7 +645,31 @@
t time.Time
}
+func (f *fakeDNSConn) Read(_ []byte) (int, error) {
+ return 0, nil
+}
+
+func (f *fakeDNSConn) Write(_ []byte) (int, error) {
+ return 0, nil
+}
+
func (f *fakeDNSConn) Close() error {
+ return nil
+}
+
+func (f *fakeDNSConn) LocalAddr() Addr {
+ return nil
+}
+
+func (f *fakeDNSConn) RemoteAddr() Addr {
+ return nil
+}
+
+func (f *fakeDNSConn) SetReadDeadline(_ time.Time) error {
+ return nil
+}
+
+func (f *fakeDNSConn) SetWriteDeadline(_ time.Time) error {
return nil
}
@@ -736,9 +756,6 @@
// Issue 16865. If a name server times out, continue to the next.
func TestRetryTimeout(t *testing.T) {
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -753,12 +770,9 @@
t.Fatal(err)
}
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
var deadline0 time.Time
- d.rh = func(s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
+ d := fakeDNSDialer(func(s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
t.Log(s, q, deadline)
if deadline.IsZero() {
@@ -776,9 +790,10 @@
}
return mockTXTResponse(q), nil
- }
+ })
+ r := &Resolver{PreferGo: true, DNSDialer: d}
- _, err = LookupTXT("www.golang.org")
+ _, err = r.LookupTXT(context.Background(), "www.golang.org")
if err != nil {
t.Fatal(err)
}
@@ -797,9 +812,6 @@
}
func testRotate(t *testing.T, rotate bool, nameservers, wantServers []string) {
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -818,18 +830,16 @@
t.Fatal(err)
}
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
var usedServers []string
- d.rh = func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ d := fakeDNSDialer(func(s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
usedServers = append(usedServers, s)
return mockTXTResponse(q), nil
- }
+ })
+ r := Resolver{PreferGo: true, DNSDialer: d}
// len(nameservers) + 1 to allow rotation to get back to start
for i := 0; i < len(nameservers)+1; i++ {
- if _, err := LookupTXT("www.golang.org"); err != nil {
+ if _, err := r.LookupTXT(context.Background(), "www.golang.org"); err != nil {
t.Fatal(err)
}
}
diff --git a/src/net/lookup.go b/src/net/lookup.go
index cc2013e..3785853 100644
--- a/src/net/lookup.go
+++ b/src/net/lookup.go
@@ -97,6 +97,13 @@
// GODEBUG=netdns=go, but scoped to just this resolver.
PreferGo bool
+ // DNSDialer is used by Go's built-in DNS resolver to make TCP and
+ // UDP connections to DNS services. The Resolver will never call this
+ // it with names, only valid IP addresses. The Conn returned must be
+ // a *TCPConn or *UDPConn as requested by the network parameter.
+ // If nil the default dialer is used.
+ DNSDialer func(ctx context.Context, network, host string) (Conn, error)
+
// TODO(bradfitz): optional interface impl override hook
// TODO(bradfitz): Timeout time.Duration?
}
diff --git a/src/net/lookup_unix.go b/src/net/lookup_unix.go
index be2ced9..207e8d4 100644
--- a/src/net/lookup_unix.go
+++ b/src/net/lookup_unix.go
@@ -48,6 +48,25 @@
return lookupProtocolMap(name)
}
+func (r *Resolver) dnsDialer() dnsDialer {
+ dialCtx := r.DNSDialer
+ if dialCtx == nil {
+ dialCtx = (&Dialer{}).DialContext
+ }
+ return func(ctx context.Context, network, server string) (dnsConn, error) {
+ // Calling Dial here is scary -- we have to be sure not to
+ // dial a name that will require a DNS lookup, or Dial will
+ // call back here to translate it. The DNS config parser has
+ // already checked that all the cfg.servers are IP
+ // addresses, which Dial will use without a DNS lookup.
+ c, err := dialCtx(ctx, network, server)
+ if err != nil {
+ return nil, mapErr(err)
+ }
+ return c.(dnsConn), nil
+ }
+}
+
func (r *Resolver) lookupHost(ctx context.Context, host string) (addrs []string, err error) {
order := systemConf().hostLookupOrder(host)
if !r.PreferGo && order == hostLookupCgo {
@@ -57,12 +76,12 @@
// cgo not available (or netgo); fall back to Go's DNS resolver
order = hostLookupFilesDNS
}
- return goLookupHostOrder(ctx, host, order)
+ return goLookupHostOrder(ctx, r.dnsDialer(), host, order)
}
func (r *Resolver) lookupIP(ctx context.Context, host string) (addrs []IPAddr, err error) {
if r.PreferGo {
- return goLookupIP(ctx, host)
+ return goLookupIP(ctx, r.dnsDialer(), host)
}
order := systemConf().hostLookupOrder(host)
if order == hostLookupCgo {
@@ -72,7 +91,7 @@
// cgo not available (or netgo); fall back to Go's DNS resolver
order = hostLookupFilesDNS
}
- addrs, _, err = goLookupIPCNAMEOrder(ctx, host, order)
+ addrs, _, err = goLookupIPCNAMEOrder(ctx, r.dnsDialer(), host, order)
return
}
@@ -98,17 +117,17 @@
return cname, err
}
}
- return goLookupCNAME(ctx, name)
+ return goLookupCNAME(ctx, r.dnsDialer(), name)
}
-func (*Resolver) lookupSRV(ctx context.Context, service, proto, name string) (string, []*SRV, error) {
+func (r *Resolver) lookupSRV(ctx context.Context, service, proto, name string) (string, []*SRV, error) {
var target string
if service == "" && proto == "" {
target = name
} else {
target = "_" + service + "._" + proto + "." + name
}
- cname, rrs, err := lookup(ctx, target, dnsTypeSRV)
+ cname, rrs, err := lookup(ctx, r.dnsDialer(), target, dnsTypeSRV)
if err != nil {
return "", nil, err
}
@@ -121,8 +140,8 @@
return cname, srvs, nil
}
-func (*Resolver) lookupMX(ctx context.Context, name string) ([]*MX, error) {
- _, rrs, err := lookup(ctx, name, dnsTypeMX)
+func (r *Resolver) lookupMX(ctx context.Context, name string) ([]*MX, error) {
+ _, rrs, err := lookup(ctx, r.dnsDialer(), name, dnsTypeMX)
if err != nil {
return nil, err
}
@@ -135,8 +154,8 @@
return mxs, nil
}
-func (*Resolver) lookupNS(ctx context.Context, name string) ([]*NS, error) {
- _, rrs, err := lookup(ctx, name, dnsTypeNS)
+func (r *Resolver) lookupNS(ctx context.Context, name string) ([]*NS, error) {
+ _, rrs, err := lookup(ctx, r.dnsDialer(), name, dnsTypeNS)
if err != nil {
return nil, err
}
@@ -148,7 +167,7 @@
}
func (r *Resolver) lookupTXT(ctx context.Context, name string) ([]string, error) {
- _, rrs, err := lookup(ctx, name, dnsTypeTXT)
+ _, rrs, err := lookup(ctx, r.dnsDialer(), name, dnsTypeTXT)
if err != nil {
return nil, err
}
@@ -165,5 +184,5 @@
return ptrs, err
}
}
- return goLookupPTR(ctx, addr)
+ return goLookupPTR(ctx, r.dnsDialer(), addr)
}
To view, visit change 37260. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 1:
Matthew, thoughts?
(2 comments)
Patch Set #1, Line 104:
// If nil the default dialer is used.If nil, (comma)
Patch Set #1, Line 105:
DNSDialer func(ctx context.Context, network, host string) (Conn, error)Why a func instead of a *Dialer?
Also, the DNS prefix is probably unnecessary. It's already a field on a type named "Resolver".
To view, visit change 37260. To unsubscribe, visit settings.
Matthew Dempsky (Gerrit)
Matthew Dempsky posted comments on this change.
Patch set 1:
(1 comment)
Patch Set #1, Line 105:
DNSDialer func(ctx context.Context, network, host string) (Conn, error)
Why a func instead of a *Dialer?
FWIW, net/http.Transport has Dial and DialContext function-typed fields.
Agreed on dropping the "DNS" prefix. We only ever Dial DNS servers. net/http.Transport doesn't call its HTTPDial. (Though it does have DialTLS, so if anything it should be DialDNS.)
My 2c is one of:
Dial func(ctx context.Context, network, host string) (Conn, error)
Dialer *Dialer
I suppose the advantage of using the function is users can provide whatever fancy custom logic they want, whereas using *Dialer they're limited to what net.Dialer.Dial supports.
Shrug. I'm fine either way. I'd probably lean towards the func for consistency with net/http.
To view, visit change 37260. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch Set #1, Line 105:
DNSDialer func(ctx context.Context, network, host string) (Conn, error)
FWIW, net/http.Transport has Dial and DialContext function-typed fields.
net/http.Transport has its fields because they predate net.Dialer. :-/
To view, visit change 37260. To unsubscribe, visit settings.
Matthew Dempsky (Gerrit)
Matthew Dempsky posted comments on this change.
Patch Set #1, Line 105:
DNSDialer func(ctx context.Context, network, host string) (Conn, error)
net/http.Transport has its fields because they predate net.Dialer. :-/
Ah. I suppose that updates me towards *Dialer.
To view, visit change 37260. To unsubscribe, visit settings.
Mikio Hara (Gerrit)
Mikio Hara posted comments on this change.
Patch Set #1, Line 105:
host stringwhat's your plan on DNS transport support?
"addr net.Addr" might be clean if your plan is just for making classic DNS transports.
if you think there's a possibility of accommodating fancy transports such as DNS over TLS or DNS over HTTP(S), taking a string as a remote identifier, which might be a URL, IP literal or name and service name or port number separated by colon, makes sense.
To view, visit change 37260. To unsubscribe, visit settings.
Matt Harden (Gerrit)
Matt Harden uploaded patch set #2 to this change.
net: allow Resolver to use a custom dialer In some cases it is desirable to customize the way the DNS server is contacted, for instance to use a specific LocalAddr. While most operating-system level resolvers do not allow this, we have the opportunity to do so with the Go resolver. Most of the code was already in place to allow tests to override the dialer. This exposes that functionality, and as a side effect eliminates the need for a testing hook. Fixes #17404 Change-Id: I1c5e570f8edbcf630090f8ec6feb52e379e3e5c0 --- M src/net/dnsclient_unix.go M src/net/dnsclient_unix_test.go M src/net/lookup.go M src/net/lookup_unix.go 4 files changed, 120 insertions(+), 113 deletions(-)
To view, visit change 37260. To unsubscribe, visit settings.
Matt Harden (Gerrit)
Matt Harden posted comments on this change.
Patch set 2:
(2 comments)
Patch Set #1, Line 105:
Dial func(ctx context.Context, network, host string) (Conn, error)
FWIW, net/http.Transport has Dial and DialContext function-typed fields.
I used a function because it's more flexible and it's easy to get the function needed from a Dialer. This is an advanced feature anyway, so why not have full flexibility? I also considered an interface (DialContexter?) but it would only have a single method.
I actually wish we had made Dialer an interface because I expect the -er suffix to mean "interface".
One reason I didn't just call it Dial in the first place is the following. Unlike http.Transport, Resolver is actually used in dialing. I could see some poor misguided person doing the following:
r := net.Resolver{PreferGo: true} c, err := r.Dial(ctx, "tcp", "example.net")expecting to get a connection. Luckily they would get a panic instead (assuming they didn't set Dial), but still it could be confusing.
Patch Set #1, Line 105:
string) (Co
what's your plan on DNS transport support?
I have no plan for exotic transports but I think that argues for the function rather than *Dialer. If we used a string, then what? We would have to add specific support for every possible transport into Resolver, which is a bad idea IMO.
To view, visit change 37260. To unsubscribe, visit settings.
Matt Harden (Gerrit)
Patch Set #1, Line 104:
// If nil, the default dialer is used.If nil, (comma)
Done
Patch Set #1, Line 105:
string) (Co
I have no plan for exotic transports but I think that argues for the functi
Done
To view, visit change 37260. To unsubscribe, visit settings.
Mikio Hara (Gerrit)
Mikio Hara posted comments on this change.
Patch set 2:
(1 comment)
Patch Set #1, Line 105:
string) (Co
have to add specific support for every possible transport into Resolver, which is a bad idea IMO.
then, isn't it better to use net.Addr instead of string?
To view, visit change 37260. To unsubscribe, visit settings.
Matt Harden (Gerrit)
Matt Harden posted comments on this change.
Patch Set #1, Line 105:
string) (Co> have to add specific support for every possible transport into Resolver,
I see now; you're talking about the hostname parameter specifically. I thought you were suggesting to use a net.Addr or string in place of the Dialer / dial function itself.
A net.Addr might be better theoretically, but the current type signature matches that of (*net.Dialer).DialContext, and that convenience trumps the safety of using net.Addr. Note that we guarantee to only pass the dialer a parseable IP address so that we don't end up with the dialer calling back into the resolver.
To view, visit change 37260. To unsubscribe, visit settings.
Matt Harden (Gerrit)
ping?
Matt Harden (Gerrit)
Matt Harden posted comments on this change.
Patch set 3:
(2 comments)
Patch Set #1, Line 105:
string) (Co
I see now; you're talking about the hostname parameter specifically. I thou
Ack
Patch Set #1, Line 105:
Dial func(ctx context.Context, network, host string) (Conn, error)
I used a function because it's more flexible and it's easy to get the funct
Where do we stand on this? Do I need to change this to a *Dialer instead of a func? I prefer the func but I'll change it if that will get this review done. :-)
To view, visit change 37260. To unsubscribe, visit settings.
Mikio Hara (Gerrit)
Mikio Hara posted comments on this change.
Patch set 3:
(1 comment)
Patch Set #1, Line 105:
string) (CoI see now; you're talking about the hostname parameter specifically.
Well, yes and no. I'm still considering another option RoundTrip. We now have the package golang.org/xnet/dns/dnsmessage, I kind of feel like preferring RoundTrip than Dial. But if you want Dial first, that's okay. In that case, please rename the parameter host to hostport because it's an IP literal followed by a colon and a port number.
To view, visit change 37260. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 3:
Paul, thoughts on this?
(2 comments)
Patch Set #3, Line 54:
dialCtx = (&Dialer{}).DialContextpull this out into a global var so you don't need to allocate (I'm guessing it allocates) each time.
Patch Set #3, Line 79:
return goLookupHostOrder(ctx, r.dnsDialer(), host, order)In Paul Marks' https://go-review.googlesource.com/c/32572/ these funcs all become methods on *Resolver, which would make this code have fewer parameters.
We should probably coordinate these two CLs.
To view, visit change 37260. To unsubscribe, visit settings.
Paul Marks (Gerrit)
Paul Marks posted comments on this change.
Patch set 3:
I concur that these functions should turn into (r *Resolver) methods, because the plumbing looks very similar to my change.
Having the ability to override the dialer seems fine, although it would probably be less painful to make the kernel pick the right source address, so that all applications benefit equally.
(1 comment)
Delete "it".
To view, visit change 37260. To unsubscribe, visit settings.
Matt Harden (Gerrit)
Matt Harden posted comments on this change.
Patch set 3:
Patch Set 3:
(1 comment)
I concur that these functions should turn into (r *Resolver) methods, because the plumbing looks very similar to my change.
Having the ability to override the dialer seems fine, although it would probably be less painful to make the kernel pick the right source address, so that all applications benefit equally.
How would we make the kernel (actually the C resolver library) pick the right source address for the DNS lookup from Go? I don't think that's possible.
(1 comment)
Patch Set #1, Line 105:
string) (Co
> I see now; you're talking about the hostname parameter specifically.
What exactly would me the type signature and semantics of RoundTrip? I don't see any example in golang.org/x/net/dns/dnsmessage. I guess it would be RoundTrip(query []byte) (result []byte, err error) with the query and result encoded according to RFC 1305? Or should it be RoundTrip(questions []Question) (answers []Resource, err error) in which case we would need to bring some or all of the types from dnsmessage into the standard library, right?
In either case I think we would want to provide a ready-to-use implementation of RoundTrip that accepted a Dial function (or Dialer).
To view, visit change 37260. To unsubscribe, visit settings.
Mikio Hara (Gerrit)
Mikio Hara posted comments on this change.
Patch set 3:
(1 comment)
Patch Set #1, Line 105:
string) (Co
The latter with help of type-aliasing introduced in Go 1.9, but the former is also fine. The RoundTrip-ish stuff would allow us to make the standard library work together with external DNS extension packages.
RoundTrip that accepted a Dial function (or Dialer).
Yup, we probably can imitate the net/http package of standard library. When applying the type and interface structure in net/http to net, we can consider that http.Client is net.Resolver, http.RoundTripper is <non-existence> and http.Transport is <non-existence>.
To view, visit change 37260. To unsubscribe, visit settings.
Paul Marks (Gerrit)
Paul Marks posted comments on this change.
Patch set 3:Code-Review +1
Having the ability to override the dialer seems fine, although it would probably be less painful to make the kernel pick the right source address, so that all applications benefit equally.
How would we make the kernel (actually the C resolver library) pick the right source address for the DNS lookup from Go? I don't think that's possible.
My point is that source address selection is typically the kernel's responsibility, so Go needn't be involved at all. I'm not against this feature, but some people who use it might be patching a problem at the "wrong" layer of the stack.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 3:
Matt, can you rebase this now that Paul's CL is in?
Matt Harden (Gerrit)
Matt Harden uploaded patch set #4 to this change.
net: allow Resolver to use a custom dialer In some cases it is desirable to customize the way the DNS server is contacted, for instance to use a specific LocalAddr. While most operating-system level resolvers do not allow this, we have the opportunity to do so with the Go resolver. Most of the code was already in place to allow tests to override the dialer. This exposes that functionality, and as a side effect eliminates the need for a testing hook. Fixes #17404 Change-Id: I1c5e570f8edbcf630090f8ec6feb52e379e3e5c0 --- M src/net/dnsclient_unix.go M src/net/dnsclient_unix_test.go M src/net/lookup.go M src/net/lookup_unix.go 4 files changed, 200 insertions(+), 130 deletions(-)
diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go
index 6613dc7..75d70d3 100644
--- a/src/net/dnsclient_unix.go
+++ b/src/net/dnsclient_unix.go
@@ -25,13 +25,6 @@
"time"
)
-// A dnsDialer provides dialing suitable for DNS queries.
-type dnsDialer interface {
- dialDNS(ctx context.Context, network, addr string) (dnsConn, error)
-}
-
-var testHookDNSDialer = func() dnsDialer { return &Dialer{} }
-
// A dnsConn represents a DNS transport endpoint.
type dnsConn interface {
io.Closer
@@ -116,33 +109,8 @@
return resp, nil
}
-func (d *Dialer) dialDNS(ctx context.Context, network, server string) (dnsConn, error) {
- switch network {
- case "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6":
- default:
- return nil, UnknownNetworkError(network)
- }
- // Calling Dial here is scary -- we have to be sure not to
- // dial a name that will require a DNS lookup, or Dial will
- // call back here to translate it. The DNS config parser has
- // already checked that all the cfg.servers are IP
- // addresses, which Dial will use without a DNS lookup.
- c, err := d.DialContext(ctx, network, server)
- if err != nil {
- return nil, mapErr(err)
- }
- switch network {
- case "tcp", "tcp4", "tcp6":
- return c.(*TCPConn), nil
- case "udp", "udp4", "udp6":
- return c.(*UDPConn), nil
- }
- panic("unreachable")
-}
-
// exchange sends a query on the connection and hopes for a response.
-func exchange(ctx context.Context, server, name string, qtype uint16, timeout time.Duration) (*dnsMsg, error) {
- d := testHookDNSDialer()
+func (r *Resolver) exchange(ctx context.Context, server, name string, qtype uint16, timeout time.Duration) (*dnsMsg, error) {
out := dnsMsg{
dnsMsgHdr: dnsMsgHdr{
recursion_desired: true,
@@ -158,7 +126,7 @@
ctx, cancel := context.WithDeadline(ctx, time.Now().Add(timeout))
defer cancel()
- c, err := d.dialDNS(ctx, network, server)
+ c, err := r.dial(ctx, network, server)
if err != nil {
return nil, err
}
@@ -181,7 +149,7 @@
// Do a lookup for a single name, which must be rooted
// (otherwise answer will not find the answers).
-func tryOneName(ctx context.Context, cfg *dnsConfig, name string, qtype uint16) (string, []dnsRR, error) {
+func (r *Resolver) tryOneName(ctx context.Context, cfg *dnsConfig, name string, qtype uint16) (string, []dnsRR, error) {
var lastErr error
serverOffset := cfg.serverOffset()
sLen := uint32(len(cfg.servers))
@@ -190,7 +158,7 @@
for j := uint32(0); j < sLen; j++ {
server := cfg.servers[(serverOffset+j)%sLen]
- msg, err := exchange(ctx, server, name, qtype, cfg.timeout)
+ msg, err := r.exchange(ctx, server, name, qtype, cfg.timeout)
if err != nil {
lastErr = &DNSError{
Err: err.Error(),
@@ -333,7 +301,7 @@
conf := resolvConf.dnsConfig
resolvConf.mu.RUnlock()
for _, fqdn := range conf.nameList(name) {
- cname, rrs, err = tryOneName(ctx, conf, fqdn, qtype)
+ cname, rrs, err = r.tryOneName(ctx, conf, fqdn, qtype)
if err == nil {
break
}
@@ -512,7 +480,7 @@
for _, fqdn := range conf.nameList(name) {
for _, qtype := range qtypes {
go func(qtype uint16) {
- cname, rrs, err := tryOneName(ctx, conf, fqdn, qtype)
+ cname, rrs, err := r.tryOneName(ctx, conf, fqdn, qtype)
lane <- racer{cname, rrs, err}
}(qtype)
}
diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go
index ec28af6..3bb146e 100644
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -10,7 +10,6 @@
"context"
"fmt"
"internal/poll"
- "internal/testenv"
"io/ioutil"
"os"
"path"
@@ -20,6 +19,8 @@
"testing"
"time"
)
+
+var goResolver = Resolver{PreferGo: true}
// Test address from 192.0.2.0/24 block, reserved by RFC 5737 for documentation.
const TestAddr uint32 = 0xc0000201
@@ -41,18 +42,30 @@
}
func TestDNSTransportFallback(t *testing.T) {
- testenv.MustHaveExternalNetwork(t)
-
+ fake := fakeDNSServer{
+ rh: func(n, _ string, _ *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ r := &dnsMsg{
+ dnsMsgHdr: dnsMsgHdr{
+ rcode: dnsRcodeSuccess,
+ },
+ }
+ if n == "udp" {
+ r.truncated = true
+ }
+ return r, nil
+ },
+ }
+ r := Resolver{PreferGo: true, Dial: fake.DialContext}
for _, tt := range dnsTransportFallbackTests {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
- msg, err := exchange(ctx, tt.server, tt.name, tt.qtype, time.Second)
+ msg, err := r.exchange(ctx, tt.server, tt.name, tt.qtype, time.Second)
if err != nil {
t.Error(err)
continue
}
switch msg.rcode {
- case tt.rcode, dnsRcodeServerFailure:
+ case tt.rcode:
default:
t.Errorf("got %v from %v; want %v", msg.rcode, tt.server, tt.rcode)
continue
@@ -82,13 +95,28 @@
}
func TestSpecialDomainName(t *testing.T) {
- testenv.MustHaveExternalNetwork(t)
+ fake := fakeDNSServer{func(_, _ string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ r := &dnsMsg{
+ dnsMsgHdr: dnsMsgHdr{
+ id: q.id,
+ },
+ }
+ switch q.question[0].Name {
+ case "example.com.":
+ r.rcode = dnsRcodeSuccess
+ default:
+ r.rcode = dnsRcodeNameError
+ }
+
+ return r, nil
+ }}
+ r := Resolver{PreferGo: true, Dial: fake.DialContext}
server := "8.8.8.8:53"
for _, tt := range specialDomainNameTests {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
- msg, err := exchange(ctx, server, tt.name, tt.qtype, 3*time.Second)
+ msg, err := r.exchange(ctx, server, tt.name, tt.qtype, 3*time.Second)
if err != nil {
t.Error(err)
continue
@@ -143,14 +171,39 @@
}
}
+var fakeDNSServerSuccessful = fakeDNSServer{func(_, _ string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ r := &dnsMsg{
+ dnsMsgHdr: dnsMsgHdr{
+ id: q.id,
+ response: true,
+ },
+ question: q.question,
+ }
+ if len(q.question) == 1 && q.question[0].Qtype == dnsTypeA {
+ r.answer = []dnsRR{
+ &dnsRR_A{
+ Hdr: dnsRR_Header{
+ Name: q.question[0].Name,
+ Rrtype: dnsTypeA,
+ Class: dnsClassINET,
+ Rdlength: 4,
+ },
+ A: TestAddr,
+ },
+ }
+ }
+ return r, nil
+}}
+
// Issue 13705: don't try to resolve onion addresses, etc
func TestLookupTorOnion(t *testing.T) {
- addrs, err := DefaultResolver.goLookupIP(context.Background(), "foo.onion")
- if len(addrs) > 0 {
- t.Errorf("unexpected addresses: %v", addrs)
- }
+ r := Resolver{PreferGo: true, Dial: fakeDNSServerSuccessful.DialContext}
+ addrs, err := r.LookupIPAddr(context.Background(), "foo.onion")
if err != nil {
t.Fatalf("lookup = %v; want nil", err)
+ }
+ if len(addrs) > 0 {
+ t.Errorf("unexpected addresses: %v", addrs)
}
}
@@ -241,7 +294,7 @@
}
func TestUpdateResolvConf(t *testing.T) {
- testenv.MustHaveExternalNetwork(t)
+ r := Resolver{PreferGo: true, Dial: fakeDNSServerSuccessful.DialContext}
conf, err := newResolvConfTest()
if err != nil {
@@ -261,7 +314,7 @@
for j := 0; j < N; j++ {
go func(name string) {
defer wg.Done()
- ips, err := DefaultResolver.goLookupIP(context.Background(), name)
+ ips, err := r.LookupIPAddr(context.Background(), name)
if err != nil {
t.Error(err)
return
@@ -396,7 +449,60 @@
}
func TestGoLookupIPWithResolverConfig(t *testing.T) {
- testenv.MustHaveExternalNetwork(t)
+ fake := fakeDNSServer{func(n, s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ switch s {
+ case "[2001:4860:4860::8888]:53", "8.8.8.8:53":
+ break
+ default:
+ time.Sleep(10 * time.Millisecond)
+ return nil, poll.ErrTimeout
+ }
+ r := &dnsMsg{
+ dnsMsgHdr: dnsMsgHdr{
+ id: q.id,
+ response: true,
+ },
+ question: q.question,
+ }
+ for _, question := range q.question {
+ switch question.Qtype {
+ case dnsTypeA:
+ switch question.Name {
+ case "hostname.as112.net.":
+ break
+ case "ipv4.google.com.":
+ r.answer = append(r.answer, &dnsRR_A{
+ Hdr: dnsRR_Header{
+ Name: q.question[0].Name,
+ Rrtype: dnsTypeA,
+ Class: dnsClassINET,
+ Rdlength: 4,
+ },
+ A: TestAddr,
+ })
+ default:
+
+ }
+ case dnsTypeAAAA:
+ switch question.Name {
+ case "hostname.as112.net.":
+ break
+ case "ipv6.google.com.":
+ r.answer = append(r.answer, &dnsRR_AAAA{
+ Hdr: dnsRR_Header{
+ Name: q.question[0].Name,
+ Rrtype: dnsTypeAAAA,
+ Class: dnsClassINET,
+ Rdlength: 16,
+ },
+ AAAA: TestAddr6,
+ })
+ }
+ }
+ }
+ return r, nil
+ }}
+ r := Resolver{PreferGo: true, Dial: fake.DialContext}
conf, err := newResolvConfTest()
if err != nil {
@@ -409,14 +515,8 @@
t.Error(err)
continue
}
- addrs, err := DefaultResolver.goLookupIP(context.Background(), tt.name)
+ addrs, err := r.LookupIPAddr(context.Background(), tt.name)
if err != nil {
- // This test uses external network connectivity.
- // We need to take care with errors on both
- // DNS message exchange layer and DNS
- // transport layer because goLookupIP may fail
- // when the IP connectivity on node under test
- // gets lost during its run.
if err, ok := err.(*DNSError); !ok || tt.error != nil && (err.Name != tt.error.(*DNSError).Name || err.Server != tt.error.(*DNSError).Server || err.IsTimeout != tt.error.(*DNSError).IsTimeout) {
t.Errorf("got %v; want %v", err, tt.error)
}
@@ -441,7 +541,17 @@
// Test that goLookupIPOrder falls back to the host file when no DNS servers are available.
func TestGoLookupIPOrderFallbackToFile(t *testing.T) {
- testenv.MustHaveExternalNetwork(t)
+ fake := fakeDNSServer{func(n, s string, q *dnsMsg, tm time.Time) (*dnsMsg, error) {
+ r := &dnsMsg{
+ dnsMsgHdr: dnsMsgHdr{
+ id: q.id,
+ response: true,
+ },
+ question: q.question,
+ }
+ return r, nil
+ }}
+ r := Resolver{PreferGo: true, Dial: fake.DialContext}
// Add a config that simulates no dns servers being available.
conf, err := newResolvConfTest()
@@ -459,14 +569,14 @@
name := fmt.Sprintf("order %v", order)
// First ensure that we get an error when contacting a non-existent host.
- _, _, err := DefaultResolver.goLookupIPCNAMEOrder(context.Background(), "notarealhost", order)
+ _, _, err := r.goLookupIPCNAMEOrder(context.Background(), "notarealhost", order)
if err == nil {
t.Errorf("%s: expected error while looking up name not in hosts file", name)
continue
}
// Now check that we get an address when the name appears in the hosts file.
- addrs, _, err := DefaultResolver.goLookupIPCNAMEOrder(context.Background(), "thor", order) // entry is in "testdata/hosts"
+ addrs, _, err := r.goLookupIPCNAMEOrder(context.Background(), "thor", order) // entry is in "testdata/hosts"
if err != nil {
t.Errorf("%s: expected to successfully lookup host entry", name)
continue
@@ -489,9 +599,6 @@
func TestErrorForOriginalNameWhenSearching(t *testing.T) {
const fqdn = "doesnotexist.domain"
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -502,10 +609,7 @@
t.Fatal(err)
}
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
- d.rh = func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ fake := fakeDNSServer{func(_, _ string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
r := &dnsMsg{
dnsMsgHdr: dnsMsgHdr{
id: q.id,
@@ -520,7 +624,7 @@
}
return r, nil
- }
+ }}
cases := []struct {
strictErrors bool
@@ -530,8 +634,8 @@
{false, &DNSError{Name: fqdn, Err: errNoSuchHost.Error()}},
}
for _, tt := range cases {
- r := Resolver{StrictErrors: tt.strictErrors}
- _, err = r.goLookupIP(context.Background(), fqdn)
+ r := Resolver{PreferGo: true, StrictErrors: tt.strictErrors, Dial: fake.DialContext}
+ _, err = r.LookupIPAddr(context.Background(), fqdn)
if err == nil {
t.Fatal("expected an error")
}
@@ -545,9 +649,6 @@
// Issue 15434. If a name server gives a lame referral, continue to the next.
func TestIgnoreLameReferrals(t *testing.T) {
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -559,10 +660,7 @@
t.Fatal(err)
}
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
- d.rh = func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ fake := fakeDNSServer{func(_, s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
t.Log(s, q)
r := &dnsMsg{
dnsMsgHdr: dnsMsgHdr{
@@ -590,9 +688,10 @@
}
return r, nil
- }
+ }}
+ r := Resolver{PreferGo: true, Dial: fake.DialContext}
- addrs, err := DefaultResolver.goLookupIP(context.Background(), "www.golang.org")
+ addrs, err := r.LookupIPAddr(context.Background(), "www.golang.org")
if err != nil {
t.Fatal(err)
}
@@ -611,7 +710,7 @@
ctx := context.Background()
for i := 0; i < b.N; i++ {
- DefaultResolver.goLookupIP(ctx, "www.example.com")
+ goResolver.LookupIPAddr(ctx, "www.example.com")
}
}
@@ -620,7 +719,7 @@
ctx := context.Background()
for i := 0; i < b.N; i++ {
- DefaultResolver.goLookupIP(ctx, "some.nonexistent")
+ goResolver.LookupIPAddr(ctx, "some.nonexistent")
}
}
@@ -643,23 +742,24 @@
ctx := context.Background()
for i := 0; i < b.N; i++ {
- DefaultResolver.goLookupIP(ctx, "www.example.com")
+ goResolver.LookupIPAddr(ctx, "www.example.com")
}
}
-type fakeDNSDialer struct {
- // reply handler
- rh func(s string, q *dnsMsg, t time.Time) (*dnsMsg, error)
+type fakeDNSServer struct {
+ rh func(n, s string, q *dnsMsg, t time.Time) (*dnsMsg, error)
}
-func (f *fakeDNSDialer) dialDNS(_ context.Context, n, s string) (dnsConn, error) {
- return &fakeDNSConn{f.rh, s, time.Time{}}, nil
+func (server *fakeDNSServer) DialContext(_ context.Context, n, s string) (Conn, error) {
+ return &fakeDNSConn{nil, server, n, s, time.Time{}}, nil
}
type fakeDNSConn struct {
- rh func(s string, q *dnsMsg, t time.Time) (*dnsMsg, error)
- s string
- t time.Time
+ Conn
+ server *fakeDNSServer
+ n string
+ s string
+ t time.Time
}
func (f *fakeDNSConn) Close() error {
@@ -672,7 +772,7 @@
}
func (f *fakeDNSConn) dnsRoundTrip(q *dnsMsg) (*dnsMsg, error) {
- return f.rh(f.s, q, f.t)
+ return f.server.rh(f.n, f.s, q, f.t)
}
// UDP round-tripper algorithm should ignore invalid DNS responses (issue 13281).
@@ -749,9 +849,6 @@
// Issue 16865. If a name server times out, continue to the next.
func TestRetryTimeout(t *testing.T) {
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -766,12 +863,9 @@
t.Fatal(err)
}
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
var deadline0 time.Time
- d.rh = func(s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
+ fake := fakeDNSServer{func(_, s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
t.Log(s, q, deadline)
if deadline.IsZero() {
@@ -789,9 +883,10 @@
}
return mockTXTResponse(q), nil
- }
+ }}
+ r := &Resolver{PreferGo: true, Dial: fake.DialContext}
- _, err = LookupTXT("www.golang.org")
+ _, err = r.LookupTXT(context.Background(), "www.golang.org")
if err != nil {
t.Fatal(err)
}
@@ -810,9 +905,6 @@
}
func testRotate(t *testing.T, rotate bool, nameservers, wantServers []string) {
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -831,18 +923,16 @@
t.Fatal(err)
}
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
var usedServers []string
- d.rh = func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) {
+ fake := fakeDNSServer{func(_, s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
usedServers = append(usedServers, s)
return mockTXTResponse(q), nil
- }
+ }}
+ r := Resolver{PreferGo: true, Dial: fake.DialContext}
// len(nameservers) + 1 to allow rotation to get back to start
for i := 0; i < len(nameservers)+1; i++ {
- if _, err := LookupTXT("www.golang.org"); err != nil {
+ if _, err := r.LookupTXT(context.Background(), "www.golang.org"); err != nil {
t.Fatal(err)
}
}
@@ -878,9 +968,6 @@
// Issue 17448. With StrictErrors enabled, temporary errors should make
// LookupIP fail rather than return a partial result.
func TestStrictErrorsLookupIP(t *testing.T) {
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -1017,10 +1104,7 @@
}
for i, tt := range cases {
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
- d.rh = func(s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
+ fake := fakeDNSServer{func(_, s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
t.Log(s, q)
switch tt.resolveWhich(&q.question[0]) {
@@ -1082,11 +1166,11 @@
return nil, fmt.Errorf("Unexpected Qtype: %v", q.question[0].Qtype)
}
return r, nil
- }
+ }}
for _, strict := range []bool{true, false} {
- r := Resolver{StrictErrors: strict}
- ips, err := r.goLookupIP(context.Background(), name)
+ r := Resolver{PreferGo: true, StrictErrors: strict, Dial: fake.DialContext}
+ ips, err := r.LookupIPAddr(context.Background(), name)
var wantErr error
if strict {
@@ -1118,9 +1202,6 @@
// Issue 17448. With StrictErrors enabled, temporary errors should make
// LookupTXT stop walking the search list.
func TestStrictErrorsLookupTXT(t *testing.T) {
- origTestHookDNSDialer := testHookDNSDialer
- defer func() { testHookDNSDialer = origTestHookDNSDialer }()
-
conf, err := newResolvConfTest()
if err != nil {
t.Fatal(err)
@@ -1141,10 +1222,7 @@
const searchY = "test.y.golang.org."
const txt = "Hello World"
- d := &fakeDNSDialer{}
- testHookDNSDialer = func() dnsDialer { return d }
-
- d.rh = func(s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
+ fake := fakeDNSServer{func(_, s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) {
t.Log(s, q)
switch q.question[0].Name {
@@ -1155,10 +1233,10 @@
default:
return nil, fmt.Errorf("Unexpected Name: %v", q.question[0].Name)
}
- }
+ }}
for _, strict := range []bool{true, false} {
- r := Resolver{StrictErrors: strict}
+ r := Resolver{StrictErrors: strict, Dial: fake.DialContext}
_, rrs, err := r.lookup(context.Background(), name, dnsTypeTXT)
var wantErr error
var wantRRs int
diff --git a/src/net/lookup.go b/src/net/lookup.go
index 463b374..adb929c 100644
--- a/src/net/lookup.go
+++ b/src/net/lookup.go
@@ -107,6 +107,13 @@
// with resolvers that process AAAA queries incorrectly.
StrictErrors bool
+ // Dial is used by Go's built-in DNS resolver to make TCP and
+ // UDP connections to DNS services. The Resolver will never call this
+ // with names, only valid IP addresses. The Conn returned must be
+ // a *TCPConn or *UDPConn as requested by the network parameter.
+ // If nil the default dialer is used.
+ Dial func(ctx context.Context, network, hostport string) (Conn, error)
+
// TODO(bradfitz): optional interface impl override hook
// TODO(bradfitz): Timeout time.Duration?
}
diff --git a/src/net/lookup_unix.go b/src/net/lookup_unix.go
index 8d4b7bd..641ffe7 100644
--- a/src/net/lookup_unix.go
+++ b/src/net/lookup_unix.go
@@ -48,6 +48,23 @@
return lookupProtocolMap(name)
}
+func (r *Resolver) dial(ctx context.Context, network, server string) (dnsConn, error) {
+ dialCtx := r.Dial
+ if dialCtx == nil {
+ dialCtx = (&Dialer{}).DialContext
+ }
+ // Calling Dial here is scary -- we have to be sure not to
+ // dial a name that will require a DNS lookup, or Dial will
+ // call back here to translate it. The DNS config parser has
+ // already checked that all the cfg.servers are IP
+ // addresses, which Dial will use without a DNS lookup.
+ c, err := dialCtx(ctx, network, server)
+ if err != nil {
+ return nil, mapErr(err)
+ }
+ return c.(dnsConn), nil
+}
+
func (r *Resolver) lookupHost(ctx context.Context, host string) (addrs []string, err error) {
order := systemConf().hostLookupOrder(host)
if !r.PreferGo && order == hostLookupCgo {
To view, visit change 37260. To unsubscribe, visit settings.
Matt Harden (Gerrit)
Matt Harden posted comments on this change.
Patch set 4:
Patch Set 3:
Matt, can you rebase this now that Paul's CL is in?
Done. I also updated the tests so that none of them require internet connectivity to run (but they still test what I believe they were intended to).
Matt Harden (Gerrit)
Patch Set 3:
(1 comment)
I'm leaning toward RoundTrip as well, now. Does anyone want to argue against that? Modeling after http, I think it should accept and return structures like Question and Resource (DNSQuestion and DNSResource?), which would need to be exposed by the package.
Matt Harden (Gerrit)
Patch Set 3: Code-Review+1
Having the ability to override the dialer seems fine, although it would probably be less painful to make the kernel pick the right source address, so that all applications benefit equally.
How would we make the kernel (actually the C resolver library) pick the right source address for the DNS lookup from Go? I don't think that's possible.
My point is that source address selection is typically the kernel's responsibility, so Go needn't be involved at all. I'm not against this feature, but some people who use it might be patching a problem at the "wrong" layer of the stack.
OK I see your point, I think. There is a list of DNS servers to contact, and the kernel should choose the right source IP automatically when connecting to them. I guess in this user's case it isn't doing so?
Matt Harden (Gerrit)
(1 comment)
In Paul Marks' https://go-review.googlesource.com/c/32572/ these funcs all
Ack
To view, visit change 37260. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 5:
(4 comments)
The Resolver will never call this // with names, only valid IP addresses.
The provided addr argument is always an IP address and port; it is never a DNS name.
Patch Set #4, Line 114:
// If nil the default dialer is used.If nil, the ...
but I try to call out optional things even earlier.
// Dial optionally specifies an alternate dialer for Go's built-in DNS resolver // to make ...
Patch Set #4, Line 115:
Dial func(ctx context.Context, network, hostport string) (Conn, error)"addr", to remove the name "host" from
Patch Set #4, Line 54:
(&Dialer{}).DialContextsee my earlier comment?
To view, visit change 37260. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
RELNOTE=yes
Brad Fitzpatrick (Gerrit)
(3 comments)
The Resolver will never call this
// with names, only valid IP addresses.The provided addr argument is always an IP address and port; it is never a
Done
Patch Set #4, Line 114:
// If nil the default dialer is used.If nil, the ...
Done
Patch Set #4, Line 115:
Dial func(ctx context.Context, network, hostport string) (Conn, error)"addr", to remove the name "host" from
Done
To view, visit change 37260. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick uploaded patch set #6 to the change originally created by Matt Harden.
net: allow Resolver to use a custom dialer
In some cases it is desirable to customize the way the DNS server is
contacted, for instance to use a specific LocalAddr. While most
operating-system level resolvers do not allow this, we have the
opportunity to do so with the Go resolver. Most of the code was
already in place to allow tests to override the dialer. This exposes
that functionality, and as a side effect eliminates the need for a
testing hook.
Fixes #17404
Change-Id: I1c5e570f8edbcf630090f8ec6feb52e379e3e5c0
---
M src/net/dnsclient_unix.go
M src/net/dnsclient_unix_test.go
M src/net/lookup.go
M src/net/lookup_unix.go
4 files changed, 211 insertions(+), 130 deletions(-)
To view, visit change 37260. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 6:Run-TryBot +1Code-Review +2
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 6:
TryBots beginning. Status page: https://farmer.golang.org/try?commit=a1dd7e61
Gobot Gobot (Gerrit)
Build is still in progress...
This change failed on darwin-amd64-10_11:
See https://storage.googleapis.com/go-build-log/a1dd7e61/darwin-amd64-10_11_c7de3701.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 6:TryBot-Result -1
9 of 18 TryBots failed:
Failed on darwin-amd64-10_11: https://storage.googleapis.com/go-build-log/a1dd7e61/darwin-amd64-10_11_c7de3701.log
Failed on freebsd-amd64-gce101: https://storage.googleapis.com/go-build-log/a1dd7e61/freebsd-amd64-gce101_c3019716.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/a1dd7e61/linux-amd64_c0c2acca.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/a1dd7e61/linux-386_5d39dc92.log
Failed on windows-amd64-gce: https://storage.googleapis.com/go-build-log/a1dd7e61/windows-amd64-gce_a974311c.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/a1dd7e61/linux-amd64-race_64442594.log
Failed on windows-386-gce: https://storage.googleapis.com/go-build-log/a1dd7e61/windows-386-gce_ea870a7b.log
Failed on openbsd-amd64-60: https://storage.googleapis.com/go-build-log/a1dd7e61/openbsd-amd64-60_dc7a0bcc.log
Failed on linux-arm: https://storage.googleapis.com/go-build-log/a1dd7e61/linux-arm_e3f71583.log
Consult https://build.golang.org/ to see whether they are new failures.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick uploaded patch set #7 to the change originally created by Matt Harden.
net: allow Resolver to use a custom dialer
In some cases it is desirable to customize the way the DNS server is
contacted, for instance to use a specific LocalAddr. While most
operating-system level resolvers do not allow this, we have the
opportunity to do so with the Go resolver. Most of the code was
already in place to allow tests to override the dialer. This exposes
that functionality, and as a side effect eliminates the need for a
testing hook.
Fixes #17404
Change-Id: I1c5e570f8edbcf630090f8ec6feb52e379e3e5c0
---
M src/go/build/deps_test.go
M src/net/dnsclient_unix.go
M src/net/dnsclient_unix_test.go
M src/net/lookup.go
M src/net/lookup_unix.go
5 files changed, 213 insertions(+), 131 deletions(-)
To view, visit change 37260. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 7:Run-TryBot +1
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 7:
TryBots beginning. Status page: https://farmer.golang.org/try?commit=d82adc15
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick posted comments on this change.
Patch set 7:
(1 comment)
File src/go/build/deps_test.go:
Patch Set #7, Line 307:
"context", "math/rand", "os", "reflect", "sort", "syscall", "time",I added reflect so the error message can be more useful. reflect was already imported via "sort", so it's not a real addition.
To view, visit change 37260. To unsubscribe, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot posted comments on this change.
Patch set 7:TryBot-Result +1
TryBots are happy.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick merged this change.
net: allow Resolver to use a custom dialer
In some cases it is desirable to customize the way the DNS server is
contacted, for instance to use a specific LocalAddr. While most
operating-system level resolvers do not allow this, we have the
opportunity to do so with the Go resolver. Most of the code was
already in place to allow tests to override the dialer. This exposes
that functionality, and as a side effect eliminates the need for a
testing hook.
Fixes #17404
Change-Id: I1c5e570f8edbcf630090f8ec6feb52e379e3e5c0
Reviewed-on: https://go-review.googlesource.com/37260
Run-TryBot: Brad Fitzpatrick <brad...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
---
M src/go/build/deps_test.go
M src/net/dnsclient_unix.go
M src/net/dnsclient_unix_test.go
M src/net/lookup.go
M src/net/lookup_unix.go
5 files changed, 213 insertions(+), 131 deletions(-)
diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
index 2cceb5a..ec8dd06 100644
--- a/src/go/build/deps_test.go
+++ b/src/go/build/deps_test.go
@@ -304,7 +304,7 @@
// do networking portably, it must have a small dependency set: just L0+basic os.
"net": {
"L0", "CGO",
- "context", "math/rand", "os", "sort", "syscall", "time",
+ "context", "math/rand", "os", "reflect", "sort", "syscall", "time",
"internal/nettrace", "internal/poll",
"internal/syscall/windows", "internal/singleflight", "internal/race",
"golang_org/x/net/lif", "golang_org/x/net/route",
index a23e5f6..d0ac430 100644
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -10,7 +10,6 @@
"context"
"fmt"
"internal/poll"
- "internal/testenv"
"io/ioutil"
"os"
"path"
@@ -21,6 +20,8 @@
"time"
)
+var goResolver = Resolver{PreferGo: true}
+@@ -143,15 +171,40 @@+ }
}
type resolvConfTest struct {
index 463b374..818f91c 100644
--- a/src/net/lookup.go
+++ b/src/net/lookup.go
@@ -107,6 +107,15 @@
// with resolvers that process AAAA queries incorrectly.
StrictErrors bool
+ // Dial optionally specifies an alternate dialer for use by
+ // Go's built-in DNS resolver to make TCP and UDP connections
+ // to DNS services. The provided addr will always be an IP
+ // address and not a hostname.
+ // The Conn returned must be a *TCPConn or *UDPConn as
+ // requested by the network parameter. If nil, the default
+ // dialer is used.
+ Dial func(ctx context.Context, network, addr string) (Conn, error)
+
// TODO(bradfitz): optional interface impl override hook
// TODO(bradfitz): Timeout time.Duration?
}
diff --git a/src/net/lookup_unix.go b/src/net/lookup_unix.go
index 158cc94..a485d70 100644
--- a/src/net/lookup_unix.go
+++ b/src/net/lookup_unix.go
@@ -8,6 +8,8 @@
import (
"context"
+ "errors"
+ "reflect"
"sync"
)
@@ -51,6 +53,31 @@
return lookupProtocolMap(name)
}
+func (r *Resolver) dial(ctx context.Context, network, server string) (dnsConn, error) {
+ // Calling Dial here is scary -- we have to be sure not to
+ // dial a name that will require a DNS lookup, or Dial will
+ // call back here to translate it. The DNS config parser has
+ // already checked that all the cfg.servers are IP
+ // addresses, which Dial will use without a DNS lookup.
+ var c Conn
+ var err error
+ if r.Dial != nil {
+ c, err = r.Dial(ctx, network, server)
+ } else {
+ var d Dialer
+ c, err = d.DialContext(ctx, network, server)
+ }
+ if err != nil {
+ return nil, mapErr(err)
+ }
+ dc, ok := c.(dnsConn)
+ if !ok {
+ c.Close()
+ return nil, errors.New("net: Resolver.Dial returned unsupported connection type " + reflect.TypeOf(c).String())
+ }
+ return dc, nil
+}
+
func (r *Resolver) lookupHost(ctx context.Context, host string) (addrs []string, err error) {
order := systemConf().hostLookupOrder(host)
if !r.PreferGo && order == hostLookupCgo {
To view, visit change 37260. To unsubscribe, visit settings.