net: don't return IPv4 unspecified addr for Resolve*Addr of [::] or [::]:n

ResolveTCPAddr, ResolveUDPAddr, and ResolveIPAddr return at most one
address. When given a name like "golang.org" to resolve that might
have more than 1 address, the net package has historically preferred
IPv4 addresses, with the assumption that many users don't yet have
IPv6 connectivity and randomly selecting between an IPv4 address and
an IPv6 address at runtime wouldn't be a good experience for IPv4-only
users.

In CL 45088 (78cf0e56) I modified the resolution of the
unspecified/empty address to internally resolve to both IPv6 "::" and
0.0.0.0 to fix issue #18806.

That code has 3 other callers I hadn't considered, though: the
Resolve*Addr functions. Since they preferred IPv4, any Resolve*Addr of
"[::]:port" or "::" (for ResolveIPAddr) would internally resolve both
"::" and 0.0.0.0 and then prefer 0.0.0.0, even though the user was
looking up an IPv6 literal.

Add tests and fix it, not by undoing the fix to #18806 but by
selecting the preference function for Resolve*Addr more explicitly: we
still prefer IPv4, but if the address being looked up was an IPv6
literal, prefer IPv6.

The tests are skipped on machines without IPv6.

Fixes #20911

Change-Id: Ib7036cc43182ae4118cd1390c254e17c04a251a3
Reviewed-on: https://go-review.googlesource.com/47554
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/net/iprawsock.go b/src/net/iprawsock.go
index 00f59631a2a571054afc3f9e98e647555448e5ef..c4b54f00c4e6e39fb476c30e2f5775d9afd73c90 100644 (file)
--- a/src/net/iprawsock.go
+++ b/src/net/iprawsock.go
@@ -91,7 +91,7 @@ func ResolveIPAddr(network, address string) (*IPAddr, error) {
        if err != nil {
                return nil, err
        }
-       return addrs.first(isIPv4).(*IPAddr), nil
+       return addrs.forResolve(network, address).(*IPAddr), nil
 }
 
 // IPConn is the implementation of the Conn and PacketConn interfaces

diff --git a/src/net/ipsock.go b/src/net/ipsock.go
index 7dafcaf630093ab5d9305053bb1aebddac896361..947bdf34897e76e62ed65a2f0a4ad8d1212b1cbb 100644 (file)
--- a/src/net/ipsock.go
+++ b/src/net/ipsock.go
@@ -50,7 +50,7 @@ func supportsIPv4map() bool {
 // An addrList represents a list of network endpoint addresses.
 type addrList []Addr
 
-// isIPv4 returns true if the Addr contains an IPv4 address.
+// isIPv4 reports whether addr contains an IPv4 address.
 func isIPv4(addr Addr) bool {
        switch addr := addr.(type) {
        case *TCPAddr:
@@ -63,6 +63,28 @@ func isIPv4(addr Addr) bool {
        return false
 }
 
+// isNotIPv4 reports whether addr does not contain an IPv4 address.
+func isNotIPv4(addr Addr) bool { return !isIPv4(addr) }
+
+// forResolve returns the most appropriate address in address for
+// a call to ResolveTCPAddr, ResolveUDPAddr, or ResolveIPAddr.
+// IPv4 is preferred, unless addr contains an IPv6 literal.
+func (addrs addrList) forResolve(network, addr string) Addr {
+       var want6 bool
+       switch network {
+       case "ip":
+               // IPv6 literal (addr does NOT contain a port)
+               want6 = count(addr, ':') > 0
+       case "tcp", "udp":
+               // IPv6 literal. (addr contains a port, so look for '[')
+               want6 = count(addr, '[') > 0
+       }
+       if want6 {
+               return addrs.first(isNotIPv4)
+       }
+       return addrs.first(isIPv4)
+}
+
 // first returns the first address which satisfies strategy, or if
 // none do, then the first address of any kind.
 func (addrs addrList) first(strategy func(Addr) bool) Addr {

diff --git a/src/net/main_test.go b/src/net/main_test.go
index bbf32cfcd945c7ef49249cd7ed165b56d0d74b3a..3e7a85ad2d9fc461c031d7c13af14f90f952b689 100644 (file)
--- a/src/net/main_test.go
+++ b/src/net/main_test.go
@@ -89,6 +89,12 @@ func setupTestData() {
                resolveTCPAddrTests = append(resolveTCPAddrTests, resolveTCPAddrTest{"tcp6", "localhost:3", &TCPAddr{IP: IPv6loopback, Port: 3}, nil})
                resolveUDPAddrTests = append(resolveUDPAddrTests, resolveUDPAddrTest{"udp6", "localhost:3", &UDPAddr{IP: IPv6loopback, Port: 3}, nil})
                resolveIPAddrTests = append(resolveIPAddrTests, resolveIPAddrTest{"ip6", "localhost", &IPAddr{IP: IPv6loopback}, nil})
+
+               // Issue 20911: don't return IPv4 addresses for
+               // Resolve*Addr calls of the IPv6 unspecified address.
+               resolveTCPAddrTests = append(resolveTCPAddrTests, resolveTCPAddrTest{"tcp", "[::]:4", &TCPAddr{IP: IPv6unspecified, Port: 4}, nil})
+               resolveUDPAddrTests = append(resolveUDPAddrTests, resolveUDPAddrTest{"udp", "[::]:4", &UDPAddr{IP: IPv6unspecified, Port: 4}, nil})
+               resolveIPAddrTests = append(resolveIPAddrTests, resolveIPAddrTest{"ip", "::", &IPAddr{IP: IPv6unspecified}, nil})
        }
 
        ifi := loopbackInterface()

diff --git a/src/net/tcpsock.go b/src/net/tcpsock.go
index 74878fc614a6c50042fae6c6c4fc07e5c71f9b4d..e957aa3005a173e5950451ed25ef51c5d3423415 100644 (file)
--- a/src/net/tcpsock.go
+++ b/src/net/tcpsock.go
@@ -77,7 +77,7 @@ func ResolveTCPAddr(network, address string) (*TCPAddr, error) {
        if err != nil {
                return nil, err
        }
-       return addrs.first(isIPv4).(*TCPAddr), nil
+       return addrs.forResolve(network, address).(*TCPAddr), nil
 }
 
 // TCPConn is an implementation of the Conn interface for TCP network

diff --git a/src/net/udpsock.go b/src/net/udpsock.go
index 28b6906c5c91b6203a5abfc2398e7f39899372db..2c0f74fdabdd293599463cc9c494d8e1bc586d7c 100644 (file)
--- a/src/net/udpsock.go
+++ b/src/net/udpsock.go
@@ -80,7 +80,7 @@ func ResolveUDPAddr(network, address string) (*UDPAddr, error) {
        if err != nil {
                return nil, err
        }
-       return addrs.first(isIPv4).(*UDPAddr), nil
+       return addrs.forResolve(network, address).(*UDPAddr), nil
 }
 
 // UDPConn is the implementation of the Conn and PacketConn interfaces