Skip to content

Commit ca34ff9

Browse files
committed
fix: RouteNotFound handler fallback to root handler for groups with middleware
When a group has middleware, the RouteNotFound handler registered at the root level is now properly used as a fallback. Previously, groups with middleware would always use the default NotFoundHandler instead of falling back to the root's custom RouteNotFound handler. This fix: - Adds a routeNotFoundHandler field to Echo to store the root handler - Modifies group.Use() to use a fallback handler that checks for the root's RouteNotFound handler at request time - Updates tests to reflect the new expected behavior Fixes #2485 Signed-off-by: majiayu000 <[email protected]>
1 parent d0f9d1e commit ca34ff9

File tree

3 files changed

+45
-6
lines changed

3 files changed

+45
-6
lines changed

echo.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ type Echo struct {
105105
Debug bool
106106
HideBanner bool
107107
HidePort bool
108+
109+
// routeNotFoundHandler is the handler registered via RouteNotFound for "/*" path.
110+
// It is used as a fallback for groups with middleware that don't have their own RouteNotFound handler.
111+
routeNotFoundHandler HandlerFunc
108112
}
109113

110114
// Route contains a handler and information for matching against requests.
@@ -542,6 +546,10 @@ func (e *Echo) TRACE(path string, h HandlerFunc, m ...MiddlewareFunc) *Route {
542546
//
543547
// Example: `e.RouteNotFound("/*", func(c echo.Context) error { return c.NoContent(http.StatusNotFound) })`
544548
func (e *Echo) RouteNotFound(path string, h HandlerFunc, m ...MiddlewareFunc) *Route {
549+
// Store the handler for "/*" path so it can be used as fallback for groups with middleware
550+
if path == "/*" || path == "*" {
551+
e.routeNotFoundHandler = h
552+
}
545553
return e.Add(RouteNotFound, path, h, m...)
546554
}
547555

group.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,17 @@ func (g *Group) Use(middleware ...MiddlewareFunc) {
2828
// are only executed if they are added to the Router with route.
2929
// So we register catch all route (404 is a safe way to emulate route match) for this group and now during routing the
3030
// Router would find route to match our request path and therefore guarantee the middleware(s) will get executed.
31-
g.RouteNotFound("", NotFoundHandler)
32-
g.RouteNotFound("/*", NotFoundHandler)
31+
//
32+
// We use a fallback handler that checks for a root-level RouteNotFound handler at request time.
33+
// This allows the root's custom 404 handler to be used as fallback for groups.
34+
fallbackNotFoundHandler := func(c Context) error {
35+
if g.echo.routeNotFoundHandler != nil {
36+
return g.echo.routeNotFoundHandler(c)
37+
}
38+
return NotFoundHandler(c)
39+
}
40+
g.RouteNotFound("", fallbackNotFoundHandler)
41+
g.RouteNotFound("/*", fallbackNotFoundHandler)
3342
}
3443

3544
// CONNECT implements `Echo#CONNECT()` for sub-routes within the Group.

group_test.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,17 +204,17 @@ func TestGroup_RouteNotFoundWithMiddleware(t *testing.T) {
204204
expectCode: http.StatusNotFound,
205205
},
206206
{
207-
name: "ok, default group 404 handler is called with middleware",
207+
name: "ok, root 404 handler is used as fallback with middleware",
208208
givenCustom404: false,
209209
whenURL: "/group/test3",
210-
expectBody: "{\"message\":\"Not Found\"}\n",
210+
expectBody: "GET /group/*",
211211
expectCode: http.StatusNotFound,
212212
},
213213
{
214-
name: "ok, (no slash) default group 404 handler is called with middleware",
214+
name: "ok, (no slash) root 404 handler is used as fallback with middleware",
215215
givenCustom404: false,
216216
whenURL: "/group",
217-
expectBody: "{\"message\":\"Not Found\"}\n",
217+
expectBody: "GET /group",
218218
expectCode: http.StatusNotFound,
219219
},
220220
}
@@ -257,3 +257,25 @@ func TestGroup_RouteNotFoundWithMiddleware(t *testing.T) {
257257
})
258258
}
259259
}
260+
261+
func TestGroup_RouteNotFoundWithMiddleware_NoRootHandler(t *testing.T) {
262+
e := New()
263+
264+
middlewareCalled := false
265+
g := e.Group("/group")
266+
g.Use(func(next HandlerFunc) HandlerFunc {
267+
return func(c Context) error {
268+
middlewareCalled = true
269+
return next(c)
270+
}
271+
})
272+
273+
req := httptest.NewRequest(http.MethodGet, "/group/unknown", nil)
274+
rec := httptest.NewRecorder()
275+
276+
e.ServeHTTP(rec, req)
277+
278+
assert.True(t, middlewareCalled)
279+
assert.Equal(t, http.StatusNotFound, rec.Code)
280+
assert.Equal(t, "{\"message\":\"Not Found\"}\n", rec.Body.String())
281+
}

0 commit comments

Comments
 (0)