diff options
| -rw-r--r-- | src/net/http/transport.go | 2 | ||||
| -rw-r--r-- | src/net/http/transport_test.go | 2 | ||||
| -rw-r--r-- | src/net/url/url.go | 54 | ||||
| -rw-r--r-- | src/net/url/url_test.go | 76 |
4 files changed, 69 insertions, 65 deletions
diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 07920cfde3..e946760963 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -655,6 +655,8 @@ func resetProxyConfig() { } func (t *Transport) connectMethodForRequest(treq *transportRequest) (cm connectMethod, err error) { + // TODO: the validPort check is redundant after CL 189258, as url.URL.Port + // only returns valid ports now. golang.org/issue/33600 if port := treq.URL.Port(); !validPort(port) { return cm, fmt.Errorf("invalid URL port %q", port) } diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index f66e72a00f..5c329543e2 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -4163,7 +4163,7 @@ func TestTransportRejectsAlphaPort(t *testing.T) { t.Fatalf("got %#v; want *url.Error", err) } got := ue.Err.Error() - want := `invalid URL port "123foo"` + want := `invalid port ":123foo" after host` if got != want { t.Errorf("got error %q; want %q", got, want) } diff --git a/src/net/url/url.go b/src/net/url/url.go index 64274a0a36..337861f80d 100644 --- a/src/net/url/url.go +++ b/src/net/url/url.go @@ -655,6 +655,11 @@ func parseHost(host string) (string, error) { } return host1 + host2 + host3, nil } + } else if i := strings.LastIndex(host, ":"); i != -1 { + colonPort := host[i:] + if !validOptionalPort(colonPort) { + return "", fmt.Errorf("invalid port %q after host", colonPort) + } } var err error @@ -1053,44 +1058,39 @@ func (u *URL) RequestURI() string { return result } -// Hostname returns u.Host, without any port number. +// Hostname returns u.Host, stripping any valid port number if present. // -// If Host is an IPv6 literal with a port number, Hostname returns the -// IPv6 literal without the square brackets. IPv6 literals may include -// a zone identifier. +// If the result is enclosed in square brackets, as literal IPv6 addresses are, +// the square brackets are removed from the result. func (u *URL) Hostname() string { - return stripPort(u.Host) + host, _ := splitHostPort(u.Host) + return host } // Port returns the port part of u.Host, without the leading colon. -// If u.Host doesn't contain a port, Port returns an empty string. +// +// If u.Host doesn't contain a valid numeric port, Port returns an empty string. func (u *URL) Port() string { - return portOnly(u.Host) + _, port := splitHostPort(u.Host) + return port } -func stripPort(hostport string) string { - colon := strings.IndexByte(hostport, ':') - if colon == -1 { - return hostport - } - if i := strings.IndexByte(hostport, ']'); i != -1 { - return strings.TrimPrefix(hostport[:i], "[") - } - return hostport[:colon] -} +// splitHostPort separates host and port. If the port is not valid, it returns +// the entire input as host, and it doesn't check the validity of the host. +// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric. +func splitHostPort(hostport string) (host, port string) { + host = hostport -func portOnly(hostport string) string { - colon := strings.IndexByte(hostport, ':') - if colon == -1 { - return "" - } - if i := strings.Index(hostport, "]:"); i != -1 { - return hostport[i+len("]:"):] + colon := strings.LastIndexByte(host, ':') + if colon != -1 && validOptionalPort(host[colon:]) { + host, port = host[:colon], host[colon+1:] } - if strings.Contains(hostport, "]") { - return "" + + if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { + host = host[1 : len(host)-1] } - return hostport[colon+len(":"):] + + return } // Marshaling interface implementations. diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go index c5fc90d515..b6f4623a52 100644 --- a/src/net/url/url_test.go +++ b/src/net/url/url_test.go @@ -422,10 +422,10 @@ var urltests = []URLTest{ }, // worst case host, still round trips { - "scheme://!$&'()*+,;=hello!:port/path", + "scheme://!$&'()*+,;=hello!:1/path", &URL{ Scheme: "scheme", - Host: "!$&'()*+,;=hello!:port", + Host: "!$&'()*+,;=hello!:1", Path: "/path", }, "", @@ -1420,11 +1420,13 @@ func TestParseErrors(t *testing.T) { {"http://[::1]", false}, {"http://[::1]:80", false}, {"http://[::1]:namedport", true}, // rfc3986 3.2.3 + {"http://x:namedport", true}, // rfc3986 3.2.3 {"http://[::1]/", false}, {"http://[::1]a", true}, {"http://[::1]%23", true}, {"http://[::1%25en0]", false}, // valid zone id {"http://[::1]:", false}, // colon, but no port OK + {"http://x:", false}, // colon, but no port OK {"http://[::1]:%38%30", true}, // not allowed: % encoding only for non-ASCII {"http://[::1%25%41]", false}, // RFC 6874 allows over-escaping in zone {"http://[%10::1]", true}, // no %xx escapes in IP address @@ -1616,46 +1618,46 @@ func TestURLErrorImplementsNetError(t *testing.T) { } } -func TestURLHostname(t *testing.T) { +func TestURLHostnameAndPort(t *testing.T) { tests := []struct { - host string // URL.Host field - want string + in string // URL.Host field + host string + port string }{ - {"foo.com:80", "foo.com"}, - {"foo.com", "foo.com"}, - {"FOO.COM", "FOO.COM"}, // no canonicalization (yet?) - {"1.2.3.4", "1.2.3.4"}, - {"1.2.3.4:80", "1.2.3.4"}, - {"[1:2:3:4]", "1:2:3:4"}, - {"[1:2:3:4]:80", "1:2:3:4"}, - {"[::1]:80", "::1"}, + {"foo.com:80", "foo.com", "80"}, + {"foo.com", "foo.com", ""}, + {"foo.com:", "foo.com", ""}, + {"FOO.COM", "FOO.COM", ""}, // no canonicalization + {"1.2.3.4", "1.2.3.4", ""}, + {"1.2.3.4:80", "1.2.3.4", "80"}, + {"[1:2:3:4]", "1:2:3:4", ""}, + {"[1:2:3:4]:80", "1:2:3:4", "80"}, + {"[::1]:80", "::1", "80"}, + {"[::1]", "::1", ""}, + {"[::1]:", "::1", ""}, + {"localhost", "localhost", ""}, + {"localhost:443", "localhost", "443"}, + {"some.super.long.domain.example.org:8080", "some.super.long.domain.example.org", "8080"}, + {"[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:17000", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "17000"}, + {"[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", ""}, + + // Ensure that even when not valid, Host is one of "Hostname", + // "Hostname:Port", "[Hostname]" or "[Hostname]:Port". + // See https://golang.org/issue/29098. + {"[google.com]:80", "google.com", "80"}, + {"google.com]:80", "google.com]", "80"}, + {"google.com:80_invalid_port", "google.com:80_invalid_port", ""}, + {"[::1]extra]:80", "::1]extra", "80"}, + {"google.com]extra:extra", "google.com]extra:extra", ""}, } for _, tt := range tests { - u := &URL{Host: tt.host} - got := u.Hostname() - if got != tt.want { - t.Errorf("Hostname for Host %q = %q; want %q", tt.host, got, tt.want) + u := &URL{Host: tt.in} + host, port := u.Hostname(), u.Port() + if host != tt.host { + t.Errorf("Hostname for Host %q = %q; want %q", tt.in, host, tt.host) } - } -} - -func TestURLPort(t *testing.T) { - tests := []struct { - host string // URL.Host field - want string - }{ - {"foo.com", ""}, - {"foo.com:80", "80"}, - {"1.2.3.4", ""}, - {"1.2.3.4:80", "80"}, - {"[1:2:3:4]", ""}, - {"[1:2:3:4]:80", "80"}, - } - for _, tt := range tests { - u := &URL{Host: tt.host} - got := u.Port() - if got != tt.want { - t.Errorf("Port for Host %q = %q; want %q", tt.host, got, tt.want) + if port != tt.port { + t.Errorf("Port for Host %q = %q; want %q", tt.in, port, tt.port) } } } |