Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

middleware.CORSConfig can not be configured per path #2620

Closed
3 tasks done
rpstw opened this issue Apr 8, 2024 · 3 comments
Closed
3 tasks done

middleware.CORSConfig can not be configured per path #2620

rpstw opened this issue Apr 8, 2024 · 3 comments

Comments

@rpstw
Copy link

rpstw commented Apr 8, 2024

Issue Description

The following code tris to configure CORS per path.However, due to optionsMethodHandler ignoring all configured middlewares, a preflight request will receive a bad response which contains no Access-Control-Allow headers.

e.Add(
	http.MethodGet,
	"/some/path",
	SomeHandlerFunc,
	middleware.CORSWithConfig(middleware.CORSConfig{
		AllowCredentials: true,
	}),
)

Checklist

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

Expected behaviour

curl -vL -X OPTIONS '10.20.0.23:3323/some-path?' -H Origin:https://some-origin.com
> OPTIONS /some-path HTTP/1.1
> Host: 10.20.0.23:3323
> User-Agent: curl/7.68.0
> Accept: */*
> Origin:https://some-origin.com
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 204 No Content
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Headers: Some-Headers
< Access-Control-Allow-Methods: GET,HEAD,PUT,PATCH,POST,DELETE
< Access-Control-Allow-Origin: https://some-origin.com
< Vary: Origin
< Vary: Access-Control-Request-Method
< Vary: Access-Control-Request-Headers
< Date: Mon, 08 Apr 2024 06:29:02 GMT
<
* Connection #0 to host 10.20.0.23 left intact

Actual behaviour

curl -vL -X OPTIONS '10.20.0.23:3323/some-path' -H Origin:https://some-origin.com
*   Trying 10.20.0.23:3323...
* TCP_NODELAY set
* Connected to 10.20.0.23 (10.20.0.23) port 3323 (#0)
> OPTIONS /some-path HTTP/1.1
> Host: 10.20.0.23:3323
> User-Agent: curl/7.68.0
> Accept: */*
> Origin:https://some-origin.com
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 204 No Content
< Allow: OPTIONS, GET
< Date: Mon, 08 Apr 2024 06:37:17 GMT
<
* Connection #0 to host 10.20.0.23 left intact

Steps to reproduce

As described above

Working code to debug

As described above

Version/commit

tried 4.11.4 and 4.10.2

Workarounds tried

As #2039 inspired, configuring a custom OPTIONS route can bypass the code where optionsMethodHandler ignoring middlewares

e.Add(
	http.MethodOptions,
	"/some-path",
	func(c echo.Context) error {
		c.Response().Header().Add(echo.HeaderAllow, "GET,OPTIONS")
		return c.NoContent(http.StatusNoContent)
	},
	middleware.CORSWithConfig(middleware.CORSConfig{
		AllowCredentials: true,
	}),
)

I'm not sure if I'm configuring CORS in a popular doable way, also the reproducible example is excerpted from a development environment which may not be as realistic as a real prod server case. Any suggestions are welcome.

@jub0bs
Copy link

jub0bs commented Apr 23, 2024

@rpstw Note that curl -vL -X OPTIONS '10.20.0.23:3323/some-path?' -H Origin:https://some-origin.com does not correspond to a CORS-preflight request: it's missing an Access-Control-Request-Method header. Therefore, I wouldn't expect a properly implemented CORS middleware to include CORS headers in a response to such a request.

As for applying different CORS middleware on different routes, I believe that Echo allows you to do that. This example uses a different CORS library, but you should be able to adapt it to your needs.

@aldas
Copy link
Contributor

aldas commented Apr 24, 2024

When you add GET route with CORS middleware with

e.Add(
	http.MethodGet,
	"/some/path",
	SomeHandlerFunc,
	middleware.CORSWithConfig(middleware.CORSConfig{
		AllowCredentials: true,
	}),
)

Echo router registers only node matching GET method in its internal routing tree. So when you issue OPTION method call to same path the CORS middleware do not get executed as it is tied (wrapping it) to GET handler. So CORS middleware is best to be used with e.Use() by it to all routes or registering .OPTIONS("path"... to be able to match only some of the requests

p.s. if choose to register own OPTIONS routes you can use echo.MethodNotAllowedHandler handler function

echo/echo.go

Line 356 in 88c379f

var MethodNotAllowedHandler = func(c Context) error {

@rpstw
Copy link
Author

rpstw commented May 16, 2024

Thanks for the explanation.

@rpstw rpstw closed this as completed May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants