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 <emmanuel@orijtech.com>
Run-TryBot: Jes Cok <xigua67damn@gmail.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

diff --git a/src/net/http/request.go b/src/net/http/request.go
index ed2cdac136d03b7461d18193fb9ef529bbf50dcd..fce2d16f956424cca3a7958e37262898e7abec0c 100644 (file)
--- 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 1531da3d8ca60adc0c9fe469064106c83a83a101..6ce32332e757e1260e966120a157c65eb7360451 100644 (file)
--- 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) {