From 52f50792087e48c26b1841f8ad74a24149e0a52a Mon Sep 17 00:00:00 2001 From: wuyangfan Date: Mon, 25 May 2026 18:05:05 +0800 Subject: [PATCH] fix: inherit parent RouteNotFound handler for groups with middleware When a group registers middleware, reuse the most specific parent RouteNotFound handler instead of always falling back to the default JSON NotFoundHandler. Fixes #2485 --- group.go | 8 +++- group_routenotfound_fallback_test.go | 55 ++++++++++++++++++++++++++++ group_test.go | 8 ++-- router.go | 51 ++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 group_routenotfound_fallback_test.go diff --git a/group.go b/group.go index cb37b123f..06e9f98fb 100644 --- a/group.go +++ b/group.go @@ -28,8 +28,12 @@ func (g *Group) Use(middleware ...MiddlewareFunc) { // are only executed if they are added to the Router with route. // So we register catch all route (404 is a safe way to emulate route match) for this group and now during routing the // Router would find route to match our request path and therefore guarantee the middleware(s) will get executed. - g.RouteNotFound("", NotFoundHandler) - g.RouteNotFound("/*", NotFoundHandler) + handler := NotFoundHandler + if inherited := g.echo.router.inheritedRouteNotFoundHandler(g.prefix); inherited != nil { + handler = inherited + } + g.RouteNotFound("", handler) + g.RouteNotFound("/*", handler) } // CONNECT implements `Echo#CONNECT()` for sub-routes within the Group. diff --git a/group_routenotfound_fallback_test.go b/group_routenotfound_fallback_test.go new file mode 100644 index 000000000..7b970e074 --- /dev/null +++ b/group_routenotfound_fallback_test.go @@ -0,0 +1,55 @@ +package echo + +import ( + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGroupRouteNotFoundFallsBackToRootHandler(t *testing.T) { + e := New() + e.HideBanner = true + e.HidePort = true + + e.RouteNotFound("/*", func(c Context) error { + return c.NoContent(http.StatusNotFound) + }) + + v0 := e.Group("/v0", func(next HandlerFunc) HandlerFunc { + return func(c Context) error { + return next(c) + } + }) + v0.POST("/resource", func(c Context) error { + return c.NoContent(http.StatusOK) + }) + + v1 := e.Group("/v1") + v1.POST("/resource", func(c Context) error { + return c.NoContent(http.StatusOK) + }) + + srv := httptest.NewServer(e) + t.Cleanup(srv.Close) + + for _, path := range []string{"/foo", "/v0/foo", "/v1/foo"} { + t.Run(path, func(t *testing.T) { + req, err := http.NewRequest(http.MethodPost, srv.URL+path, nil) + require.NoError(t, err) + + resp, err := srv.Client().Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + b, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + require.Equal(t, http.StatusNotFound, resp.StatusCode) + require.Empty(t, b) + require.Equal(t, "0", resp.Header.Get("Content-Length")) + }) + } +} diff --git a/group_test.go b/group_test.go index a97371418..ff868f5b5 100644 --- a/group_test.go +++ b/group_test.go @@ -204,17 +204,17 @@ func TestGroup_RouteNotFoundWithMiddleware(t *testing.T) { expectCode: http.StatusNotFound, }, { - name: "ok, default group 404 handler is called with middleware", + name: "ok, inherited root 404 handler is called with middleware", givenCustom404: false, whenURL: "/group/test3", - expectBody: "{\"message\":\"Not Found\"}\n", + expectBody: "GET /group/*", expectCode: http.StatusNotFound, }, { - name: "ok, (no slash) default group 404 handler is called with middleware", + name: "ok, (no slash) inherited root 404 handler is called with middleware", givenCustom404: false, whenURL: "/group", - expectBody: "{\"message\":\"Not Found\"}\n", + expectBody: "GET /group", expectCode: http.StatusNotFound, }, } diff --git a/router.go b/router.go index 912cfeac0..96fa4625c 100644 --- a/router.go +++ b/router.go @@ -7,6 +7,7 @@ import ( "bytes" "fmt" "net/http" + "strings" ) // Router is the registry of all registered routes for an `Echo` instance for @@ -155,6 +156,56 @@ func (r *Router) Routes() []*Route { return routes } +func (r *Router) inheritedRouteNotFoundHandler(groupPrefix string) HandlerFunc { + var best *routeMethod + var walk func(n *node) + walk = func(n *node) { + if h := n.notFoundHandler; h != nil && isParentRouteNotFound(h.ppath, groupPrefix) { + if best == nil || routeNotFoundSpecificity(h.ppath) > routeNotFoundSpecificity(best.ppath) { + best = h + } + } + for _, child := range n.staticChildren { + walk(child) + } + if n.paramChild != nil { + walk(n.paramChild) + } + if n.anyChild != nil { + walk(n.anyChild) + } + } + walk(r.tree) + if best != nil { + return best.handler + } + return nil +} + +func isParentRouteNotFound(routePath, groupPrefix string) bool { + if routePath == groupPrefix || routePath == groupPrefix+"/*" { + return false + } + if routePath == "" || routePath == "/*" { + return true + } + if strings.HasSuffix(routePath, "/*") { + base := strings.TrimSuffix(routePath, "/*") + if base == "" { + return true + } + return groupPrefix == base || strings.HasPrefix(groupPrefix, base+"/") + } + return groupPrefix == routePath || strings.HasPrefix(groupPrefix, routePath+"/") +} + +func routeNotFoundSpecificity(path string) int { + if path == "" || path == "/*" { + return 0 + } + return len(path) +} + // Reverse generates a URL from route name and provided parameters. func (r *Router) Reverse(name string, params ...interface{}) string { uri := new(bytes.Buffer)