net/http: refine trailing-slash redirect logic

Do not add a trailing slash and redirect if the path already
ends in a slash.

Also, and unrelatedly, add a test for cleanPath.

Fixes #65624.

Change-Id: Ifcf9edc929d2eb6db88132c09d2bade85c5dda3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/562557
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index f633bf07995ca6d1065e25c0accbb8f8505e6506..301a9fdc4bf2c5f13f816bb5f89b585a639a3d6d 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -597,6 +597,22 @@ func TestServeWithSlashRedirectForHostPatterns(t *testing.T) {
        }
 }
 
+// Test that we don't attempt trailing-slash redirect on a path that already has
+// a trailing slash.
+// See issue #65624.
+func TestMuxNoSlashRedirectWithTrailingSlash(t *testing.T) {
+       mux := NewServeMux()
+       mux.HandleFunc("/{x}/", func(w ResponseWriter, r *Request) {
+               fmt.Fprintln(w, "ok")
+       })
+       w := httptest.NewRecorder()
+       req, _ := NewRequest("GET", "/", nil)
+       mux.ServeHTTP(w, req)
+       if g, w := w.Code, 404; g != w {
+               t.Errorf("got %d, want %d", g, w)
+       }
+}
+
 func TestShouldRedirectConcurrency(t *testing.T) { run(t, testShouldRedirectConcurrency) }
 func testShouldRedirectConcurrency(t *testing.T, mode testMode) {
        mux := NewServeMux()

diff --git a/src/net/http/server.go b/src/net/http/server.go
index d42fdc6322574c8f5e94d1218d22d743be67e263..0ba88d1119e4f97aa9cbe6dff3ee6640388887e2 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -2577,8 +2577,8 @@ func (mux *ServeMux) matchOrRedirect(host, method, path string, u *url.URL) (_ *
 
        n, matches := mux.tree.match(host, method, path)
        // If we have an exact match, or we were asked not to try trailing-slash redirection,
-       // then we're done.
-       if !exactMatch(n, path) && u != nil {
+       // or the URL already has a trailing slash, then we're done.
+       if !exactMatch(n, path) && u != nil && !strings.HasSuffix(path, "/") {
                // If there is an exact match with a trailing slash, then redirect.
                path += "/"
                n2, _ := mux.tree.match(host, method, path)

diff --git a/src/net/http/server_test.go b/src/net/http/server_test.go
index e81e3bb6b00f9b9c80bdbba40786351bba02c925..f4aafc853bd5d6647d320da600de1f6c2c8522ad 100644 (file)
--- a/src/net/http/server_test.go
+++ b/src/net/http/server_test.go
@@ -250,6 +250,24 @@ func TestEscapedPathsAndPatterns(t *testing.T) {
        t.Run("1.21", func(t *testing.T) { run(t, true) })
 }
 
+func TestCleanPath(t *testing.T) {
+       for _, test := range []struct {
+               in, want string
+       }{
+               {"//", "/"},
+               {"/x", "/x"},
+               {"//x", "/x"},
+               {"x//", "/x/"},
+               {"a//b/////c", "/a/b/c"},
+               {"/foo/../bar/./..//baz", "/baz"},
+       } {
+               got := cleanPath(test.in)
+               if got != test.want {
+                       t.Errorf("%s: got %q, want %q", test.in, got, test.want)
+               }
+       }
+}
+
 func BenchmarkServerMatch(b *testing.B) {
        fn := func(w ResponseWriter, r *Request) {
                fmt.Fprintf(w, "OK")