diff --git a/config/headers_test.go b/config/headers_test.go index daac9808..f2c3ad2b 100644 --- a/config/headers_test.go +++ b/config/headers_test.go @@ -69,6 +69,35 @@ func TestHeadersRoundTripperSameHost(t *testing.T) { } } +func TestHeadersRoundTripperReusedRequest(t *testing.T) { + // The round tripper must not mutate the request it is given: reusing the + // same request must not accumulate another copy of every header. + var received []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + received = r.Header.Values("X-Custom-Header") + fmt.Fprint(w, "ok") + })) + t.Cleanup(server.Close) + + headers := &Headers{ + Headers: map[string]Header{ + "X-Custom-Header": {Values: []string{"testvalue"}}, + }, + } + rt := NewHeadersRoundTripper(headers, http.DefaultTransport) + + req, err := http.NewRequest(http.MethodGet, server.URL, nil) + require.NoError(t, err) + + for i := range 3 { + resp, err := rt.RoundTrip(req) + require.NoError(t, err) + resp.Body.Close() + require.Equalf(t, []string{"testvalue"}, received, "header duplicated on request %d", i+1) + require.Emptyf(t, req.Header.Values("X-Custom-Header"), "the caller's request was modified") + } +} + func TestHeadersRoundTripperCrossHostRedirect(t *testing.T) { // Cookie must be set on the initial request but stripped on cross-host redirects. cookieOnRedirect := "" diff --git a/config/http_config.go b/config/http_config.go index d633479c..35a90fca 100644 --- a/config/http_config.go +++ b/config/http_config.go @@ -22,7 +22,6 @@ import ( "encoding/json" "errors" "fmt" - "maps" "net" "net/http" "net/url" @@ -1240,8 +1239,13 @@ func cloneRequest(r *http.Request) *http.Request { // Shallow copy of the struct. r2 := new(http.Request) *r2 = *r - // Deep copy of the Header. - maps.Copy(r.Header, r2.Header) + // Deep copy of the Header. The shallow copy above leaves r2.Header + // aliasing r.Header, so without this every round tripper that adds a + // header would mutate the caller's request. + r2.Header = r.Header.Clone() + if r2.Header == nil { + r2.Header = make(http.Header) + } return r2 } diff --git a/config/http_config_test.go b/config/http_config_test.go index 4d61ae12..9b3de4df 100644 --- a/config/http_config_test.go +++ b/config/http_config_test.go @@ -2749,3 +2749,27 @@ func TestLoadHTTPConfigFileResolvesPathsRelativeToConfigFile(t *testing.T) { _, err = client.Get(ts.URL) require.NoErrorf(t, err, "can't fetch URL: %v", err) } + +func TestCloneRequest(t *testing.T) { + t.Run("clone does not share the caller's header", func(t *testing.T) { + r, err := http.NewRequest(http.MethodGet, "http://example.com", nil) + require.NoError(t, err) + r.Header.Set("X-Original", "value") + + r2 := cloneRequest(r) + r2.Header.Add("X-Added", "value") + + require.Equalf(t, "value", r2.Header.Get("X-Original"), "existing headers must be carried over") + require.Emptyf(t, r.Header.Values("X-Added"), "the original request must not be modified") + }) + + t.Run("clone of a request without a header is usable", func(t *testing.T) { + r, err := http.NewRequest(http.MethodGet, "http://example.com", nil) + require.NoError(t, err) + r.Header = nil + + r2 := cloneRequest(r) + require.NotPanics(t, func() { r2.Header.Set("X-Added", "value") }) + require.Nilf(t, r.Header, "the original request must not be modified") + }) +}