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

Allow All Origin header not added in the response #132

Open
Sannan-Dubizzle opened this issue Dec 19, 2023 · 8 comments
Open

Allow All Origin header not added in the response #132

Sannan-Dubizzle opened this issue Dec 19, 2023 · 8 comments

Comments

@Sannan-Dubizzle
Copy link

Sannan-Dubizzle commented Dec 19, 2023

I am trying to use this for my gin service.

r := gin.Default()
r.Use(cors.Default())

//routing here

r.run(3000)

but my response do not have any header allowing all origins

@jub0bs
Copy link

jub0bs commented Dec 22, 2023

Please post a minimal reproducible example.

@idc77
Copy link

idc77 commented Feb 20, 2024

Almost same here

		r := gin.Default()
		cfig := cors.DefaultConfig()
		cfig.AllowAllOrigins = true
		cfig.AddAllowHeaders("authorization")
		r.Use(cors.New(cfig))

In my case,
the route was defined as
/api/v1/recipe/

but I POSTed to

/api/v1/recipe

which lead to a 307 redirect and no CORS being sent.

@dbhoot
Copy link
Contributor

dbhoot commented Feb 23, 2024

Does your request contain an origin header? The cors middleware exits early when there is no origin on the request.

@jub0bs
Copy link

jub0bs commented Feb 23, 2024

@idc77 Please post a minimal reproducible example, including server and client code. Otherwise, it's all speculations.

@jjhuff
Copy link

jjhuff commented Feb 26, 2024

Does your request contain an origin header? The cors middleware exits early when there is no origin on the request.

I'm curious about that early bailout. When you have a mix of CORS (Origin header) and non-CORS requests and a CDN, I think you can run into trouble. See here: https://fetch.spec.whatwg.org/#cors-protocol-and-http-caches

https://github.com/rs/cors seems to take the path of always setting Vary: Origin, which I'm not a fan of since it reduces the effectiveness of the CDN when you have lots of different origins.

@jub0bs
Copy link

jub0bs commented Feb 27, 2024

@jjhuff You cannot guarantee that only requests participating in the CORS protocol will hit the resource of interest. Therefore, the CORS middleware must cater for non-CORS requests also, by letting them pass through (hence the "early bailout").

As for cache effectiveness, if at least two discrete origins are allowed in your CORS configuration, the cache middleware needs to specify Vary: Origin; otherwise, cache poisoning becomes a possibility. However, if a single origin or all origins (via the wildcard) are allowed, the CORS middleware can implement this special case without the need to include a Vary: Origin header in responses:

if Access-Control-Allow-Origin is set to * or a static origin for a particular resource, then configure the server to always send Access-Control-Allow-Origin in responses for the resource — for non-CORS requests as well as CORS requests — and do not use Vary.

Neither rs/cors (as you pointed out) nor gin-contrib/cors implement the special case as described by the quote above, but other CORS-middleware libraries do, specifically for better cache effectiveness.

@jub0bs
Copy link

jub0bs commented Mar 8, 2024

@jjhuff But you're right that systematically including Vary: Origin in responses that contain Access-Control-Allow-Origin: * is not ideal for cache effectiveness.

@jjhuff
Copy link

jjhuff commented Mar 8, 2024

@jub0bs I ended switching to fcors for that reason :) Working great!

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

5 participants