net/url: restrict :port checking to [ipv6]:port form

Go 1.4 and earlier accepted mysql://x@y(z:123)/foo
and I don't see any compelling reason to break that.

The CL during Go 1.5 that broke this syntax was
trying to fix golang#11208 and was probably too aggressive.
I added a test case for golang#11208 to make sure that stays
fixed.

Relaxing the check did not re-break golang#11208 nor did
it cause any existing test to fail. I added a test for the
mysql://x@y(z:123)/foo syntax being preserved.

Fixes golang#12023.

Change-Id: I659d39f18c85111697732ad24b757169d69284fc
Reviewed-on: https://go-review.googlesource.com/13253
Reviewed-by: Andrew Gerrand <[email protected]>
Reviewed-by: Mikio Hara <[email protected]>

src/net/url/url.go

@@ -478,7 +478,6 @@ func parseAuthority(authority string) (user *Userinfo, host string, err error) {
 // information. That is, as host[:port].
 func parseHost(host string) (string, error) {
 	litOrName := host
-	var colonPort string // ":80" or ""
 	if strings.HasPrefix(host, "[") {
 		// Parse an IP-Literal in RFC 3986 and RFC 6874.
 		// E.g., "[fe80::1], "[fe80::1%25en0]"
@@ -490,7 +489,10 @@ func parseHost(host string) (string, error) {
 		if i < 0 {
 			return "", errors.New("missing ']' in host")
 		}
-		colonPort = host[i+1:]
+		colonPort := host[i+1:]
+		if !validOptionalPort(colonPort) {
+			return "", fmt.Errorf("invalid port %q after host", colonPort)
+		}
 		// Parse a host subcomponent without a ZoneID in RFC
 		// 6874 because the ZoneID is allowed to use the
 		// percent encoded form.
@@ -500,11 +502,8 @@ func parseHost(host string) (string, error) {
 		} else {
 			litOrName = host[1:j]
 		}
-	} else {
-		if i := strings.Index(host, ":"); i != -1 {
-			colonPort = host[i:]
-		}
 	}
+
 	// A URI containing an IP-Literal without a ZoneID or
 	// IPv4address in RFC 3986 and RFC 6847 must not be
 	// percent-encoded.
@@ -517,9 +516,6 @@ func parseHost(host string) (string, error) {
 	if strings.Contains(litOrName, "%") {
 		return "", errors.New("percent-encoded characters in host")
 	}
-	if !validOptionalPort(colonPort) {
-		return "", fmt.Errorf("invalid port %q after host", colonPort)
-	}
 	var err error
 	if host, err = unescape(host, encodeHost); err != nil {
 		return "", err

src/net/url/url_test.go

@@ -1081,13 +1081,17 @@ func TestParseAuthority(t *testing.T) {
 		{"http://[::1]/", false},
 		{"http://[::1]a", true},
 		{"http://[::1]%23", true},
-		{"http://[::1%25en0]", false}, // valid zone id
-		{"http://[::1]:", true},       // colon, but no port
-		{"http://[::1]:%38%30", true}, // no hex in port
-		{"http://[::1%25%10]", false}, // TODO: reject the %10 after the valid zone %25 separator?
-		{"http://[%10::1]", true},     // no %xx escapes in IP address
-		{"http://[::1]/%48", false},   // %xx in path is fine
-		{"http://%41:8080/", true},    // TODO: arguably we should accept reg-name with %xx
+		{"http://[::1%25en0]", false},     // valid zone id
+		{"http://[::1]:", true},           // colon, but no port
+		{"http://[::1]:%38%30", true},     // no hex in port
+		{"http://[::1%25%10]", false},     // TODO: reject the %10 after the valid zone %25 separator?
+		{"http://[%10::1]", true},         // no %xx escapes in IP address
+		{"http://[::1]/%48", false},       // %xx in path is fine
+		{"http://%41:8080/", true},        // TODO: arguably we should accept reg-name with %xx
+		{"mysql://x@y(z:123)/foo", false}, // golang.org/issue/12023
+		{"mysql://x@y(1.2.3.4:123)/foo", false},
+		{"mysql://x@y([2001:db8::1]:123)/foo", false},
+		{"http://[]%20%48%54%54%50%2f%31%2e%31%0a%4d%79%48%65%61%64%65%72%3a%20%31%32%33%0a%0a/", true}, // golang.org/issue/11208
 	}
 	for _, tt := range tests {
 		u, err := Parse(tt.in)