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

When CORS is turned on, Vary: Origin header gets overwritten by Vary: Accept-Encoding if request includes Accept-Encoding header #802

Closed
albertchae opened this issue Apr 11, 2022 · 2 comments · Fixed by #803
Assignees
Labels

Comments

@albertchae
Copy link
Contributor

Describe the bug

CORS requests should have the Vary header set to Origin if the server specifies a single origin (that may dynamically change based on the requesting origin as part of an allowlist). This header is set correctly thanks to https://github.com/go-chi/cors/blob/9b0b248d5e6ba10c954f076a98c5f7760f243882/cors.go#L242-L247

However, if the request has the Accept-Encoding header set, the Vary header gets overwritten in old versions of go-chi/chi because of this line https://github.com/go-chi/chi/blob/86f9a6e7ce9bf453eaa339b51f88f586edbccbc1/middleware/compress.go#L321

Version Info

I'm not sure how to run flipt --version in my local environment since I've been using task dev, but it's happening on the latest master commit 8a63fbb

To Reproduce

  1. Enable CORS in flipt with specific hosts, e.g. change https://github.com/markphelps/flipt/blob/8a63fbbb050cb286d90042645c40806a32b80598/config/local.yml#L7-L9 to
cors:
  enabled: true
  allowed_origins: "http://localhost"
  1. Start the server
  2. curl -i --request GET --url 'http://localhost:8080/api/v1/flags/test' --header 'Origin: http://localhost' returns
Vary: Origin
  1. curl -i --request GET --url 'http://localhost:8080/api/v1/flags/test' --header 'Accept-Encoding: gzip, br' --header 'Origin: http://localhost' returns
Vary: Accept-Encoding

Expected behavior

I would expect the curl in step 4 to return either

Vary: Accept-Encoding, Origin

or

Vary: Accept-Encoding
Vary: Origin

Additional context

This bug was fixed in go-chi/chi#640 which hasn't been released yet, so I have 2 proof of concept solutions, neither of which I really like:

  1. Update go-chi/chi directly to the commit hash with the fix albertchae@4eb7e45
  2. Add a custom handler that'll add Vary: Origin after go-chi/chi overwrites it. I don't really know go so I mostly copied an online example but I verified I see the header correctly after (one flaw with this currently is that if Accept-Encoding is not present in the request, this adds Vary: Origin twice) albertchae@59e1c37
@markphelps
Copy link
Collaborator

markphelps commented Apr 11, 2022

Thanks for the bug report @albertchae and awesome work digging in and documenting the reproduction steps!

I think I'd prefer your solution 1, just to update to a non-release commit hash. It seems Chi hasnt released a new version in 6 months, but hopefully they will soon. In the mean time I'm good with updating to the specific hash.

Feel free to send a PR for albertchae@4eb7e45 and I'll merge it.

Thanks again!

@albertchae
Copy link
Contributor Author

@markphelps got it, thanks. Opened #803

And thanks again for a great project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants