net/http: avoid writing to Transport.ProxyConnectHeader
Previously, we accidentally wrote the Proxy-Authorization header for
the initial CONNECT request to the shared ProxyConnectHeader map when
it was non-nil.
Fixes #36431
Change-Id: I5cb414f391dddf8c23d85427eb6973f14c949025
Reviewed-on: https://go-review.googlesource.com/c/go/+/213638
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 64d8510..fa2303c 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -1559,15 +1559,16 @@
if hdr == nil {
hdr = make(Header)
}
+ if pa := cm.proxyAuth(); pa != "" {
+ hdr = hdr.Clone()
+ hdr.Set("Proxy-Authorization", pa)
+ }
connectReq := &Request{
Method: "CONNECT",
URL: &url.URL{Opaque: cm.targetAddr},
Host: cm.targetAddr,
Header: hdr,
}
- if pa := cm.proxyAuth(); pa != "" {
- connectReq.Header.Set("Proxy-Authorization", pa)
- }
// If there's no done channel (no deadline or cancellation
// from the caller possible), at least set some (long)
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 08ee4ab..5fc60e1 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -1550,6 +1550,44 @@
}
}
+// Issue 36431: calls to RoundTrip should not mutate t.ProxyConnectHeader.
+//
+// (A bug caused dialConn to instead write the per-request Proxy-Authorization
+// header through to the shared Header instance, introducing a data race.)
+func TestTransportProxyDialDoesNotMutateProxyConnectHeader(t *testing.T) {
+ setParallel(t)
+ defer afterTest(t)
+
+ proxy := httptest.NewTLSServer(NotFoundHandler())
+ defer proxy.Close()
+ c := proxy.Client()
+
+ tr := c.Transport.(*Transport)
+ tr.Proxy = func(*Request) (*url.URL, error) {
+ u, _ := url.Parse(proxy.URL)
+ u.User = url.UserPassword("aladdin", "opensesame")
+ return u, nil
+ }
+ h := tr.ProxyConnectHeader
+ if h == nil {
+ h = make(Header)
+ }
+ tr.ProxyConnectHeader = h.Clone()
+
+ req, err := NewRequest("GET", "https://golang.fake.tld/", nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ _, err = c.Do(req)
+ if err == nil {
+ t.Errorf("unexpected Get success")
+ }
+
+ if !reflect.DeepEqual(tr.ProxyConnectHeader, h) {
+ t.Errorf("tr.ProxyConnectHeader = %v; want %v", tr.ProxyConnectHeader, h)
+ }
+}
+
// TestTransportGzipRecursive sends a gzip quine and checks that the
// client gets the same value back. This is more cute than anything,
// but checks that we don't recurse forever, and checks that
