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

Author: wuyangfan

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.

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"))
+		})
+	}
+}

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,
 		},
 	}

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)