From 9323859cbb6b815e1c2faa5ae86f5080afe931a6 Mon Sep 17 00:00:00 2001 From: Solaris-star <820622658@qq.com> Date: Wed, 22 Jul 2026 05:57:30 +0800 Subject: [PATCH 1/2] fix(router): keep group auto-404 in sync with root RouteNotFound Group.Use auto-registers nil-handler RouteNotFound routes so group middleware still runs on 404s. Those handlers were filled from r.notFoundHandler at registration time, which stayed the default even after e.RouteNotFound("/*", h). Groups with middleware therefore used the stock 404 JSON body instead of the root catch-all (#2485). When a RouteNotFound is added for /* or "", update r.notFoundHandler so later group auto-404 routes inherit the same handler. Fixes #2485 --- group_test.go | 38 ++++++++++++++++++++++++++++++++------ router.go | 8 ++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/group_test.go b/group_test.go index 6a5a31e30..4f5e456f8 100644 --- a/group_test.go +++ b/group_test.go @@ -801,20 +801,20 @@ func TestGroup_RouteNotFoundWithMiddleware(t *testing.T) { expectMiddlewareCalled: true, // because RouteNotFound is added after middleware is added }, { - name: "ok, default group 404 handler is called with middleware", + name: "ok, root RouteNotFound falls back for group auto-404 with middleware", givenCustom404: false, whenURL: "/group/test3", - expectBody: "404 (global) GET /group/*", + expectBody: "404 (local) GET /group/*", expectCode: http.StatusNotFound, - expectMiddlewareCalled: true, // because RouteNotFound is added before middleware is added + expectMiddlewareCalled: true, // group middleware wraps auto RouteNotFound (#2485) }, { - name: "ok, (no slash) default group 404 handler is called with middleware", + name: "ok, (no slash) root RouteNotFound falls back for group auto-404 with middleware", givenCustom404: false, whenURL: "/group", - expectBody: "404 (global) GET /group", + expectBody: "404 (local) GET /group", expectCode: http.StatusNotFound, - expectMiddlewareCalled: true, // because RouteNotFound is added before middleware is added + expectMiddlewareCalled: true, // group middleware wraps auto RouteNotFound (#2485) }, } for _, tc := range testCases { @@ -937,3 +937,29 @@ func TestGroup_UseMultipleTimes(t *testing.T) { }) } + + +func TestGroup_RouteNotFoundFallsBackToRoot(t *testing.T) { + // Root catch-all must still apply for groups that auto-register 404 routes + // when middleware is attached (issue #2485). + e := New() + 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 nil }) + + v1 := e.Group("/v1") + v1.POST("/resource", func(c *Context) error { return nil }) + + for _, path := range []string{"/foo", "/v0/foo", "/v1/foo"} { + req := httptest.NewRequest(http.MethodPost, path, nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.Equal(t, http.StatusNotFound, rec.Code, path) + assert.Equal(t, 0, rec.Body.Len(), path) + } +} diff --git a/router.go b/router.go index 99950aae7..ee7b1d14a 100644 --- a/router.go +++ b/router.go @@ -522,6 +522,14 @@ func (r *DefaultRouter) Add(route Route) (RouteInfo, error) { method := route.Method path := normalizePathSlash(route.Path) + // Keep default/group-auto 404 handlers in sync with the root catch-all + // RouteNotFound. Group.Use registers nil-handler 404 routes that fill from + // r.notFoundHandler; without this, e.RouteNotFound("/*", h) is ignored for + // groups that have middleware (#2485). + if method == RouteNotFound && route.Handler != nil && (path == "/*" || path == "") { + r.notFoundHandler = route.Handler + } + h := applyMiddleware(route.Handler, route.Middlewares...) if !allowOverwritingRoute { for _, rr := range r.routes { From 51adb321cac5047cfa88fae15c300c4d40e0e125 Mon Sep 17 00:00:00 2001 From: Solaris-star <820622658@qq.com> Date: Thu, 23 Jul 2026 17:37:37 +0800 Subject: [PATCH 2/2] fix(#3052): drop Router side-effect, document RouterConfig.NotFoundHandler Per maintainer guidance, adding a route via e.RouteNotFound must not have side effects on the Router. Revert the Add() mutation of r.notFoundHandler. For groups that auto-register catch-all 404 routes (i.e. groups with middleware), the supported way to customise the 404 handler is RouterConfig.NotFoundHandler, which the group's nil-handler routes resolve to at registration time and which group middleware wraps. Update the existing #2485 test expectations to reflect this design and add TestGroup_RouteNotFoundUsesRouterConfig documenting the supported path. --- group_test.go | 67 +++++++++++++++++++++++++++++++++------------------ router.go | 8 ------ 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/group_test.go b/group_test.go index 4f5e456f8..56b2a465f 100644 --- a/group_test.go +++ b/group_test.go @@ -801,20 +801,27 @@ func TestGroup_RouteNotFoundWithMiddleware(t *testing.T) { expectMiddlewareCalled: true, // because RouteNotFound is added after middleware is added }, { - name: "ok, root RouteNotFound falls back for group auto-404 with middleware", + // #2485: a group that auto-registers 404 routes (because it has + // middleware) falls back to the Router's configured NotFoundHandler, + // NOT to a later e.RouteNotFound("/*", ...) call. The group's nil + // handler is resolved to r.notFoundHandler at registration time, and + // e.RouteNotFound only adds a tree route that the group's own catch-all + // shadows. To customise the group 404, configure RouterConfig.NotFoundHandler + // (see TestGroup_RouteNotFoundUsesRouterConfig). + name: "ok, group auto-404 uses configured NotFoundHandler, not later e.RouteNotFound", givenCustom404: false, whenURL: "/group/test3", - expectBody: "404 (local) GET /group/*", + expectBody: "404 (global) GET /group/*", expectCode: http.StatusNotFound, - expectMiddlewareCalled: true, // group middleware wraps auto RouteNotFound (#2485) + expectMiddlewareCalled: true, // group middleware still wraps the auto 404 route }, { - name: "ok, (no slash) root RouteNotFound falls back for group auto-404 with middleware", + name: "ok, (no slash) group auto-404 uses configured NotFoundHandler, not later e.RouteNotFound", givenCustom404: false, whenURL: "/group", - expectBody: "404 (local) GET /group", + expectBody: "404 (global) GET /group", expectCode: http.StatusNotFound, - expectMiddlewareCalled: true, // group middleware wraps auto RouteNotFound (#2485) + expectMiddlewareCalled: true, // group middleware still wraps the auto 404 route }, } for _, tc := range testCases { @@ -938,28 +945,40 @@ func TestGroup_UseMultipleTimes(t *testing.T) { } +// TestGroup_RouteNotFoundUsesRouterConfig documents the supported way to +// customise the 404 handler for groups that auto-register catch-all routes +// (i.e. groups that have middleware). Per maintainer guidance on #2485/#3052, +// adding a route via e.RouteNotFound must NOT have side effects on the Router; +// instead configure RouterConfig.NotFoundHandler, which the group's auto 404 +// routes resolve to at registration time and which group middleware wraps. +func TestGroup_RouteNotFoundUsesRouterConfig(t *testing.T) { + customNotFound := func(c *Context) error { + return c.String(http.StatusNotFound, "custom-404 "+c.Request().Method+" "+c.Path()) + } -func TestGroup_RouteNotFoundFallsBackToRoot(t *testing.T) { - // Root catch-all must still apply for groups that auto-register 404 routes - // when middleware is attached (issue #2485). - e := New() - e.RouteNotFound("/*", func(c *Context) error { - return c.NoContent(http.StatusNotFound) + e := NewWithConfig(Config{ + Router: NewRouter(RouterConfig{ + NotFoundHandler: customNotFound, + }), }) - v0 := e.Group("/v0", func(next HandlerFunc) HandlerFunc { - return func(c *Context) error { return next(c) } + middlewareCalled := false + g := e.Group("/v0") + g.Use(func(next HandlerFunc) HandlerFunc { + return func(c *Context) error { + middlewareCalled = true + return next(c) + } }) - v0.POST("/resource", func(c *Context) error { return nil }) + g.POST("/resource", func(c *Context) error { return c.NoContent(http.StatusOK) }) - v1 := e.Group("/v1") - v1.POST("/resource", func(c *Context) error { return nil }) + // Unmatched path inside the group: the group's auto-registered catch-all + // resolves to the configured NotFoundHandler, and group middleware wraps it. + req := httptest.NewRequest(http.MethodPost, "/v0/missing", nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) - for _, path := range []string{"/foo", "/v0/foo", "/v1/foo"} { - req := httptest.NewRequest(http.MethodPost, path, nil) - rec := httptest.NewRecorder() - e.ServeHTTP(rec, req) - assert.Equal(t, http.StatusNotFound, rec.Code, path) - assert.Equal(t, 0, rec.Body.Len(), path) - } + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Equal(t, "custom-404 POST /v0/*", rec.Body.String()) + assert.True(t, middlewareCalled, "group middleware must wrap the auto 404 route") } diff --git a/router.go b/router.go index ee7b1d14a..99950aae7 100644 --- a/router.go +++ b/router.go @@ -522,14 +522,6 @@ func (r *DefaultRouter) Add(route Route) (RouteInfo, error) { method := route.Method path := normalizePathSlash(route.Path) - // Keep default/group-auto 404 handlers in sync with the root catch-all - // RouteNotFound. Group.Use registers nil-handler 404 routes that fill from - // r.notFoundHandler; without this, e.RouteNotFound("/*", h) is ignored for - // groups that have middleware (#2485). - if method == RouteNotFound && route.Handler != nil && (path == "/*" || path == "") { - r.notFoundHandler = route.Handler - } - h := applyMiddleware(route.Handler, route.Middlewares...) if !allowOverwritingRoute { for _, rr := range r.routes {