net/http: make Transport reject URLs with bogus ports with non-digits

Fixes #14353

Change-Id: I9cb5a5192ecdae37c100969395ed6a1564b8d34e
Reviewed-on: https://go-review.googlesource.com/32482
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Martin Möhrmann <martisch@uos.de>

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index de666fb55480bd54915949305e462c215ffce92f..585765d7894cea24d5e5fe639ee8eba99d114465 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -567,10 +567,18 @@ func (e *envOnce) reset() {
 }
 
 func (t *Transport) connectMethodForRequest(treq *transportRequest) (cm connectMethod, err error) {
+       if port := treq.URL.Port(); !validPort(port) {
+               return cm, fmt.Errorf("invalid URL port %q", port)
+       }
        cm.targetScheme = treq.URL.Scheme
        cm.targetAddr = canonicalAddr(treq.URL)
        if t.Proxy != nil {
                cm.proxyURL, err = t.Proxy(treq.Request)
+               if err == nil && cm.proxyURL != nil {
+                       if port := cm.proxyURL.Port(); !validPort(port) {
+                               return cm, fmt.Errorf("invalid proxy URL port %q", port)
+                       }
+               }
        }
        return cm, err
 }
@@ -2156,3 +2164,15 @@ func (cl *connLRU) remove(pc *persistConn) {
 func (cl *connLRU) len() int {
        return len(cl.m)
 }
+
+// validPort reports whether p (without the colon) is a valid port in
+// a URL, per RFC 3986 Section 3.2.3, which says the port may be
+// empty, or only contain digits.
+func validPort(p string) bool {
+       for _, r := range []byte(p) {
+               if r < '0' || r > '9' {
+                       return false
+               }
+       }
+       return true
+}

diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index cf01e29c850bd81da5ec91f9d697a8aac71d1f1c..b3052e146de52a6dc52f625f4c6dbf8f3a683305 100644 (file)
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -3446,6 +3446,24 @@ func TestTransportEventTraceRealDNS(t *testing.T) {
        }
 }
 
+// Issue 14353: port can only contain digits.
+func TestTransportRejectsAlphaPort(t *testing.T) {
+       res, err := Get("http://dummy.tld:123foo/bar")
+       if err == nil {
+               res.Body.Close()
+               t.Fatal("unexpected sucess")
+       }
+       ue, ok := err.(*url.Error)
+       if !ok {
+               t.Fatalf("got %#v; want *url.Error", err)
+       }
+       got := ue.Err.Error()
+       want := `invalid URL port "123foo"`
+       if got != want {
+               t.Errorf("got error %q; want %q", got, want)
+       }
+}
+
 // Test the httptrace.TLSHandshake{Start,Done} hooks with a https http1
 // connections. The http2 test is done in TestTransportEventTrace_h2
 func TestTLSHandshakeTrace(t *testing.T) {