net/http: avoid appending an existing trailing slash to path again

This CL is similar to CL 562557, and it takes over CL 594175.

While here, unrelatedly remove mapKeys function, use slices.Sorted(maps.Keys(ms))
to simplify code.

Fixes #67657

Change-Id: Id8b99216f87a6dcfd6d5fa61407b515324c79112
Reviewed-on: https://go-review.googlesource.com/c/go/+/594737
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Joedian Reid <joedian@google.com>

diff --git a/src/net/http/routing_tree_test.go b/src/net/http/routing_tree_test.go
index 7de6b19507230ee4fa416eac077a666adfd24d29..f3f216357d91b6125581f4b5e2ef86d977f2171d 100644 (file)
--- a/src/net/http/routing_tree_test.go
+++ b/src/net/http/routing_tree_test.go
@@ -7,6 +7,7 @@ package http
 import (
        "fmt"
        "io"
+       "maps"
        "strings"
        "testing"
 
@@ -261,9 +262,7 @@ func TestMatchingMethods(t *testing.T) {
                t.Run(test.name, func(t *testing.T) {
                        ms := map[string]bool{}
                        test.tree.matchingMethods(test.host, test.path, ms)
-                       keys := mapKeys(ms)
-                       slices.Sort(keys)
-                       got := strings.Join(keys, ",")
+                       got := strings.Join(slices.Sorted(maps.Keys(ms)), ",")
                        if got != test.want {
                                t.Errorf("got %s, want %s", got, test.want)
                        }

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 3ec10c2f14da56d0ac2c3cfb96db944919658373..b2858ba8f2b55956331e2f66c6552c4a33c66d66 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -613,6 +613,22 @@ func TestMuxNoSlashRedirectWithTrailingSlash(t *testing.T) {
        }
 }
 
+// Test that we don't attempt trailing-slash response 405 on a path that already has
+// a trailing slash.
+// See issue #67657.
+func TestMuxNoSlash405WithTrailingSlash(t *testing.T) {
+       mux := NewServeMux()
+       mux.HandleFunc("GET /{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 a5e98f1d957dcba585d501cf3dcafc833d2e5ee3..1ff72a04550c42b1fdb0579a6b2ae373cbb2dadf 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -16,6 +16,7 @@ import (
        "internal/godebug"
        "io"
        "log"
+       "maps"
        "math/rand"
        "net"
        "net/textproto"
@@ -2721,19 +2722,10 @@ func (mux *ServeMux) matchingMethods(host, path string) []string {
        ms := map[string]bool{}
        mux.tree.matchingMethods(host, path, ms)
        // matchOrRedirect will try appending a trailing slash if there is no match.
-       mux.tree.matchingMethods(host, path+"/", ms)
-       methods := mapKeys(ms)
-       slices.Sort(methods)
-       return methods
-}
-
-// TODO(jba): replace with maps.Keys when it is defined.
-func mapKeys[K comparable, V any](m map[K]V) []K {
-       var ks []K
-       for k := range m {
-               ks = append(ks, k)
+       if !strings.HasSuffix(path, "/") {
+               mux.tree.matchingMethods(host, path+"/", ms)
        }
-       return ks
+       return slices.Sorted(maps.Keys(ms))
 }
 
 // ServeHTTP dispatches the request to the handler whose