net: force LookupAddr results to be rooted DNS paths when using cgo

Go 1.4 and before have always returned DNS names with a trailing dot
for reverse lookups, as they do for basically all other routines returning
DNS names. Go 1.4 and before always implemented LookupAddr using
pure Go (not C library calls).

Go 1.5 added the ability to make a C library call to implement LookupAddr.
Unfortunately the C library call returns a DNS name without a trailing dot
(an unrooted name), meaning that if turn off cgo during make.bash then
you still get the rooted name but with cgo on you get an unrooted name.
The unrooted name is inconsistent with the pure Go implementation
and with all previous Go releases, so change it to a rooted name.

Fixes #12189.

Change-Id: I3d6b72277c121fe085ea6af30e5fe8019fc490ad
Reviewed-on: https://go-review.googlesource.com/13697
Reviewed-by: Rob Pike <r@golang.org>

4 files changed, 150 insertions, 0 deletions

diff --git a/src/net/cgo_unix.go b/src/net/cgo_unix.go
index d9d5f0377c..cb89d65457 100644
--- a/src/net/cgo_unix.go
+++ b/src/net/cgo_unix.go
@@ -222,6 +222,11 @@ func cgoLookupPTR(addr string) ([]string, error, bool) {
 			break
 		}
 	}
+	// Add trailing dot to match pure Go reverse resolver
+	// and all other lookup routines. See golang.org/issue/12189.
+	if len(b) > 0 && b[len(b)-1] != '.' {
+		b = append(b, '.')
+	}
 	return []string{string(b)}, nil, true
 }
 
diff --git a/src/net/lookup_test.go b/src/net/lookup_test.go
index a42ae298ef..86957b5575 100644
--- a/src/net/lookup_test.go
+++ b/src/net/lookup_test.go
@@ -5,6 +5,7 @@
 package net
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 	"testing"
@@ -392,3 +393,111 @@ func TestLookupIPDeadline(t *testing.T) {
 	// happen due to issue 4856 for now.
 	t.Logf("%v succeeded, %v failed (%v timeout, %v temporary, %v other, %v unknown)", qstats.succeeded, qstats.failed, qstats.timeout, qstats.temporary, qstats.other, qstats.unknown)
 }
+
+func TestLookupDots(t *testing.T) {
+	if testing.Short() || !*testExternal {
+		t.Skipf("skipping external network test")
+	}
+
+	fixup := forceGoDNS()
+	defer fixup()
+	testDots(t, "go")
+
+	if forceCgoDNS() {
+		testDots(t, "cgo")
+	}
+}
+
+func testDots(t *testing.T, mode string) {
+	names, err := LookupAddr("8.8.8.8") // Google dns server
+	if err != nil {
+		t.Errorf("LookupAddr(8.8.8.8): %v (mode=%v)", err, mode)
+	} else {
+		for _, name := range names {
+			if !strings.HasSuffix(name, ".google.com.") {
+				t.Errorf("LookupAddr(8.8.8.8) = %v, want names ending in .google.com. with trailing dot (mode=%v)", names, mode)
+				break
+			}
+		}
+	}
+
+	cname, err := LookupCNAME("www.mit.edu")
+	if err != nil || !strings.HasSuffix(cname, ".") {
+		t.Errorf("LookupCNAME(www.mit.edu) = %v, %v, want cname ending in . with trailing dot (mode=%v)", cname, err, mode)
+	}
+
+	mxs, err := LookupMX("google.com")
+	if err != nil {
+		t.Errorf("LookupMX(google.com): %v (mode=%v)", err, mode)
+	} else {
+		for _, mx := range mxs {
+			if !strings.HasSuffix(mx.Host, ".google.com.") {
+				t.Errorf("LookupMX(google.com) = %v, want names ending in .google.com. with trailing dot (mode=%v)", mxString(mxs), mode)
+				break
+			}
+		}
+	}
+
+	nss, err := LookupNS("google.com")
+	if err != nil {
+		t.Errorf("LookupNS(google.com): %v (mode=%v)", err, mode)
+	} else {
+		for _, ns := range nss {
+			if !strings.HasSuffix(ns.Host, ".google.com.") {
+				t.Errorf("LookupNS(google.com) = %v, want names ending in .google.com. with trailing dot (mode=%v)", nsString(nss), mode)
+				break
+			}
+		}
+	}
+
+	cname, srvs, err := LookupSRV("xmpp-server", "tcp", "google.com")
+	if err != nil {
+		t.Errorf("LookupSRV(xmpp-server, tcp, google.com): %v (mode=%v)", err, mode)
+	} else {
+		if !strings.HasSuffix(cname, ".google.com.") {
+			t.Errorf("LookupSRV(xmpp-server, tcp, google.com) returned cname=%v, want name ending in .google.com. with trailing dot (mode=%v)", cname, mode)
+		}
+		for _, srv := range srvs {
+			if !strings.HasSuffix(srv.Target, ".google.com.") {
+				t.Errorf("LookupSRV(xmpp-server, tcp, google.com) returned addrs=%v, want names ending in .google.com. with trailing dot (mode=%v)", srvString(srvs), mode)
+				break
+			}
+		}
+	}
+}
+
+func mxString(mxs []*MX) string {
+	var buf bytes.Buffer
+	sep := ""
+	fmt.Fprintf(&buf, "[")
+	for _, mx := range mxs {
+		fmt.Fprintf(&buf, "%s%s:%d", sep, mx.Host, mx.Pref)
+		sep = " "
+	}
+	fmt.Fprintf(&buf, "]")
+	return buf.String()
+}
+
+func nsString(nss []*NS) string {
+	var buf bytes.Buffer
+	sep := ""
+	fmt.Fprintf(&buf, "[")
+	for _, ns := range nss {
+		fmt.Fprintf(&buf, "%s%s", sep, ns.Host)
+		sep = " "
+	}
+	fmt.Fprintf(&buf, "]")
+	return buf.String()
+}
+
+func srvString(srvs []*SRV) string {
+	var buf bytes.Buffer
+	sep := ""
+	fmt.Fprintf(&buf, "[")
+	for _, srv := range srvs {
+		fmt.Fprintf(&buf, "%s%s:%d:%d:%d", sep, srv.Target, srv.Port, srv.Priority, srv.Weight)
+		sep = " "
+	}
+	fmt.Fprintf(&buf, "]")
+	return buf.String()
+}
diff --git a/src/net/non_unix_test.go b/src/net/non_unix_test.go
new file mode 100644
index 0000000000..eddca562f9
--- /dev/null
+++ b/src/net/non_unix_test.go
@@ -0,0 +1,11 @@
+// Copyright 2015 The Go Authors.  All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build nacl plan9 windows
+
+package net
+
+// See unix_test.go for what these (don't) do.
+func forceGoDNS() func() { return func() {} }
+func forceCgoDNS() bool  { return false }
diff --git a/src/net/unix_test.go b/src/net/unix_test.go
index 59f5c2d85b..358ff31072 100644
--- a/src/net/unix_test.go
+++ b/src/net/unix_test.go
@@ -404,3 +404,28 @@ func TestUnixgramConnLocalAndRemoteNames(t *testing.T) {
 		}
 	}
 }
+
+// forceGoDNS forces the resolver configuration to use the pure Go resolver
+// and returns a fixup function to restore the old settings.
+func forceGoDNS() func() {
+	c := systemConf()
+	oldGo := c.netGo
+	oldCgo := c.netCgo
+	fixup := func() {
+		c.netGo = oldGo
+		c.netCgo = oldCgo
+	}
+	c.netGo = true
+	c.netCgo = false
+	return fixup
+}
+
+// forceCgoDNS forces the resolver configuration to use the cgo resolver
+// and returns true to indicate that it did so.
+// (On non-Unix systems forceCgoDNS returns false.)
+func forceCgoDNS() bool {
+	c := systemConf()
+	c.netGo = false
+	c.netCgo = true
+	return true
+}