From 1ca0941d09b5a61dedf7ef2985e199a7f7db1ace Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Mon, 1 Jan 2024 09:01:58 +0800 Subject: [PATCH 1/5] net/http: fix Request.Clone with SetPathValue Fixes #64911 --- src/net/http/request.go | 14 ++++++++++++++ src/net/http/request_test.go | 15 +++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/net/http/request.go b/src/net/http/request.go index ed2cdac136d03..169a5b5c033ba 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) + + // See issue 64911. + if s := r.matches; s != nil { + s2 := make([]string, len(s)) + copy(s2, s) + r2.matches = s2 + } + if s := r2.otherValues; s != nil { + s2 := make(map[string]string, len(s)) + for k, v := range s { + s2[k] = v + } + r2.otherValues = s2 + } return r2 } diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 1531da3d8ca60..7ebf8a3bc94c5 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -1053,6 +1053,21 @@ func TestRequestCloneTransferEncoding(t *testing.T) { } } +func TestRequestClonePathValues(t *testing.T) { + orig, err := http.NewRequest("GET", "https://example.com/", nil) + if err != nil { + panic(err) + } + orig.SetPathValue("orig", "orig") + + copy := orig.Clone(context.Background()) + copy.SetPathValue("copy", "copy") + + if orig.PathValue("copy") != "" { + t.Fatal("orig.PathValue is changed") + } +} + // 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) { From fbb3f76e30506b3a43a25a8278e94c925b547f56 Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Mon, 1 Jan 2024 09:19:12 +0800 Subject: [PATCH 2/5] re --- src/net/http/pprof/testdata/delta_mutex.go | 2 +- src/net/http/request.go | 6 ++++-- src/net/http/request_test.go | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/net/http/pprof/testdata/delta_mutex.go b/src/net/http/pprof/testdata/delta_mutex.go index 634069c8a0ddc..2b0374197c7e1 100644 --- a/src/net/http/pprof/testdata/delta_mutex.go +++ b/src/net/http/pprof/testdata/delta_mutex.go @@ -17,8 +17,8 @@ import ( "fmt" "log" "net/http" - "net/http/pprof" "net/http/httptest" + "net/http/pprof" "runtime" ) diff --git a/src/net/http/request.go b/src/net/http/request.go index 169a5b5c033ba..fce2d16f95642 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -398,13 +398,13 @@ func (r *Request) Clone(ctx context.Context) *Request { r2.PostForm = cloneURLValues(r.PostForm) r2.MultipartForm = cloneMultipartForm(r.MultipartForm) - // See issue 64911. + // 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 := r2.otherValues; s != nil { + if s := r.otherValues; s != nil { s2 := make(map[string]string, len(s)) for k, v := range s { s2[k] = v @@ -1441,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 7ebf8a3bc94c5..7bfb836eca1f5 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -1053,7 +1053,7 @@ func TestRequestCloneTransferEncoding(t *testing.T) { } } -func TestRequestClonePathValues(t *testing.T) { +func TestRequestClonePathValue(t *testing.T) { orig, err := http.NewRequest("GET", "https://example.com/", nil) if err != nil { panic(err) From a67316079cf3a90e6444eabbdecba81edf240110 Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Mon, 1 Jan 2024 09:45:53 +0800 Subject: [PATCH 3/5] re --- src/net/http/request_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 7bfb836eca1f5..7927ef9d5b687 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -1054,17 +1054,23 @@ func TestRequestCloneTransferEncoding(t *testing.T) { } func TestRequestClonePathValue(t *testing.T) { - orig, err := http.NewRequest("GET", "https://example.com/", nil) - if err != nil { - panic(err) - } + orig, _ := http.NewRequest("GET", "https://example.org/", nil) orig.SetPathValue("orig", "orig") copy := orig.Clone(context.Background()) copy.SetPathValue("copy", "copy") - if orig.PathValue("copy") != "" { - t.Fatal("orig.PathValue is changed") + if orig.PathValue("copy") != "" { // should not be changed + t.Fatal(`orig.PathValue("copy") != ""`) + } + if orig.PathValue("orig") != "orig" { + t.Fatal(`orig.PathValue("orig") != "orig"`) + } + if copy.PathValue("orig") != "orig" { + t.Fatal(`copy.PathValue("orig") != "orig"`) + } + if copy.PathValue("copy") != "copy" { + t.Fatal(`copy.PathValue("copy") != "copy"`) } } From d177cf8d34f9dba7eaa39cd9e572ff5617c11661 Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Mon, 1 Jan 2024 12:32:29 +0800 Subject: [PATCH 4/5] re --- src/net/http/pprof/testdata/delta_mutex.go | 2 +- src/net/http/request_test.go | 30 +++++++++++++--------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/net/http/pprof/testdata/delta_mutex.go b/src/net/http/pprof/testdata/delta_mutex.go index 2b0374197c7e1..634069c8a0ddc 100644 --- a/src/net/http/pprof/testdata/delta_mutex.go +++ b/src/net/http/pprof/testdata/delta_mutex.go @@ -17,8 +17,8 @@ import ( "fmt" "log" "net/http" - "net/http/httptest" "net/http/pprof" + "net/http/httptest" "runtime" ) diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 7927ef9d5b687..3637c54910e20 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -1053,24 +1053,30 @@ func TestRequestCloneTransferEncoding(t *testing.T) { } } +// Ensure that Request.Clone works correctly with PathValue. +// See issue #64911. func TestRequestClonePathValue(t *testing.T) { - orig, _ := http.NewRequest("GET", "https://example.org/", nil) - orig.SetPathValue("orig", "orig") + req, _ := http.NewRequest("GET", "https://example.org/", nil) + req.SetPathValue("p1", "orig") - copy := orig.Clone(context.Background()) - copy.SetPathValue("copy", "copy") + clonedReq := req.Clone(context.Background()) + clonedReq.SetPathValue("p2", "copy") - if orig.PathValue("copy") != "" { // should not be changed - t.Fatal(`orig.PathValue("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 orig.PathValue("orig") != "orig" { - t.Fatal(`orig.PathValue("orig") != "orig"`) + if g, w := req.PathValue("p1"), "orig"; g != w { + t.Fatalf("p1 mismatch got %q, want %q", g, w) } - if copy.PathValue("orig") != "orig" { - t.Fatal(`copy.PathValue("orig") != "orig"`) + + // 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 copy.PathValue("copy") != "copy" { - t.Fatal(`copy.PathValue("copy") != "copy"`) + if g, w := clonedReq.PathValue("p2"), "copy"; g != w { + t.Fatalf("p2 mismatch got %q, want %q", g, w) } } From 1981db16475a49fe8d4b874a6bceec64d28a1332 Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Wed, 3 Jan 2024 12:11:57 +0800 Subject: [PATCH 5/5] remove # --- src/net/http/request_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 3637c54910e20..6ce32332e757e 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -1054,7 +1054,7 @@ func TestRequestCloneTransferEncoding(t *testing.T) { } // Ensure that Request.Clone works correctly with PathValue. -// See issue #64911. +// See issue 64911. func TestRequestClonePathValue(t *testing.T) { req, _ := http.NewRequest("GET", "https://example.org/", nil) req.SetPathValue("p1", "orig")