middleware: resolve SupressNotFound preflight like the router does

middleware/supress_notfound.go

@@ -12,11 +12,35 @@ import (
 // This is handy to put at the top of your middleware stack to avoid unnecessary
 // processing of requests that are not going to match any routes anyway. For
 // example its super annoying to see a bunch of 404's in your logs from bots.
+//
+// Note that a route which exists but has no handler for the request method is
+// also reported as not found, so such requests get a 404 from here rather than
+// a 405 from the router.
 func SupressNotFound(router *chi.Mux) func(next http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			rctx := chi.RouteContext(r.Context())
-			match := rctx.Routes.Match(chi.NewRouteContext(), r.Method, r.URL.Path)
+
+			// Resolve the method and path the same way the router resolves them
+			// when it dispatches. Otherwise the preflight can reject a request
+			// the router would have served: dispatch prefers RawPath over the
+			// decoded URL.Path, and middleware earlier in the chain may have
+			// overridden RouteMethod (GetHead does this for HEAD requests).
+			method := rctx.RouteMethod
+			if method == "" {
+				method = r.Method
+			}
+			path := r.URL.Path
+			if r.URL.RawPath != "" {
+				path = r.URL.RawPath
+			}
+			if path == "" {
+				path = "/"
+			}
+
+			// Match records state on the context it is given, so probe with a
+			// throwaway context, as GetHead does.
+			match := rctx.Routes.Match(chi.NewRouteContext(), method, path)
 			if !match {
 				router.NotFoundHandler().ServeHTTP(w, r)
 				return

middleware/supress_notfound_test.go

@@ -80,6 +80,59 @@ func TestSupressNotFoundMount(t *testing.T) {
 	}
 }
 
+// SupressNotFound must resolve the method and path the same way the router
+// resolves them when it dispatches, otherwise it rejects requests the router
+// would have served.
+func TestSupressNotFoundResolvesRequestLikeRouter(t *testing.T) {
+	t.Run("raw path", func(t *testing.T) {
+		r := chi.NewRouter()
+		r.Use(SupressNotFound(r))
+		r.Get("/files/{id}", func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte("id=" + chi.URLParam(r, "id")))
+		})
+
+		w := httptest.NewRecorder()
+		// URL.Path is /files/a/b, RawPath is /files/a%2Fb. The router dispatches
+		// on RawPath, so {id} is a%2Fb and the route matches.
+		r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/files/a%2Fb", nil))
+
+		if w.Code != http.StatusOK {
+			t.Fatalf("status: got %d, want %d", w.Code, http.StatusOK)
+		}
+		if got := w.Body.String(); got != "id=a%2Fb" {
+			t.Errorf("body: got %q, want %q", got, "id=a%2Fb")
+		}
+	})
+
+	t.Run("method overridden upstream", func(t *testing.T) {
+		r := chi.NewRouter()
+		// GetHead rewrites HEAD to GET before SupressNotFound runs.
+		r.Use(GetHead)
+		r.Use(SupressNotFound(r))
+		r.Get("/x", func(w http.ResponseWriter, r *http.Request) {})
+
+		w := httptest.NewRecorder()
+		r.ServeHTTP(w, httptest.NewRequest(http.MethodHead, "/x", nil))
+
+		if w.Code != http.StatusOK {
+			t.Fatalf("status: got %d, want %d", w.Code, http.StatusOK)
+		}
+	})
+
+	t.Run("unknown path still suppressed", func(t *testing.T) {
+		r := chi.NewRouter()
+		r.Use(SupressNotFound(r))
+		r.Get("/files/{id}", func(w http.ResponseWriter, r *http.Request) {})
+
+		w := httptest.NewRecorder()
+		r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/files/a/b/c", nil))
+
+		if w.Code != http.StatusNotFound {
+			t.Fatalf("status: got %d, want %d", w.Code, http.StatusNotFound)
+		}
+	})
+}
+
 func TestSupressNotFoundRoutePattern(t *testing.T) {
 	r := chi.NewRouter()
 	r.Use(SupressNotFound(r))