Stowage/go
2
0
Fork 0
mirror of https://github.com/golang/go.git synced 2025-12-08 06:10:04 +00:00
Code Issues Projects Releases Packages Wiki Activity

net/http: use HTTP 307 redirects in ServeMux

Browse source 
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>
This commit is contained in:
Sean Liao 2025-11-15 23:58:58 +00:00
parent 87269224cb
commit 831af61120
3 changed files with 55 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

65
src/net/http/serve_test.go
View file

@ -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)
}
}

6
src/net/http/server.go
View file

@ -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 {

8
src/net/http/server_test.go
View file

@ -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