net: don't enclose non-literal IPv6 addresses in square brackets

The net package uses various textual representations for network
identifiers and locators on the Internet protocol suite as API.
In fact, the representations are the composition of subset of multple
RFCs: RFC 3986, RFC 4007, RFC 4632, RFC 4291 and RFC 5952.

RFC 4007 describes guidelines for the use of textual representation of
IPv6 addressing/routing scope zone and doesn't prohibit the format for
implementation dependent purposes, as in, specifying a literal IPv6
address and its connected region of routing topology as application
user interface. However, a non-literal IPv6 address, for example, a
host name, with a zone enclosed in square brackets confuses us because
a zone is basically for non-global IPv6 addresses and a pair of square
brackets is used as a set of delimiters between a literal IPv6 address
and a service name or transport port number.

To mitigate such confusion, this change makes JoinHostPort not enclose
non-literal IPv6 addresses in square brackets and SplitHostPort accept
the form "host%zone:port" to recommend that anything enclosed in
square brackets should be a literal IPv6 address.

Before this change:
	JoinHostPort("name%zone", "80") = "[name%zone]:80"
	JoinHostPort("[::1%zone]", "80") = "[::1%zone]:80"
	SplitHostPort("name%zone:80") = "", "", "address name%zone:80: missing brackets in address"
	SplitHostPort("[name%zone]:80") = "name%zone", "80", nil
	SplitHostPort("[::1%zone]:80") = "::1%zone", "80", nil

After this change:
	JoinHostPort("name%zone", "80") = "name%zone:80"
	JoinHostPort("[::1%zone]", "80") = "[::1%zone]:80"
	SplitHostPort("name%zone:80") = "name%zone", "80", nil
	SplitHostPort("[name%zone]:80") = "name%zone", "80", nil // for backwards compatibility
	SplitHostPort("[::1%zone]:80") = "::1%zone", "80", nil

Also updates docs and test cases on SplitHostPort and JoinHostPort for
clarification.

Fixes #18059.
Fixes #18060.

Change-Id: I5c3ccce4fa0fbdd58f698fc280635ea4a14d2a37
Reviewed-on: https://go-review.googlesource.com/40510
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

2 files changed, 108 insertions, 59 deletions

diff --git a/src/net/ip_test.go b/src/net/ip_test.go
index f765582cad..ad13388dd2 100644
--- a/src/net/ip_test.go
+++ b/src/net/ip_test.go
@@ -469,61 +469,77 @@ func TestNetworkNumberAndMask(t *testing.T) {
 	}
 }
 
-var splitJoinTests = []struct {
-	host string
-	port string
-	join string
-}{
-	{"www.google.com", "80", "www.google.com:80"},
-	{"127.0.0.1", "1234", "127.0.0.1:1234"},
-	{"::1", "80", "[::1]:80"},
-	{"fe80::1%lo0", "80", "[fe80::1%lo0]:80"},
-	{"localhost%lo0", "80", "[localhost%lo0]:80"},
-	{"", "0", ":0"},
+func TestSplitHostPort(t *testing.T) {
+	for _, tt := range []struct {
+		hostPort string
+		host     string
+		port     string
+	}{
+		// Host name
+		{"localhost:http", "localhost", "http"},
+		{"localhost:80", "localhost", "80"},
 
-	{"google.com", "https%foo", "google.com:https%foo"}, // Go 1.0 behavior
-	{"127.0.0.1", "", "127.0.0.1:"},                     // Go 1.0 behavior
-	{"www.google.com", "", "www.google.com:"},           // Go 1.0 behavior
-}
+		// Go-specific host name with zone identifier
+		{"localhost%lo0:http", "localhost%lo0", "http"},
+		{"localhost%lo0:80", "localhost%lo0", "80"},
+		{"[localhost%lo0]:http", "localhost%lo0", "http"}, // Go 1 behavior
+		{"[localhost%lo0]:80", "localhost%lo0", "80"},     // Go 1 behavior
 
-var splitFailureTests = []struct {
-	hostPort string
-	err      string
-}{
-	{"www.google.com", "missing port in address"},
-	{"127.0.0.1", "missing port in address"},
-	{"[::1]", "missing port in address"},
-	{"[fe80::1%lo0]", "missing port in address"},
-	{"[localhost%lo0]", "missing port in address"},
-	{"localhost%lo0", "missing port in address"},
+		// IP literal
+		{"127.0.0.1:http", "127.0.0.1", "http"},
+		{"127.0.0.1:80", "127.0.0.1", "80"},
+		{"[::1]:http", "::1", "http"},
+		{"[::1]:80", "::1", "80"},
 
-	{"::1", "too many colons in address"},
-	{"fe80::1%lo0", "too many colons in address"},
-	{"fe80::1%lo0:80", "too many colons in address"},
+		// IP literal with zone identifier
+		{"[::1%lo0]:http", "::1%lo0", "http"},
+		{"[::1%lo0]:80", "::1%lo0", "80"},
 
-	{"localhost%lo0:80", "missing brackets in address"},
+		// Go-specific wildcard for host name
+		{":http", "", "http"}, // Go 1 behavior
+		{":80", "", "80"},     // Go 1 behavior
 
-	// Test cases that didn't fail in Go 1.0
+		// Go-specific wildcard for service name or transport port number
+		{"golang.org:", "golang.org", ""}, // Go 1 behavior
+		{"127.0.0.1:", "127.0.0.1", ""},   // Go 1 behavior
+		{"[::1]:", "::1", ""},             // Go 1 behavior
 
-	{"[foo:bar]", "missing port in address"},
-	{"[foo:bar]baz", "missing port in address"},
-	{"[foo]bar:baz", "missing port in address"},
+		// Opaque service name
+		{"golang.org:https%foo", "golang.org", "https%foo"}, // Go 1 behavior
+	} {
+		if host, port, err := SplitHostPort(tt.hostPort); host != tt.host || port != tt.port || err != nil {
+			t.Errorf("SplitHostPort(%q) = %q, %q, %v; want %q, %q, nil", tt.hostPort, host, port, err, tt.host, tt.port)
+		}
+	}
 
-	{"[foo]:[bar]:baz", "too many colons in address"},
+	for _, tt := range []struct {
+		hostPort string
+		err      string
+	}{
+		{"golang.org", "missing port in address"},
+		{"127.0.0.1", "missing port in address"},
+		{"[::1]", "missing port in address"},
+		{"[fe80::1%lo0]", "missing port in address"},
+		{"[localhost%lo0]", "missing port in address"},
+		{"localhost%lo0", "missing port in address"},
 
-	{"[foo]:[bar]baz", "unexpected '[' in address"},
-	{"foo[bar]:baz", "unexpected '[' in address"},
+		{"::1", "too many colons in address"},
+		{"fe80::1%lo0", "too many colons in address"},
+		{"fe80::1%lo0:80", "too many colons in address"},
 
-	{"foo]bar:baz", "unexpected ']' in address"},
-}
+		// Test cases that didn't fail in Go 1
 
-func TestSplitHostPort(t *testing.T) {
-	for _, tt := range splitJoinTests {
-		if host, port, err := SplitHostPort(tt.join); host != tt.host || port != tt.port || err != nil {
-			t.Errorf("SplitHostPort(%q) = %q, %q, %v; want %q, %q, nil", tt.join, host, port, err, tt.host, tt.port)
-		}
-	}
-	for _, tt := range splitFailureTests {
+		{"[foo:bar]", "missing port in address"},
+		{"[foo:bar]baz", "missing port in address"},
+		{"[foo]bar:baz", "missing port in address"},
+
+		{"[foo]:[bar]:baz", "too many colons in address"},
+
+		{"[foo]:[bar]baz", "unexpected '[' in address"},
+		{"foo[bar]:baz", "unexpected '[' in address"},
+
+		{"foo]bar:baz", "unexpected ']' in address"},
+	} {
 		if host, port, err := SplitHostPort(tt.hostPort); err == nil {
 			t.Errorf("SplitHostPort(%q) should have failed", tt.hostPort)
 		} else {
@@ -539,9 +555,43 @@ func TestSplitHostPort(t *testing.T) {
 }
 
 func TestJoinHostPort(t *testing.T) {
-	for _, tt := range splitJoinTests {
-		if join := JoinHostPort(tt.host, tt.port); join != tt.join {
-			t.Errorf("JoinHostPort(%q, %q) = %q; want %q", tt.host, tt.port, join, tt.join)
+	for _, tt := range []struct {
+		host     string
+		port     string
+		hostPort string
+	}{
+		// Host name
+		{"localhost", "http", "localhost:http"},
+		{"localhost", "80", "localhost:80"},
+
+		// Go-specific host name with zone identifier
+		{"localhost%lo0", "http", "localhost%lo0:http"},
+		{"localhost%lo0", "80", "localhost%lo0:80"},
+
+		// IP literal
+		{"127.0.0.1", "http", "127.0.0.1:http"},
+		{"127.0.0.1", "80", "127.0.0.1:80"},
+		{"::1", "http", "[::1]:http"},
+		{"::1", "80", "[::1]:80"},
+
+		// IP literal with zone identifier
+		{"::1%lo0", "http", "[::1%lo0]:http"},
+		{"::1%lo0", "80", "[::1%lo0]:80"},
+
+		// Go-specific wildcard for host name
+		{"", "http", ":http"}, // Go 1 behavior
+		{"", "80", ":80"},     // Go 1 behavior
+
+		// Go-specific wildcard for service name or transport port number
+		{"golang.org", "", "golang.org:"}, // Go 1 behavior
+		{"127.0.0.1", "", "127.0.0.1:"},   // Go 1 behavior
+		{"::1", "", "[::1]:"},             // Go 1 behavior
+
+		// Opaque service name
+		{"golang.org", "https%foo", "golang.org:https%foo"}, // Go 1 behavior
+	} {
+		if hostPort := JoinHostPort(tt.host, tt.port); hostPort != tt.hostPort {
+			t.Errorf("JoinHostPort(%q, %q) = %q; want %q", tt.host, tt.port, hostPort, tt.hostPort)
 		}
 	}
 }
diff --git a/src/net/ipsock.go b/src/net/ipsock.go
index f1394a7ed8..9618c66440 100644
--- a/src/net/ipsock.go
+++ b/src/net/ipsock.go
@@ -107,10 +107,11 @@ func ipv6only(addr IPAddr) bool {
 }
 
 // SplitHostPort splits a network address of the form "host:port",
-// "[host]:port" or "[ipv6-host%zone]:port" into host or
-// ipv6-host%zone and port. A literal address or host name for IPv6
-// must be enclosed in square brackets, as in "[::1]:80",
-// "[ipv6-host]:http" or "[ipv6-host%zone]:80".
+// "host%zone:port", "[host]:port" or "[host%zone]:port" into host or
+// host%zone and port.
+//
+// A literal IPv6 address in hostport must be enclosed in square
+// brackets, as in "[::1]:80", "[::1%lo0]:80".
 func SplitHostPort(hostport string) (host, port string, err error) {
 	const (
 		missingPort   = "missing port in address"
@@ -154,9 +155,6 @@ func SplitHostPort(hostport string) (host, port string, err error) {
 		if byteIndex(host, ':') >= 0 {
 			return addrErr(hostport, tooManyColons)
 		}
-		if byteIndex(host, '%') >= 0 {
-			return addrErr(hostport, "missing brackets in address")
-		}
 	}
 	if byteIndex(hostport[j:], '[') >= 0 {
 		return addrErr(hostport, "unexpected '[' in address")
@@ -181,11 +179,12 @@ func splitHostZone(s string) (host, zone string) {
 }
 
 // JoinHostPort combines host and port into a network address of the
-// form "host:port" or, if host contains a colon or a percent sign,
-// "[host]:port".
+// form "host:port" or "host%zone:port", if host is a literal IPv6
+// address, "[host]:port" or [host%zone]:port.
 func JoinHostPort(host, port string) string {
-	// If host has colons or a percent sign, have to bracket it.
-	if byteIndex(host, ':') >= 0 || byteIndex(host, '%') >= 0 {
+	// We assume that host is a literal IPv6 address if host has
+	// colons.
+	if byteIndex(host, ':') >= 0 {
 		return "[" + host + "]:" + port
 	}
 	return host + ":" + port