router not setting ContextKeyHeaderAllow for group, causing cors preflight to not work

[aakil786]

Issue Description

Checklist

• Dependencies installed
• No typos
• Searched existing issues and docs

Expected behaviour

curl --location --request OPTIONS 'http://localhost:8092/api/test'
--data ''

should return
Allow: OPTIONS, POST
Vary: Origin
Date: Fri, 05 Jan 2024 12:42:24 GMT

Actual behaviour

returns

Vary: Origin
Date: Fri, 05 Jan 2024 12:42:24 GMT

not reaching this code in router

			ctx.Set(ContextKeyHeaderAllow, currentNode.methods.allowHeader)

instead it is going into this code above

	if matchedRouteMethod != nil {
		rPath = matchedRouteMethod.ppath
		rPNames = matchedRouteMethod.pnames

Steps to reproduce

Working code to debug

package main

import (
	"github.com/labstack/echo/v4"
	"github.com/labstack/echo/v4/middleware"
	"net/http"
)

func main() {
	e := echo.New()
	e.HideBanner = true
	e.Use(middleware.CORSWithConfig(middleware.CORSConfig{
		AllowOrigins: []string{"*"},
		AllowHeaders: []string{echo.HeaderOrigin, echo.HeaderContentType, echo.HeaderAccept},
	}))
	e.Use(middleware.Logger())
	e.Use(middleware.Recover())
	e.GET("/", func(c echo.Context) error {
		return c.String(http.StatusOK, "Hello, World!")
	})
	api := e.Group("/api", nil)
	api.POST("/test", func(c echo.Context) error {
		return c.String(http.StatusOK, "Hello, Test!")
	})
	e.Logger.Fatal(e.Start(":8092"))
}

Version/commit

v4.11.3 and v4.11.4 (only 2 tested)

[aldas]

Hi, sorry for late reply but I think problem is not with router. You example is missing origin header. CORS preflight request must have origin header.

If there would be origin header the response would be:

x@x:~/$ curl --location --request OPTIONS 'http://localhost:8092/api/test' -v -H "Origin: http://test.test" 
* processing: http://localhost:8092/api/test
*   Trying [::1]:8092...
* Connected to localhost (::1) port 8092
> OPTIONS /api/test HTTP/1.1
> Host: localhost:8092
> User-Agent: curl/8.2.1
> Accept: */*
> Origin: http://test.test
> 
< HTTP/1.1 204 No Content
< Access-Control-Allow-Headers: Origin,Content-Type,Accept
< Access-Control-Allow-Methods: GET,HEAD,PUT,PATCH,POST,DELETE
< Access-Control-Allow-Origin: *
< Vary: Origin
< Vary: Access-Control-Request-Method
< Vary: Access-Control-Request-Headers
< Date: Tue, 06 Feb 2024 06:06:27 GMT
< 
* Connection #0 to host localhost left intact

[aldas]

but I do admit that Access-Control-Allow-Methods: GET,HEAD,PUT,PATCH,POST,DELETE means that router actually matches 404 route /api/* that is being added with api := e.Group("/api", nil) line. that last nil makes it to add these additional routes.

but nil as middleware makes your other requests panic

For example this panics:

curl --location --request POST 'http://localhost:8092/api/test' -v -H "Origin: http://test.test"

[jub0bs]

@aakil786 Somewhat related: #2534

[aakil786]

Ok yes removing the nil (for middleware) solves the problem, with my test code sample. My actual code however does not pass a nil but uses a keycloak middleware, and I still have a problem. Apologies for my poor sample.

[aakil786]

Ok it looks like the issue is when passing a middleware to the group.

package main

import (
	"github.com/labstack/echo/v4"
	"github.com/labstack/echo/v4/middleware"
	"net/http"
	"time"
)

func main() {
	e := echo.New()
	e.HideBanner = true
	e.Use(middleware.CORSWithConfig(middleware.CORSConfig{
		AllowOrigins: []string{"*"},
		AllowHeaders: []string{echo.HeaderOrigin, echo.HeaderContentType, echo.HeaderAccept},
	}))
	e.Use(middleware.Logger())
	e.Use(middleware.Recover())
	e.GET("/", func(c echo.Context) error {
		return c.String(http.StatusOK, "Hello, World!")
	})
	api := e.Group("/api", CustomLogger())
	api.POST("/test", func(c echo.Context) error {
		return c.String(http.StatusOK, "Hello, Test!")
	})
	e.Logger.Fatal(e.Start(":8092"))
}

// CustomLogger logs the start and end time of each request
func CustomLogger() echo.MiddlewareFunc {
	return func(next echo.HandlerFunc) echo.HandlerFunc {
		return func(c echo.Context) error {
			start := time.Now()
			if err := next(c); err != nil {
				c.Error(err)
			}
			end := time.Now()
			latency := end.Sub(start)
			c.Logger().Printf("---->>>> Latency for %s: %s", c.Request().URL, latency.String())

			return nil
		}
	}
}

curl --location --request OPTIONS 'http://localhost:8092/api/test' -v -H "Origin: http://test.test"

returns the following when a middleware is passed in

< HTTP/1.1 204 No Content
< Access-Control-Allow-Headers: Origin,Content-Type,Accept
< Access-Control-Allow-Methods: GET,HEAD,PUT,PATCH,POST,DELETE
< Access-Control-Allow-Origin: *
< Vary: Origin
< Vary: Access-Control-Request-Method
< Vary: Access-Control-Request-Headers
< Date: Thu, 25 Apr 2024 14:34:43 GMT

when the middleware is removed then it returns

< HTTP/1.1 204 No Content
< Access-Control-Allow-Headers: Origin,Content-Type,Accept
< Access-Control-Allow-Methods: OPTIONS, POST
< Access-Control-Allow-Origin: *
< Allow: OPTIONS, POST
< Vary: Origin
< Vary: Access-Control-Request-Method
< Vary: Access-Control-Request-Headers
< Date: Thu, 25 Apr 2024 14:36:26 GMT

[aldas]

I'm quite sure this caused by hidden routes that group middlewares add. This is very old hack to make sure that group middlewares are applied to 404 cases. Which we can not remove as there are plenty of people "using" that feature.

echo/group.go

Lines 22 to 33 in 88c379f

	func (g *Group) Use(middleware ...MiddlewareFunc) {
	g.middleware = append(g.middleware, middleware...)
	if len(g.middleware) == 0 {
	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.
	g.RouteNotFound("", NotFoundHandler)
	g.RouteNotFound("/*", NotFoundHandler)
	}