Revert back to v4 behavior for group registering implicit 404 handlers. This will fix: CORS middleware doesnt automatically handle OPTIONS routes for groups anymore since upgrade to v5

Author: aldas

echo.go

@@ -100,6 +100,10 @@ type Echo struct {
 
 	// formParseMaxMemory is passed to Context for multipart form parsing (See http.Request.ParseMultipartForm)
 	formParseMaxMemory int64
+
+	// noGroupAutoRegisterRoutes is a flag that indicates whether echo.Group should NOT register 404 routes automatically
+	// when there are middlewares registered with the group.
+	noGroupAutoRegisterRoutes bool
 }
 
 // JSONSerializer is the interface that encodes and decodes JSON to and from interfaces.
@@ -288,6 +292,10 @@ type Config struct {
 	// FormParseMaxMemory is default value for memory limit that is used
 	// when parsing multipart forms (See (*http.Request).ParseMultipartForm)
 	FormParseMaxMemory int64
+
+	// NoGroupAutoRegister404Routes bool is a flag that indicates whether echo.Group should NOT register 404 routes automatically
+	// when there are middlewares registered with the group.
+	NoGroupAutoRegister404Routes bool
 }
 
 // NewWithConfig creates an instance of Echo with given configuration.
@@ -326,6 +334,8 @@ func NewWithConfig(config Config) *Echo {
 	if config.FormParseMaxMemory > 0 {
 		e.formParseMaxMemory = config.FormParseMaxMemory
 	}
+	e.noGroupAutoRegisterRoutes = config.NoGroupAutoRegister404Routes
+
 	return e
 }
 
@@ -342,7 +352,9 @@ func New() *Echo {
 	}
 
 	e.serveHTTPFunc = e.serveHTTP
-	e.router = NewRouter(RouterConfig{})
+	e.router = NewRouter(RouterConfig{
+		AllowOverwritingRoute: true,
+	})
 	e.HTTPErrorHandler = DefaultHTTPErrorHandler(false)
 	e.contextPool.New = func() any {
 		return newContext(nil, nil, e)
@@ -657,7 +669,11 @@ func (e *Echo) Add(method, path string, handler HandlerFunc, middleware ...Middl
 
 // Group creates a new router group with prefix and optional group-level middleware.
 func (e *Echo) Group(prefix string, m ...MiddlewareFunc) (g *Group) {
-	g = &Group{prefix: prefix, echo: e}
+	g = &Group{
+		echo:                 e,
+		prefix:               prefix,
+		noAutoRegisterRoutes: e.noGroupAutoRegisterRoutes,
+	}
 	g.Use(m...)
 	return
 }

echo_test.go

@@ -824,12 +824,12 @@ func TestEchoServeHTTPPathEncoding(t *testing.T) {
 func TestEchoGroup(t *testing.T) {
 	e := New()
 	buf := new(bytes.Buffer)
-	e.Use(MiddlewareFunc(func(next HandlerFunc) HandlerFunc {
+	e.Use(func(next HandlerFunc) HandlerFunc {
 		return func(c *Context) error {
 			buf.WriteString("0")
 			return next(c)
 		}
-	}))
+	})
 	h := func(c *Context) error {
 		return c.NoContent(http.StatusOK)
 	}

group.go

@@ -15,12 +15,31 @@ type Group struct {
 	echo       *Echo
 	prefix     string
 	middleware []MiddlewareFunc
+
+	// noAutoRegisterRoutes is a flag that indicates whether Group should NOT register 404 routes automatically
+	// when there are middlewares registered with the group.
+	// Note: if you decide not to register 404 routes automatically, make sure to check if all your middlewares are executed
+	// as expected. For example - CORS middleware.
+	noAutoRegisterRoutes bool
 }
 
 // Use implements `Echo#Use()` for sub-routes within the Group.
 // Group middlewares are not executed on request when there is no matching route found.
 func (g *Group) Use(middleware ...MiddlewareFunc) {
 	g.middleware = append(g.middleware, middleware...)
+	if len(g.middleware) == 0 {
+		return
+	}
+	if g.noAutoRegisterRoutes {
+		return
+	}
+	// group level middlewares are different from Echo `Pre` and `Use` middlewares (those are global). Group level middlewares
+	// 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.
+	// Note: we use nil handler so Router would choose the default 404 handler. This may not work with custom routers.
+	g.RouteNotFound("", nil)
+	g.RouteNotFound("/*", nil)
 }
 
 // CONNECT implements `Echo#CONNECT()` for sub-routes within the Group. Panics on error.

group_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestGroup_withoutRouteWillNotExecuteMiddleware(t *testing.T) {
+func TestGroup_withoutRouteWillExecuteMiddleware(t *testing.T) {
 	e := New()
 
 	called := false
@@ -24,17 +24,18 @@ func TestGroup_withoutRouteWillNotExecuteMiddleware(t *testing.T) {
 			return c.NoContent(http.StatusTeapot)
 		}
 	}
-	// even though group has middleware it will not be executed when there are no routes under that group
+	// even though group has middleware it will be executed when there are no routes under that group
+	// because implicit routes ("" and "/*") are created for the group
 	_ = e.Group("/group", mw)
 
 	status, body := request(http.MethodGet, "/group/nope", e)
-	assert.Equal(t, http.StatusNotFound, status)
-	assert.Equal(t, `{"message":"Not Found"}`+"\n", body)
+	assert.Equal(t, http.StatusTeapot, status)
+	assert.Equal(t, "", body)
 
-	assert.False(t, called)
+	assert.True(t, called)
 }
 
-func TestGroup_withRoutesWillNotExecuteMiddlewareFor404(t *testing.T) {
+func TestGroup_withRoutesWillExecuteMiddlewareFor404(t *testing.T) {
 	e := New()
 
 	called := false
@@ -45,15 +46,17 @@ func TestGroup_withRoutesWillNotExecuteMiddlewareFor404(t *testing.T) {
 		}
 	}
 	// even though group has middleware and routes when we have no match on some route the middlewares for that
-	// group will not be executed
+	// group will be executed
 	g := e.Group("/group", mw)
 	g.GET("/yes", handlerFunc)
 
+	// route was `/group/yes` but we are requesting `/group/nope` which will result 404 by Router, but middleware will be
+	// not reach the handler and return 418
 	status, body := request(http.MethodGet, "/group/nope", e)
-	assert.Equal(t, http.StatusNotFound, status)
-	assert.Equal(t, `{"message":"Not Found"}`+"\n", body)
+	assert.Equal(t, http.StatusTeapot, status)
+	assert.Equal(t, "", body)
 
-	assert.False(t, called)
+	assert.True(t, called)
 }
 
 func TestGroup_multiLevelGroup(t *testing.T) {
@@ -407,7 +410,9 @@ func TestGroup_Match(t *testing.T) {
 }
 
 func TestGroup_MatchWithErrors(t *testing.T) {
-	e := New()
+	e := NewWithConfig(Config{
+		Router: NewRouter(RouterConfig{AllowOverwritingRoute: false}), // to trigger "duplicate route" error
+	})
 
 	users := e.Group("/users")
 	users.GET("/activate", func(c *Context) error {
@@ -752,25 +757,25 @@ func TestGroup_RouteNotFoundWithMiddleware(t *testing.T) {
 			name:                   "ok, custom 404 handler is called with middleware",
 			givenCustom404:         true,
 			whenURL:                "/group/test3",
-			expectBody:             "404 GET /group/*",
+			expectBody:             "404 (local) GET /group/*",
 			expectCode:             http.StatusNotFound,
 			expectMiddlewareCalled: true, // because RouteNotFound is added after middleware is added
 		},
 		{
-			name:                   "ok, default group 404 handler is not called with middleware",
+			name:                   "ok, default group 404 handler is called with middleware",
 			givenCustom404:         false,
 			whenURL:                "/group/test3",
-			expectBody:             "404 GET /*",
+			expectBody:             "404 (global) GET /group/*",
 			expectCode:             http.StatusNotFound,
-			expectMiddlewareCalled: false, // because RouteNotFound is added before middleware is added
+			expectMiddlewareCalled: true, // because RouteNotFound is added before middleware is added
 		},
 		{
 			name:                   "ok, (no slash) default group 404 handler is called with middleware",
 			givenCustom404:         false,
 			whenURL:                "/group",
-			expectBody:             "404 GET /*",
+			expectBody:             "404 (global) GET /group",
 			expectCode:             http.StatusNotFound,
-			expectMiddlewareCalled: false, // because RouteNotFound is added before middleware is added
+			expectMiddlewareCalled: true, // because RouteNotFound is added before middleware is added
 		},
 	}
 	for _, tc := range testCases {
@@ -779,13 +784,23 @@ func TestGroup_RouteNotFoundWithMiddleware(t *testing.T) {
 			okHandler := func(c *Context) error {
 				return c.String(http.StatusOK, c.Request().Method+" "+c.Path())
 			}
-			notFoundHandler := func(c *Context) error {
-				return c.String(http.StatusNotFound, "404 "+c.Request().Method+" "+c.Path())
+			old404 := notFoundHandler
+			defer func() { notFoundHandler = old404 }()
+
+			localNotFoundHandler := func(c *Context) error {
+				return c.String(http.StatusNotFound, "404 (local) "+c.Request().Method+" "+c.Path())
 			}
 
-			e := New()
+			e := NewWithConfig(Config{
+				Router: NewRouter(RouterConfig{
+					AllowOverwritingRoute: true,
+					NotFoundHandler: func(c *Context) error {
+						return c.String(http.StatusNotFound, "404 (global) "+c.Request().Method+" "+c.Path())
+					},
+				}),
+			})
 			e.GET("/test1", okHandler)
-			e.RouteNotFound("/*", notFoundHandler)
+			e.RouteNotFound("/*", localNotFoundHandler)
 
 			g := e.Group("/group")
 			g.GET("/test1", okHandler)
@@ -798,7 +813,7 @@ func TestGroup_RouteNotFoundWithMiddleware(t *testing.T) {
 				}
 			})
 			if tc.givenCustom404 {
-				g.RouteNotFound("/*", notFoundHandler)
+				g.RouteNotFound("/*", localNotFoundHandler)
 			}
 
 			req := httptest.NewRequest(http.MethodGet, tc.whenURL, nil)

route.go

@@ -12,11 +12,14 @@ import (
 )
 
 // Route contains information to adding/registering new route with the router.
-// Method+Path pair uniquely identifies the Route. It is mandatory to provide Method+Path+Handler fields.
+// Method+Path pair uniquely identifies the Route. It is mandatory to provide Method+Path fields.
 type Route struct {
-	Method      string
-	Path        string
-	Name        string
+	Method string
+	Path   string
+	Name   string
+
+	// HandlerFunc is a function that handles HTTP requests. This could be left nil when the Router implementation allows
+	// fallback to default/global handlers in certain situations.
 	Handler     HandlerFunc
 	Middlewares []MiddlewareFunc
 }

router.go

@@ -73,11 +73,27 @@ type DefaultRouter struct {
 
 // RouterConfig is configuration options for (default) router
 type RouterConfig struct {
-	NotFoundHandler           HandlerFunc
-	MethodNotAllowedHandler   HandlerFunc
-	OptionsMethodHandler      HandlerFunc
-	AllowOverwritingRoute     bool
-	UnescapePathParamValues   bool
+	// NotFoundHandler is a handler that is executed when no route matches the request.
+	NotFoundHandler HandlerFunc
+
+	// MethodNotAllowedHandler is a handler that is executed when no route with exact METHOD matches the request but
+	// there is a route with same path but different method.
+	MethodNotAllowedHandler HandlerFunc
+
+	// OptionsMethodHandler is a handler that is executed when an OPTIONS request is made.
+	OptionsMethodHandler HandlerFunc
+
+	// AllowOverwritingRoute allows overwriting existing routes. If false, then adding a route with the same method
+	// and path will return an error.
+	AllowOverwritingRoute bool
+
+	// UnescapePathParamValues forces router to unescape path parameter values before setting them in context.
+	UnescapePathParamValues bool
+
+	// UseEscapedPathForMatching forces router to use an escaped path (req.URL.RawPath instead of req.URL.Path) for matching.
+	// Difference between URL.RawPath and URL.Path is:
+	//  * URL.Path is where request path is stored. Value is stored in decoded form: /%47%6f%2f becomes /Go/.
+	//  * URL.RawPath is an optional field which only gets set if the default encoding is different from Path.
 	UseEscapedPathForMatching bool
 }
 
@@ -446,8 +462,16 @@ func newAddRouteError(route Route, err error) *AddRouteError {
 // Add registers a new route for method and path with matching handler.
 func (r *DefaultRouter) Add(route Route) (RouteInfo, error) {
 	if route.Handler == nil {
-		return RouteInfo{}, newAddRouteError(route, errors.New("adding route without handler function"))
+		switch route.Method {
+		case RouteNotFound:
+			route.Handler = r.notFoundHandler
+		case http.MethodOptions:
+			route.Handler = r.optionsMethodHandler
+		default:
+			return RouteInfo{}, newAddRouteError(route, errors.New("adding route without handler function"))
+		}
 	}
+
 	method := route.Method
 	path := normalizePathSlash(route.Path)