From 7d1b82dbf108d6cf80f959bb1558f365cee0ec0e Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Wed, 3 Jan 2024 04:18:15 +0000 Subject: [PATCH] net/http: make Request.Clone create fresh copies for matches and otherValues This change fixes Request.Clone to correctly work with SetPathValue by creating fresh copies for matches and otherValues so that SetPathValue for cloned requests doesn't pollute the original request. While here, also added a doc for Request.SetPathValue. Fixes #64911 Change-Id: I2831b38e135935dfaea2b939bb9db554c75b65ef GitHub-Last-Rev: 1981db16475a49fe8d4b874a6bceec64d28a1332 GitHub-Pull-Request: golang/go#64913 Reviewed-on: https://go-review.googlesource.com/c/go/+/553375 Reviewed-by: Emmanuel Odeke Run-TryBot: Jes Cok Auto-Submit: Dmitri Shuralyov Reviewed-by: Jonathan Amsterdam TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov --- src/net/http/request.go | 16 ++++++++++++++++ src/net/http/request_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/src/net/http/request.go b/src/net/http/request.go index ed2cdac136..fce2d16f95 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -397,6 +397,20 @@ func (r *Request) Clone(ctx context.Context) *Request { r2.Form = cloneURLValues(r.Form) r2.PostForm = cloneURLValues(r.PostForm) r2.MultipartForm = cloneMultipartForm(r.MultipartForm) + + // Copy matches and otherValues. See issue 61410. + if s := r.matches; s != nil { + s2 := make([]string, len(s)) + copy(s2, s) + r2.matches = s2 + } + if s := r.otherValues; s != nil { + s2 := make(map[string]string, len(s)) + for k, v := range s { + s2[k] = v + } + r2.otherValues = s2 + } return r2 } @@ -1427,6 +1441,8 @@ func (r *Request) PathValue(name string) string { return r.otherValues[name] } +// SetPathValue sets name to value, so that subsequent calls to r.PathValue(name) +// return value. func (r *Request) SetPathValue(name, value string) { if i := r.patIndex(name); i >= 0 { r.matches[i] = value diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 1531da3d8c..6ce32332e7 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -1053,6 +1053,33 @@ func TestRequestCloneTransferEncoding(t *testing.T) { } } +// Ensure that Request.Clone works correctly with PathValue. +// See issue 64911. +func TestRequestClonePathValue(t *testing.T) { + req, _ := http.NewRequest("GET", "https://example.org/", nil) + req.SetPathValue("p1", "orig") + + clonedReq := req.Clone(context.Background()) + clonedReq.SetPathValue("p2", "copy") + + // Ensure that any modifications to the cloned + // request do not pollute the original request. + if g, w := req.PathValue("p2"), ""; g != w { + t.Fatalf("p2 mismatch got %q, want %q", g, w) + } + if g, w := req.PathValue("p1"), "orig"; g != w { + t.Fatalf("p1 mismatch got %q, want %q", g, w) + } + + // Assert on the changes to the cloned request. + if g, w := clonedReq.PathValue("p1"), "orig"; g != w { + t.Fatalf("p1 mismatch got %q, want %q", g, w) + } + if g, w := clonedReq.PathValue("p2"), "copy"; g != w { + t.Fatalf("p2 mismatch got %q, want %q", g, w) + } +} + // Issue 34878: verify we don't panic when including basic auth (Go 1.13 regression) func TestNoPanicOnRoundTripWithBasicAuth(t *testing.T) { run(t, testNoPanicWithBasicAuth) } func testNoPanicWithBasicAuth(t *testing.T, mode testMode) { -- 2.48.1