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

breaking change with returned Access-Control-Allow-Origin header #117

Closed
cainlevy opened this issue Nov 10, 2017 · 4 comments
Closed

breaking change with returned Access-Control-Allow-Origin header #117

cainlevy opened this issue Nov 10, 2017 · 4 comments
Assignees

Comments

@cainlevy
Copy link
Contributor

cainlevy commented Nov 10, 2017

Prior to #116, specifying AllowedOriginValidator acted as an override to AllowedOrigins. Afterwards, the same configuration will return Access-Control-Allow-Origin: * because of the default value for allowedOrigins.

// before: will only reflect allowed origins
// after: will reflect "*"
gorilla.CORS(
  gorilla.AllowedOriginValidator(myValidator)
)

Fixing this requires specifying a blank AllowedOrigins to override the default value:

gorilla.CORS(
  gorilla.AllowedOrigins([]string{})
  gorilla.AllowedOriginValidator(myValidator)
)
@cainlevy
Copy link
Contributor Author

I think the difference between gorilla/handlers and the other examples listed in #116 is that gorilla/handlers treated AllowedOriginValidator as an override and ignored the default * configuration.

It should be possible to specify AllowedOriginValidator(myValidator) alongside AllowCredentials(). It should not be possible to specify AllowedOrigins([]string{"*"}) alongside AllowCredentials().

@faryon93
Copy link

faryon93 commented Dec 1, 2017

+1

@kisielk
Copy link
Contributor

kisielk commented Dec 1, 2017

@elithrar thoughts on this one? It's been a while since I've looked at CORS details

@elithrar elithrar self-assigned this Dec 1, 2017
@elithrar
Copy link
Contributor

elithrar commented Dec 1, 2017

Let me take a look this weekend & assess.

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

No branches or pull requests

4 participants