diff options
author | Bryan C. Mills <bcmills@google.com> | 2020-01-07 12:03:28 -0500 |
---|---|---|
committer | Bryan C. Mills <bcmills@google.com> | 2020-01-07 20:29:45 +0000 |
commit | 367da16e91af75888e44b02932d228af7c9b0244 (patch) | |
tree | 297dfb823774c60a40b5eb122dd2a6dc2a10a41e | |
parent | b570a4147dae9efd28b48cec5b0500f599a2a99e (diff) | |
download | go-367da16e91af75888e44b02932d228af7c9b0244.tar.gz go-367da16e91af75888e44b02932d228af7c9b0244.zip |
[release-branch.go1.13] 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.
Updates #36431
Fixes #36434
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>
(cherry picked from commit 249c85d3aab2ad2d0bcbf36efe606fdd66f25c72)
Reviewed-on: https://go-review.googlesource.com/c/go/+/213657
-rw-r--r-- | src/net/http/transport.go | 7 | ||||
-rw-r--r-- | src/net/http/transport_test.go | 38 |
2 files changed, 42 insertions, 3 deletions
diff --git a/src/net/http/transport.go b/src/net/http/transport.go index b46c4e7066..903e0b51ef 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -1526,15 +1526,16 @@ func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (pconn *pers 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) - } connectReq.Write(conn) // Read response. diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index f8c13a7091..d0b12b3cb0 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -1483,6 +1483,44 @@ func TestTransportDialPreservesNetOpProxyError(t *testing.T) { } } +// 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 |