From: Sean Liao Date: Sat, 15 Nov 2025 23:58:58 +0000 (+0000) Subject: net/http: use HTTP 307 redirects in ServeMux X-Git-Tag: go1.26rc1~187 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=831af61120;p=gostls13.git net/http: use HTTP 307 redirects in ServeMux Clients receiving an HTTP 301 Moved Permanently may conservatively change the method of a POST request to GET. The newer HTTP 307 Temporary Redirect and 308 Permanent Redirect explicitly allows retrying POST requests after the redirect. These should be safe for ServeMux as this internal redirect is generated before user provided handlers are called. As ServeMux is making the redirect for the user without explicit direction, and clients may cache Permanent Redirects indefinitely, Temporary Redirect is used in case the user adds a handler for a path, that was previously redirected but no longer should. Fixes #50243 Fixes #60769 Change-Id: I6c0b735bab03bb7b50f05457b3b8a8ba813badb2 Reviewed-on: https://go-review.googlesource.com/c/go/+/720820 LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil Reviewed-by: Mark Freeman --- diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 4a16ba02af..4aa5b3a50f 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -288,7 +288,7 @@ func testHostHandlers(t *testing.T, mode testMode) { if s != vt.expected { t.Errorf("Get(%q) = %q, want %q", vt.url, s, vt.expected) } - case StatusMovedPermanently: + case StatusTemporaryRedirect: s := r.Header.Get("Location") if s != vt.expected { t.Errorf("Get(%q) = %q, want %q", vt.url, s, vt.expected) @@ -340,7 +340,7 @@ var serveMuxTests = []struct { pattern string }{ {"GET", "google.com", "/", 404, ""}, - {"GET", "google.com", "/dir", 301, "/dir/"}, + {"GET", "google.com", "/dir", 307, "/dir/"}, {"GET", "google.com", "/dir/", 200, "/dir/"}, {"GET", "google.com", "/dir/file", 200, "/dir/"}, {"GET", "google.com", "/search", 201, "/search"}, @@ -354,14 +354,14 @@ var serveMuxTests = []struct { {"GET", "images.google.com", "/search", 201, "/search"}, {"GET", "images.google.com", "/search/", 404, ""}, {"GET", "images.google.com", "/search/foo", 404, ""}, - {"GET", "google.com", "/../search", 301, "/search"}, - {"GET", "google.com", "/dir/..", 301, ""}, - {"GET", "google.com", "/dir/..", 301, ""}, - {"GET", "google.com", "/dir/./file", 301, "/dir/"}, + {"GET", "google.com", "/../search", 307, "/search"}, + {"GET", "google.com", "/dir/..", 307, ""}, + {"GET", "google.com", "/dir/..", 307, ""}, + {"GET", "google.com", "/dir/./file", 307, "/dir/"}, // The /foo -> /foo/ redirect applies to CONNECT requests // but the path canonicalization does not. - {"CONNECT", "google.com", "/dir", 301, "/dir/"}, + {"CONNECT", "google.com", "/dir", 307, "/dir/"}, {"CONNECT", "google.com", "/../search", 404, ""}, {"CONNECT", "google.com", "/dir/..", 200, "/dir/"}, {"CONNECT", "google.com", "/dir/..", 200, "/dir/"}, @@ -454,7 +454,7 @@ func TestServeMuxHandlerRedirects(t *testing.T) { h, _ := mux.Handler(r) rr := httptest.NewRecorder() h.ServeHTTP(rr, r) - if rr.Code != 301 { + if rr.Code != 307 { if rr.Code != tt.code { t.Errorf("%s %s %s = %d, want %d", tt.method, tt.host, tt.url, rr.Code, tt.code) } @@ -473,6 +473,37 @@ func TestServeMuxHandlerRedirects(t *testing.T) { } } +func TestServeMuxHandlerRedirectPost(t *testing.T) { + setParallel(t) + mux := NewServeMux() + mux.HandleFunc("POST /test/", func(w ResponseWriter, r *Request) { + w.WriteHeader(200) + }) + + var code, retries int + startURL := "http://example.com/test" + reqURL := startURL + for retries = 0; retries <= 1; retries++ { + r := httptest.NewRequest("POST", reqURL, strings.NewReader("hello world")) + h, _ := mux.Handler(r) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, r) + code = rr.Code + switch rr.Code { + case 307: + reqURL = rr.Result().Header.Get("Location") + continue + case 200: + // ok + default: + t.Errorf("unhandled response code: %v", rr.Code) + } + } + if code != 200 { + t.Errorf("POST %s = %d after %d retries, want = 200", startURL, code, retries) + } +} + // Tests for https://golang.org/issue/900 func TestMuxRedirectLeadingSlashes(t *testing.T) { setParallel(t) @@ -492,8 +523,8 @@ func TestMuxRedirectLeadingSlashes(t *testing.T) { return } - if code, expected := resp.Code, StatusMovedPermanently; code != expected { - t.Errorf("Expected response code of StatusMovedPermanently; got %d", code) + if code, expected := resp.Code, StatusTemporaryRedirect; code != expected { + t.Errorf("Expected response code of StatusPermanentRedirect; got %d", code) return } } @@ -579,18 +610,18 @@ func TestServeWithSlashRedirectForHostPatterns(t *testing.T) { want string }{ {"GET", "http://example.com/", 404, "", ""}, - {"GET", "http://example.com/pkg/foo", 301, "/pkg/foo/", ""}, + {"GET", "http://example.com/pkg/foo", 307, "/pkg/foo/", ""}, {"GET", "http://example.com/pkg/bar", 200, "", "example.com/pkg/bar"}, {"GET", "http://example.com/pkg/bar/", 200, "", "example.com/pkg/bar/"}, - {"GET", "http://example.com/pkg/baz", 301, "/pkg/baz/", ""}, - {"GET", "http://example.com:3000/pkg/foo", 301, "/pkg/foo/", ""}, + {"GET", "http://example.com/pkg/baz", 307, "/pkg/baz/", ""}, + {"GET", "http://example.com:3000/pkg/foo", 307, "/pkg/foo/", ""}, {"CONNECT", "http://example.com/", 404, "", ""}, {"CONNECT", "http://example.com:3000/", 404, "", ""}, {"CONNECT", "http://example.com:9000/", 200, "", "example.com:9000/"}, - {"CONNECT", "http://example.com/pkg/foo", 301, "/pkg/foo/", ""}, + {"CONNECT", "http://example.com/pkg/foo", 307, "/pkg/foo/", ""}, {"CONNECT", "http://example.com:3000/pkg/foo", 404, "", ""}, - {"CONNECT", "http://example.com:3000/pkg/baz", 301, "/pkg/baz/", ""}, - {"CONNECT", "http://example.com:3000/pkg/connect", 301, "/pkg/connect/", ""}, + {"CONNECT", "http://example.com:3000/pkg/baz", 307, "/pkg/baz/", ""}, + {"CONNECT", "http://example.com:3000/pkg/connect", 307, "/pkg/connect/", ""}, } for i, tt := range tests { @@ -6940,7 +6971,7 @@ func TestMuxRedirectRelative(t *testing.T) { if got, want := resp.Header().Get("Location"), "/"; got != want { t.Errorf("Location header expected %q; got %q", want, got) } - if got, want := resp.Code, StatusMovedPermanently; got != want { + if got, want := resp.Code, StatusTemporaryRedirect; got != want { t.Errorf("Expected response code %d; got %d", want, got) } } diff --git a/src/net/http/server.go b/src/net/http/server.go index 02554d1a20..1a7f751990 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -2704,7 +2704,7 @@ func (mux *ServeMux) findHandler(r *Request) (h Handler, patStr string, _ *patte // but the path canonicalization does not. _, _, u := mux.matchOrRedirect(host, r.Method, path, r.URL) if u != nil { - return RedirectHandler(u.String(), StatusMovedPermanently), u.Path, nil, nil + return RedirectHandler(u.String(), StatusTemporaryRedirect), u.Path, nil, nil } // Redo the match, this time with r.Host instead of r.URL.Host. // Pass a nil URL to skip the trailing-slash redirect logic. @@ -2720,7 +2720,7 @@ func (mux *ServeMux) findHandler(r *Request) (h Handler, patStr string, _ *patte var u *url.URL n, matches, u = mux.matchOrRedirect(host, r.Method, path, r.URL) if u != nil { - return RedirectHandler(u.String(), StatusMovedPermanently), n.pattern.String(), nil, nil + return RedirectHandler(u.String(), StatusTemporaryRedirect), n.pattern.String(), nil, nil } if path != escapedPath { // Redirect to cleaned path. @@ -2729,7 +2729,7 @@ func (mux *ServeMux) findHandler(r *Request) (h Handler, patStr string, _ *patte patStr = n.pattern.String() } u := &url.URL{Path: path, RawQuery: r.URL.RawQuery} - return RedirectHandler(u.String(), StatusMovedPermanently), patStr, nil, nil + return RedirectHandler(u.String(), StatusTemporaryRedirect), patStr, nil, nil } } if n == nil { diff --git a/src/net/http/server_test.go b/src/net/http/server_test.go index 832f9688b6..edcf362062 100644 --- a/src/net/http/server_test.go +++ b/src/net/http/server_test.go @@ -91,12 +91,12 @@ func TestFindHandler(t *testing.T) { wantHandler string }{ {"GET", "/", "&http.handler{i:1}"}, - {"GET", "//", `&http.redirectHandler{url:"/", code:301}`}, - {"GET", "/foo/../bar/./..//baz", `&http.redirectHandler{url:"/baz", code:301}`}, + {"GET", "//", `&http.redirectHandler{url:"/", code:307}`}, + {"GET", "/foo/../bar/./..//baz", `&http.redirectHandler{url:"/baz", code:307}`}, {"GET", "/foo", "&http.handler{i:3}"}, {"GET", "/foo/x", "&http.handler{i:2}"}, {"GET", "/bar/x", "&http.handler{i:4}"}, - {"GET", "/bar", `&http.redirectHandler{url:"/bar/", code:301}`}, + {"GET", "/bar", `&http.redirectHandler{url:"/bar/", code:307}`}, {"CONNECT", "", "(http.HandlerFunc)(.*)"}, {"CONNECT", "/", "&http.handler{i:1}"}, {"CONNECT", "//", "&http.handler{i:1}"}, @@ -105,7 +105,7 @@ func TestFindHandler(t *testing.T) { {"CONNECT", "/foo", "&http.handler{i:3}"}, {"CONNECT", "/foo/x", "&http.handler{i:2}"}, {"CONNECT", "/bar/x", "&http.handler{i:4}"}, - {"CONNECT", "/bar", `&http.redirectHandler{url:"/bar/", code:301}`}, + {"CONNECT", "/bar", `&http.redirectHandler{url:"/bar/", code:307}`}, } { var r Request r.Method = test.method