search

goadesign / plugins

A collection of plugins for Goa.
https://goa.design
MIT License
68 stars 34 forks source link

CORS OPTIONS should always return 200 #149

Closed gregwebs closed 1 year ago

gregwebs commented 1 year ago

A CORS OPTIONS request is better off always returning 200. For example, if the OPTIONS request is for a url that doesn't exist, then the current code will return a 404. That will result in the browser giving a network error on the actual request. Whereas if a 200 was returned, the browser will make the actual request and get an actual 404 result. Similar for a 401 that is derived from middleware, etc.

I did not test this change. Let me know if it makes sense and I can look at testing.

raphael commented 1 year ago

That makes sense but I am not sure the change is quite right as the flow that's being modified by this commit is not only for pre-flight requests, it's also used for standard requests that should go through (with the added CORS headers).

gregwebs commented 1 year ago

Yeah, I am not sure that this change is correct either. I do not yet fully understand the codegen here.

gregwebs commented 1 year ago

Maybe one change needed would be instead of building up PreflightPaths to use a wildcard? It's really difficult to follow how all this generation works.

raphael commented 1 year ago

I think it needs to be in the if above since the Access-Control-Request-Method header is only present during preflight requests. The code will need to return too.

gregwebs commented 1 year ago

new pull requests is here: https://github.com/goadesign/plugins/pull/150

I think there is much more code gen going on that is necessary. The DSL can generate just a struct. That struct can then be an argument to normal functions that can exist as Go code rather than string templates. That also makes it easy for others to write their own code that uses the struct. The lots of templating approach is generating slightly more efficient code, but it creates a huge development and testing burden.