responseHeaders.add(HttpHeaders.VARY, HttpHeaders.ORIGIN);
in the middle of handleInternal()
In the case where the wildcard domain is supported (CorsConfiguration.ALL), this response header does not seem necessary. If all domains are supported, the response does not actually vary by requested domain/origin. Further, this implementation does not seem to provide any reasonably simple way to override this functionality.
An example of where this can become an issue is for pages being reverse proxied through a CDN that will not cache responses containing Vary headers (Akamai, for example, will deem a response uncacheable if the Vary header contains anything other than Accept-Encoding).
In the simplest case, this solves the problem:
if (!CorsConfiguration.ALL.equals(allowOrigin))
    responseHeaders.add(HttpHeaders.VARY, HttpHeaders.ORIGIN);
Affects: 4.2.3, 4.2.4, 4.3 GA
0 votes, 5 watchers
          
Daniel Robert commented
Actually, I may be wrong on at least part of this. Because CORS processing is bypassed entirely for same-origin requests (CORS doesn't factor in) the response is still variable by incoming Origin header (same-origin vs any-other-origin), in which case the current functionality is correct.
That said, despite the fact that I still need to force the Access-Control-Allow-Origin: * response header for both cross- and same-origin responses, the ability to toggle the Vary header feature above is still necessary for me. However my concerns may be unique enough that this isn't worth patching.
          
Sébastien Deleuze commented
As you noticed, CORS processing is skipped for same origin requests, so in that case Vary header is not set.
Another important point to notice is that even if you configure allowed origins to *, when allowCredentials = true which is the default configuration, the Access-Control-Allow-Origin response header set in practice is the origin specified in the request Origin header, because as specified in CORS specification "* cannot be used for a resource that supports credentials".
That said, it seems to me that your proposal could make sense for cross origin request when allowCredentials = false. Any thoughts? Would that match with your use case?
          
Jon Wingfield commented
Hi everyone.  This is causing issues for us after updating to Spring 4.3. We are serving fonts using registry.addResourceHandler, and when used in combination with CORS, IE breaks because the Vary: Origin  header is set (see http://stackoverflow.com/questions/7748140/font-face-eot-not-loading-over-https).  This wasn't an issue in 4.2.
I know this isn't a very common case, but currently we're having to dig into the entrails of CorsProcessor and SimpleUrlHandlerMapping to override the addition of that header.
Ideally, when setting Access-Control-Allow-Origin: *, CorsProcessor wouldn't add the Vary: Origin header.
          
Sébastien Deleuze commented
Hi Jon Wingfield,
Based on my current understanding, I don't think we should prevent adding Vary: Origin header when Access-Control-Allow-Origin: \* because as explained previously when allowCredentials = true we have to set the actual origin instead of * + that may cause issues with CDN when some clients use CORS and some others don't.
That said, I understand that the IE bug you try to workaround is annoying, and maybe we could try to see how to make that easier to customize the CorsProcesser. Before going further, could you double check this wasn't an issue in 4.2 (since I don't think we change CORS behavior between 4.2 and 4.3)?
          
Jon Wingfield commented
@sdeleuze Sorry for my late response :(
I'm fairly certain this resulted from a change from 4.2->4.3, as bisecting against a previous commit before our update to 4.3 showed that everything was working.  There are a few issues fixed in the 4.3.1 release related to CORS: https://jira.spring.io/secure/ReleaseNote.jspa?projectId=10000&version=15568.
I'll post some feedback this week on what would make customization of the CorsProcessor easier.
          
Nils Goroll commented
A Vary response header is required whenever the response varies on the value of a request header, whereby the lack of the respective header also counts as a variant.
The fact that CORS is dramatically inefficient with respect to http caching is a sad story, which I've whined about here: https://lists.w3.org/Archives/Public/public-webappsec/2014Apr/0055.html . I'd summarize Anne's response as "it is how it is now and we won't change it", which I find disappointing. There were zero responses to my constructive suggestion for improvement.
IMHO this issue should be closed as invalid, sending a Very: Origin response header whenever the Origin request header could influence the response is correct, irrespective of whether or not it was actually present.
          
Michael Weber commented
One more note on this.

When using a filter based CORS configuration as described here https://spring.io/blog/2015/06/08/cors-support-in-spring-framework#filter-based-cors-support the CORS processing is skipped entirely in case no Origin header was given within the request.

This in turn leads to a missing "Vary : Origin" response header which in not correct (as outlined in a previous comment by @Nils Goroll).
@see: org.springframework.web.filter.CorsFilter
An attacker aware of this bug could trigger non-ajax requests (requests with missing origin headers) to controllers that are meant to only serve ajax-request. The response might be cacheable. The cacheable version is not marked as "vary: origin" and would be delivered to ajax-requests in the future, which might be wrong and most likely is as the client needs the correct CORS headers in order to process the response.