From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Tue, 1 Nov 2016 17:40:07 +0000 (+0000)
Subject: net/http: make Transport reject URLs with bogus ports with non-digits
X-Git-Tag: go1.8beta1~401
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d89ab1398eef9a3f08db2a49ff3cbe158001474d;p=gostls13.git

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 de666fb554..585765d789 100644
--- 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 cf01e29c85..b3052e146d 100644
--- 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) {