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

CrossOriginFilter should add Vary: Origin even for non-CORS requests #1927

Closed
carey opened this issue Oct 29, 2017 · 4 comments
Closed

CrossOriginFilter should add Vary: Origin even for non-CORS requests #1927

carey opened this issue Oct 29, 2017 · 4 comments
Assignees

Comments

@carey
Copy link

carey commented Oct 29, 2017

I've run into an issue with the CrossOriginFilter with fonts which might be retrieved from the same host with no Origin request header, or from a different host with an Origin header in the request.

If a response is cached for the former case, then future cross-origin requests will use the response without a Vary response header. Because this cached response doesn't include an Access-Control-Allow-Origin header, the cross-origin request fails. If all responses include Vary: Origin, then the absence of the Origin header in the request should be included in the cache key, and a subsequent request with an Origin should go to Jetty.

I can see that there might be some issues with backwards compatibility if the filter is applied too broadly, so maybe this would need to use a new configuration option if implemented.

@gregw
Copy link
Contributor

gregw commented Oct 30, 2017

@sbordet I can't see why fixing this should cause backward compatibility issues? Can you?

We need to set Vary: Origin if the origin is null or if we do not match the origin... which makes me struggle to think when we should not set Vary: Origin, as even if we accept all origins, the absence of origin header is significant. Perhaps this filter should always set it for any request that it applies to?

@sbordet
Copy link
Contributor

sbordet commented Oct 30, 2017

See also #1053.

@carey
Copy link
Author

carey commented Oct 30, 2017

About backwards compatibility, I was thinking of a hypothetical case where someone mapped the filter to /*, which would look like it was working as intended. In this case, starting to send Vary: Origin for the entire application might have unintended side effects.

It's a bit of an edge case, though, and might be better dealt with in release notes.

@sbordet
Copy link
Contributor

sbordet commented Feb 12, 2019

Fixed in #3346.

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

4 participants