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

Support for HTTP Vary configuration (e.g. in reaction to locale-based rendering) [SPR-14070] #18642

Closed
spring-projects-issues opened this issue Mar 18, 2016 · 13 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Mar 18, 2016

Markus Malkusch opened SPR-14070 and commented

An i18n webapplication might use the AcceptHeaderLocaleResolver do deliver content in different languages based on the accept-language HTTP header.

To inform user agents and cache proxies about that kind of content negotiation, HTTP provides the VARY header.

Vary: accept-language

I suggest that Spring transparently sets an appropriate VARY header to the HTTP response if an AcceptHeaderLocaleResolver bean is used.

Also I don't see a point in using AcceptHeaderLocaleResolver without a VARY header. In fact applications doing so are broken, as an intermediate cache proxy might deliver a cached response of a wrong language. So I further suggest setting the VARY header should be a default behaviour (if AcceptHeaderLocaleResolver is used). That's also the RFCs wording as well:

An origin server SHOULD send a Vary header field when its algorithm
for selecting a representation varies based on aspects of the request
message other than the method and request target, unless the variance
cannot be crossed or the origin server has been deliberately
configured to prevent cache transparency.

Settings the VARY header appropriatly means, taking into account that the response might have such a header already. In this case, it needs to be determined if adding accept-language is necessary. It's not necessary, if the existing header is "*" or contains already "accept-language".


Reference URL: https://tools.ietf.org/html/rfc7231#section-7.1.4

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 19, 2016

Markus Malkusch commented

Oh, and I see that all other locale resolvers also do content negotiation based on the accept-language header, sometimes.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 19, 2016

Juergen Hoeller commented

Hmm, I'm wondering whether we should set the "Vary" header to "accept-language" by default then, at least when also sending a cache control header? Even without an explicit LocaleResolver setup, the request locale is going to matter in typical MVC arrangements... even for data loading in the controller, but in particular in the views.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2016

Rossen Stoyanchev commented

Yes I was also thinking this should be done in sync with cache header settings rather than despite them. So perhaps WebContentGenerator might be the place to do this possibly leaving the option to make it configurable.

/cc Brian Clozel

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2016

Brian Clozel commented

Hi Markus Malkusch,

Could you tell us more about the particular use case you're considering here?

  1. Are those static resources? HTML responses? REST API resources?
  2. How is the response varying with the language? Is it always depending on the Accept header or can also depend on session information or the URL itself?
  3. How are you configuring the HTTP Caching behavior in Spring for this?

At that point, I think that this should be a conscious choice by the application developer, since the application specifics and the caching configuration may be quite different. Let's say the language for the resource is itself defined in the resource URL - adding this header would create artificial cache misses.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2016

Juergen Hoeller commented

I am also strongly in favor of explicitness here. A LocaleResolver setup (in particular the default AcceptHeaderLocaleResolver setup) does not mean that any specific web responses actually vary by language; we need an explicit hint from the application's side here.

Since 4.2's cache control support is already explicit through the CacheControl handle, as discussed on our team call today, I suggest the addition of Vary support right there: possibly with first-class varyByLanguage(), varyByUserAgent(), varyByCookie() methods on CacheControl, and an escape hatch varyBy(String...) for arbitrary HTTP headers to be specified. All of those would add Vary conditions, not replace them, in contrast to Cache-Control where we actually replace the setup. This leaves Vary: Accept-Encoding up to the Servlet container, and custom Vary conditions up to Filters etc.

In terms of obtaining the Vary specification in WebContentGenerator and ResponseEntity, a simple String[] getVaryValues() method on CacheControl would do the job, with the resulting values individually added through HttpServletResponse.addHeader or HttpHeaders.add, respectively. On Servlet 3.0+, we may also defensively check for the presence of corresponding header values through HttpServletResponse.getHeaders first, not re-adding them in that case.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2016

Markus Malkusch commented

Could you tell us more about the particular use case you're considering here?

It's an HTTP resource which does always content negotiation based on the accept-language header. Concrete: It's an HTML response derived from message properties. I do not cache all resources.

Also I would not recommend to couple the Vary header to the caching behaviour. The RFC very clearly says, that vary SHOULD be send when content negotiation happens. Issues with caching is just one example. Caching is personally not my first concern. I want that user agents know that there exists another representation of the resource. By user agent I also mean a machine like GoogleBot.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2016

Juergen Hoeller commented

Markus Malkusch, while I generally agree about the role of Vary, I still see benefits in first-class support within our Cache-Control arrangement... not least of it all as a matter of guidance: if you're specifying cache conditions, don't forget to consider vary hints.

For the use of Vary on general resources, why not simply set the header through ResponseEntity? Admittedly, we could define a first-class vary(String...) method on the HeadersBuilder there for such content representation hints. At that point, we could also leave that general method on ResponseEntity that way, with our CacheControl API just specifying the pre-arranged varyByLanguage(), varyByUserAgent(), varyByCookie() methods which are typically used for caching purposes... and which fit pretty well with CacheControl's general design style.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2016

Rossen Stoyanchev commented

The RFC very clearly says, that vary SHOULD be send when content negotiation happens.

This is the reason why, as Juergen pointed out (your comments might have crossed), we can't automatically set the Vary header. Not only is the AcceptHeaderLocaleResolver used by default and requires no explicit configuration, but also its use to resolve the preferred locale is not an indication of actual variance by language in the response, case in point serving static resources.

It's an HTTP resource which does always content negotiation based on the accept-language header. Concrete: It's an HTML response derived from message properties. I do not cache all resources.

How do you actually serve those resources? Is there a controller method involved and if so is it @ResponseBody, view name, etc.?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2016

Markus Malkusch commented

How do you actually serve those resources? Is there a controller method involved and if so is it @ResponseBody, view name, etc.?

A controller method is involved which returns a view name.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2016

Rossen Stoyanchev commented

Okay I think WebContentInterceptor would be a good fit. It can be configured by URL pattern and while it's commonly used for cache settings, caching can be configured on or off (or somewhere in between), and the interceptor is not limited to cache settings strictly. Exposing Vary header settings there next to caching makes sense.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2016

Juergen Hoeller commented

Rossen Stoyanchev, I guess we'll introduce setVary(String...) on WebContentGenerator then, making it implicitly available through WebContentInterceptor, RequestMappingHandlerAdapter and also the Servlet wrappers... and for feature parity also on ResponseEntity, as a vary(String...) builder method?

Would you still also have it on CacheControl as specific varyByXX triggers? Or should we rather leave it at the general support in WebContentGenerator?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 22, 2016

Rossen Stoyanchev commented

Would you still also have it on CacheControl as specific varyByXX triggers?

I'd vote to keep it separate in WebContentGenerator and ResponseEntity and leave CacheControl with a single purpose. They are still co-located for consideration together and cross-references in the Javadoc can help further. We don't need to introduce it for ResourceHttpRequestHandler where CacheControl is currently accepted so I guess that's another argument they don't always go together.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 22, 2016

Juergen Hoeller commented

Sounds good to me, Rossen Stoyanchev!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants