net/http: handle MethodNotAllowed

If no pattern matches a request, but a pattern would have
matched if the request had a different method, then
serve a 405 (Method Not Allowed), and populate the
"Allow" header with the methods that would have succeeded.

Updates #61640.

Change-Id: I0ae9eb95e62c71ff7766a03043525a97099ac1bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/528401
Reviewed-by: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: jba

src/net/http/request_test.go

@@ -15,6 +15,7 @@ import (
 	"io"
 	"math"
 	"mime/multipart"
+	"net/http"
 	. "net/http"
 	"net/http/httptest"
 	"net/url"
@@ -1473,10 +1474,11 @@ func TestPathValue(t *testing.T) {
 		})
 		server := httptest.NewServer(mux)
 		defer server.Close()
-		_, err := Get(server.URL + test.url)
+		res, err := Get(server.URL + test.url)
 		if err != nil {
 			t.Fatal(err)
 		}
+		res.Body.Close()
 	}
 }
 
@@ -1499,8 +1501,57 @@ func TestSetPathValue(t *testing.T) {
 	})
 	server := httptest.NewServer(mux)
 	defer server.Close()
-	_, err := Get(server.URL + "/a/b/c/d/e")
+	res, err := Get(server.URL + "/a/b/c/d/e")
 	if err != nil {
 		t.Fatal(err)
 	}
+	res.Body.Close()
+}
+
+func TestStatus(t *testing.T) {
+	// The main purpose of this test is to check 405 responses and the Allow header.
+	h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})
+	mux := NewServeMux()
+	mux.Handle("GET /g", h)
+	mux.Handle("POST /p", h)
+	mux.Handle("PATCH /p", h)
+	mux.Handle("PUT /r", h)
+	mux.Handle("GET /r/", h)
+	server := httptest.NewServer(mux)
+	defer server.Close()
+
+	for _, test := range []struct {
+		method, path string
+		wantStatus   int
+		wantAllow    string
+	}{
+		{"GET", "/g", 200, ""},
+		{"HEAD", "/g", 200, ""},
+		{"POST", "/g", 405, "GET, HEAD"},
+		{"GET", "/x", 404, ""},
+		{"GET", "/p", 405, "PATCH, POST"},
+		{"GET", "/./p", 405, "PATCH, POST"},
+		{"GET", "/r/", 200, ""},
+		{"GET", "/r", 200, ""}, // redirected
+		{"HEAD", "/r/", 200, ""},
+		{"HEAD", "/r", 200, ""}, // redirected
+		{"PUT", "/r/", 405, "GET, HEAD"},
+		{"PUT", "/r", 200, ""},
+	} {
+		req, err := http.NewRequest(test.method, server.URL+test.path, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		res, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		res.Body.Close()
+		if g, w := res.StatusCode, test.wantStatus; g != w {
+			t.Errorf("%s %s: got %d, want %d", test.method, test.path, g, w)
+		}
+		if g, w := res.Header.Get("Allow"), test.wantAllow; g != w {
+			t.Errorf("%s %s, Allow: got %q, want %q", test.method, test.path, g, w)
+		}
+	}
 }

src/net/http/routing_tree.go

@@ -220,3 +220,30 @@ func firstSegment(path string) (seg, rest string) {
 	}
 	return path[:i], path[i:]
 }
+
+// matchingMethods adds to methodSet all the methods that would result in a
+// match if passed to routingNode.match with the given host and path.
+func (root *routingNode) matchingMethods(host, path string, methodSet map[string]bool) {
+	if host != "" {
+		root.findChild(host).matchingMethodsPath(path, methodSet)
+	}
+	root.emptyChild.matchingMethodsPath(path, methodSet)
+	if methodSet["GET"] {
+		methodSet["HEAD"] = true
+	}
+}
+
+func (n *routingNode) matchingMethodsPath(path string, set map[string]bool) {
+	if n == nil {
+		return
+	}
+	n.children.eachPair(func(method string, c *routingNode) bool {
+		if p, _ := c.matchPath(path, nil); p != nil {
+			set[method] = true
+		}
+		return true
+	})
+	// Don't look at the empty child. If there were an empty
+	// child, it would match on any method, but we only
+	// call this when we fail to match on a method.
+}

src/net/http/routing_tree_test.go

@@ -209,6 +209,65 @@ func TestRoutingNodeMatch(t *testing.T) {
 	})
 }
 
+func TestMatchingMethods(t *testing.T) {
+	hostTree := buildTree("GET a.com/", "PUT b.com/", "POST /foo/{x}")
+	for _, test := range []struct {
+		name       string
+		tree       *routingNode
+		host, path string
+		want       string
+	}{
+		{
+			"post",
+			buildTree("POST /"), "", "/foo",
+			"POST",
+		},
+		{
+			"get",
+			buildTree("GET /"), "", "/foo",
+			"GET,HEAD",
+		},
+		{
+			"host",
+			hostTree, "", "/foo",
+			"",
+		},
+		{
+			"host",
+			hostTree, "", "/foo/bar",
+			"POST",
+		},
+		{
+			"host2",
+			hostTree, "a.com", "/foo/bar",
+			"GET,HEAD,POST",
+		},
+		{
+			"host3",
+			hostTree, "b.com", "/bar",
+			"PUT",
+		},
+		{
+			// This case shouldn't come up because we only call matchingMethods
+			// when there was no match, but we include it for completeness.
+			"empty",
+			buildTree("/"), "", "/",
+			"",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			ms := map[string]bool{}
+			test.tree.matchingMethods(test.host, test.path, ms)
+			keys := mapKeys(ms)
+			sort.Strings(keys)
+			got := strings.Join(keys, ",")
+			if got != test.want {
+				t.Errorf("got %s, want %s", got, test.want)
+			}
+		})
+	}
+}
+
 func (n *routingNode) print(w io.Writer, level int) {
 	indent := strings.Repeat("    ", level)
 	if n.pattern != nil {

src/net/http/server.go

@@ -23,6 +23,7 @@ import (
 	urlpkg "net/url"
 	"path"
 	"runtime"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -2423,13 +2424,13 @@ func (mux *ServeMux) findHandler(r *Request) (h Handler, patStr string, _ *patte
 	// TODO(jba): use escaped path. This is an independent change that is also part
 	// of proposal https://go.dev/issue/61410.
 	path := r.URL.Path
-
+	host := r.URL.Host
 	// CONNECT requests are not canonicalized.
 	if r.Method == "CONNECT" {
 		// If r.URL.Path is /tree and its handler is not registered,
 		// the /tree -> /tree/ redirect applies to CONNECT requests
 		// but the path canonicalization does not.
-		_, _, u := mux.matchOrRedirect(r.URL.Host, r.Method, path, r.URL)
+		_, _, u := mux.matchOrRedirect(host, r.Method, path, r.URL)
 		if u != nil {
 			return RedirectHandler(u.String(), StatusMovedPermanently), u.Path, nil, nil
 		}
@@ -2439,7 +2440,7 @@ func (mux *ServeMux) findHandler(r *Request) (h Handler, patStr string, _ *patte
 	} else {
 		// All other requests have any port stripped and path cleaned
 		// before passing to mux.handler.
-		host := stripHostPort(r.Host)
+		host = stripHostPort(r.Host)
 		path = cleanPath(path)
 
 		// If the given path is /tree and its handler is not registered,
@@ -2460,7 +2461,16 @@ func (mux *ServeMux) findHandler(r *Request) (h Handler, patStr string, _ *patte
 		}
 	}
 	if n == nil {
-		// TODO(jba): support 405 (MethodNotAllowed) by checking for patterns with different methods.
+		// We didn't find a match with the request method. To distinguish between
+		// Not Found and Method Not Allowed, see if there is another pattern that
+		// matches except for the method.
+		allowedMethods := mux.matchingMethods(host, path)
+		if len(allowedMethods) > 0 {
+			return HandlerFunc(func(w ResponseWriter, r *Request) {
+				w.Header().Set("Allow", strings.Join(allowedMethods, ", "))
+				Error(w, StatusText(StatusMethodNotAllowed), StatusMethodNotAllowed)
+			}), "", nil, nil
+		}
 		return NotFoundHandler(), "", nil, nil
 	}
 	return n.handler, n.pattern.String(), n.pattern, matches
@@ -2542,6 +2552,30 @@ func exactMatch(n *routingNode, path string) bool {
 	return len(n.pattern.segments) == strings.Count(path, "/")
 }
 
+// matchingMethods return a sorted list of all methods that would match with the given host and path.
+func (mux *ServeMux) matchingMethods(host, path string) []string {
+	// Hold the read lock for the entire method so that the two matches are done
+	// on the same set of registered patterns.
+	mux.mu.RLock()
+	defer mux.mu.RUnlock()
+	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)
+	sort.Strings(methods)
+	return methods
+}
+
+// TODO: 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)
+	}
+	return ks
+}
+
 // ServeHTTP dispatches the request to the handler whose
 // pattern most closely matches the request URL.
 func (mux *ServeMux) ServeHTTP(w ResponseWriter, r *Request) {