Add support for public/private Cache-Control HTTP header [SPR-7129]

[spring-projects-issues]

Zoran Regvart opened SPR-7129 and commented

As per The HTTP 1.1 RFC (http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.1) Cache-Control HTTP header can have keywords public or private specified.

One would use the public keyword for cacheable resources that are behind HTTP authentication -- a common case would be static javascript or image files that not accessible to general public. An even stronger case could be made for JSON/XML data intended for an AJAX client behind HTTP authentication that still wants to cache the response.

The private keyword is intended in cases where privacy of the cached data should be enforced -- for example for cached profile pages of the user.

These improvements would benefit applications that want comply with the recent trends in web site optimizations, such as the ones outlined in the best practices for caching section of Google's page speed project (http://code.google.com/speed/page-speed/docs/caching.html).

---

Affects: 3.0.2

This issue is a sub-task of #16413

Issue Links:

• Provide controller method level alternative to WebContentInterceptor [SPR-8550] #13194 Provide controller method level alternative to WebContentInterceptor
• ServletWebRequest can't validate Etag and Last-Modified together. [SPR-11324] #15948 ServletWebRequest can't validate Etag and Last-Modified together.

Referenced from: commits 38f32e3

11 votes, 23 watchers

[spring-projects-issues]

Matthew Sgarlata commented

To be super clear, if you don't put in public in your response headers, for applications running over HTTPS, then NO STATIC RESOURCES ARE CACHED EVER. They are downloaded every single time by every single browser, including modern ones like Chrome. If you are using a massive JS library like dojo that really sucks.

[spring-projects-issues]

marc schipperheyn commented

This should be implemented in such a way that we have declarative control over this, based on requestmapping patterns

[spring-projects-issues]

marc schipperheyn commented

I would recommend cacheMappings to work like so:

value > 0: cache public x seconds
value = 0: leave caching up to the client
value = -1: don't cache
value < -1: cache private x seconds

[spring-projects-issues]

marc schipperheyn commented

Ok, to be more in-line with the existing code:

value > 0: cache public x seconds
value = 0: don't cache
value = -1: leave caching up to the client
value < -1: cache private x seconds

The code to implement this seems trivial

WebContentGenerator

	protected final void cacheForSeconds(HttpServletResponse response, int seconds, boolean mustRevalidate) {
		if (this.useExpiresHeader) {
			// HTTP 1.0 header
			response.setDateHeader(HEADER_EXPIRES, System.currentTimeMillis() + Math.abs(seconds) * 1000L);
		}
		if (this.useCacheControlHeader) {
			// HTTP 1.1 header
			String headerValue = "max-age=" + Math.abs(seconds);
			if (mustRevalidate) {
				headerValue += ", must-revalidate";
			}
			if(seconds < -1) {
				headerValue += ", private";
			}
			response.setHeader(HEADER_CACHE_CONTROL, headerValue);
		}
	}
	
	protected final void applyCacheSeconds(HttpServletResponse response, int seconds, boolean mustRevalidate) {
		if (seconds > 0 || seconds < -1) {
			cacheForSeconds(response, seconds, mustRevalidate);
		}
		else if (seconds == 0) {
			preventCaching(response);
		}
		// Leave caching to the client otherwise.
	}

[spring-projects-issues]

Steven Willems commented

Maybe it's worth looking at this implementation(by Scott Rossillo): https://github.com/foo4u/spring-mvc-cache-control/tree/master/spring-mvc-cache-control/src/main/java/net/rossillo/spring/web/mvc.

[spring-projects-issues]

marc schipperheyn commented

I think I would prefer the declarative approach. It seems to provide more flexibility if the same code base is used for various sites and provides a central place to look for problems. But perhaps both are possible a declarative approach being superseded by a cacheheader annotation.

[spring-projects-issues]

marc schipperheyn commented

Could we at the very least remove the "final" from cacheForSeconds so we can deal with this use case ourselves if it doesn't get picked up for the next release? Nothing more annoying than non overrideable methods.

[spring-projects-issues]

marc schipperheyn commented

Slight addition

value > 0: cache public x seconds
value = 0: don't cache
value = -1: leave caching up to the client or let controller method override (e.g. through annotation) == default
value < -1: cache private x seconds

[spring-projects-issues]

marc schipperheyn commented

I see that his has now been pushed to general backlog. I really think that not supporting private-cache headers is a major shortcoming of the framework, but regardless, could we at least remove the "final" from the relevant methods as a stopgap measure?

[spring-projects-issues]

Rossen Stoyanchev commented

marc schipperheyn, thanks for bringing this up and suggesting a solution. I think the WebContentInterceptor change can be made independent of any declarative caching, which (when available) can act as an override.

That said, in your solution, the public keyword is always applied (i.e. when cacheSeconds > 0). That would add the public keyword for everyone setting Cache-Control via WebContentInterceptor today and actually via all other sub-classes of WebContentGenerator, especially ResourceHttpRequestHandler. I'm not sure we can just do that. Here is a quote from Google's page speed project referenced above:

Setting the header to public effectively shares resources among multiple users,
which means that any cookies set for those resources are shared as well.
While many proxies won't actually cache any resources with cookie headers set,
it's better to avoid the risk altogether. Either set the Cache-Control header
to private or serve these resources from a cookieless domain.

We may need to make this more configurable so applications can opt-in. In the very least a flag that enables adding either public or private. And then using the approach you suggested. The only other shortcoming I see is that there is no way to specify "cache private 1 seconds".

What do you think? If you have any time at all to put together a pull request, we'll consider that right away.

[spring-projects-issues]

marc schipperheyn commented

I can have a look at implementing this. You're right about the opt-in. Most important is that defaults don't change the current way it works. I'll have to look at header combinations to see what combinations are expected generally.

[spring-projects-issues]

Rossen Stoyanchev commented

My understanding is the public directive allows caching by shared caches (e.g. proxies) whereas caching otherwise would only be done by non-shared caches (e.g. browsers). So there needs to be a way to request a Cache-Control header with all possible options: public, private, no-cache, or none. Looking at the approach of using cacheSeconds (negative vs positive), it wouldn't be possibly to request "cache x seconds", i.e. neither public, nor private.

We'll probably need to introduce an overloaded method in WebContentGenerator, something like:

protected final void checkAndPrepare(HttpServletRequest request, HttpServletResponse response,
    int cacheSeconds, boolean lastModified, String cacheabilityDirective) {

}

where the directive can be "public", "private", or null. In turn the WebContentInterceptor can have a similar property (hence one instance per directive) or it could accept cache mappings that look like this:

/foo=11,private
/bar=22,public
/baz=33

[spring-projects-issues]

marc schipperheyn commented

The performance impact of this change can even be greater if one considers using cache-control private in combination with etags. It would allow you to cache dynamic pages in a static way without worrying about staleness. One has to consider security implications of course. In any case, this would suggest that the "must-revalidate" should be an optional part of the cache-control definition (both public and private)

[spring-projects-issues]

Mauro Molinari commented

+1 for this. I'm not an expert on the subject, but after reading some documentation IMHO the use of "public/private" directives should be allowed independently of the "max-age".
Another thing I miss is the possibility to set the "s-maxage" directive, although it's less important.

Also, although it's the most natural choice, why the "must-revalidate" header inclusion is forced on the response when cacheSeconds > 0 and the handler supports the LastModified interface?

[spring-projects-issues]

Scott Rossillo commented

I wrote a project to manage Cache-Control headers on annotation based controllers and have successfully used it in multiple projects. This is my GitHub Project.

Spring MVC Cache Control is an extension to Spring MVC that aims to simplify implementing HTTP/1.1 Cache-Control headers for annotated MVC controllers.

The project also contains a very simple working sample application. The implementation defines a type and method targetable @CacheControl allowing cache-control directives to be specified at the @Controller level and then tweaked per request on @RequestMapping methods. It supports cache policies as well as max age directives, which can be mixed in any sane configuration. Care was taken to remove the need to HttpServletResponse to be referenced in the controller methods.

Currently, the headers are written by a handler interceptor. I'd be happy to adjust this code as necessary and donate to Spring Source if you're interested.

@CacheControl
@Controller
public final class DemoController {

	/**
	 * Public home page, cacheable for 5 minutes.
	 */
	@CacheControl(maxAge = 300)
	@RequestMapping({"/", "/home.do"})
	public String handleHomePageRequest(Model model) {
		model.addAttribute("pageName", "Home");
		return "page";
	}
	
	/**
	 * Directions page allowing caching for 15 minutes, but requiring 
	 * re-revalidation before serving user a potentially stale resource.
	 */
	@CacheControl(policy = { CachePolicy.MUST_REVALIDATE }, maxAge = 15 * 60)
	@RequestMapping("/directions.do")
	public String handleProducDirectionsRequest(Model model) {
		model.addAttribute("pageName", "Directions");
		return "page";
	}
	
	/**
	 * Personalized accounts page.  Content is private to the user
	 * so it should only be stored in a private cache.
	 */
	@CacheControl(policy = { CachePolicy.PRIVATE, CachePolicy.MUST_REVALIDATE }) 
	@RequestMapping("/account.do")
	public String handleAccountRequest(Model model) {
		model.addAttribute("pageName", "Your Account");
		return "page";
	}
	
	/**
	 * No caches may store the account balance page.
	 */
	@CacheControl(policy = { CachePolicy.NO_STORE })
	@RequestMapping("/balance.do")
	public String handleBalancePageRequest(Model model) {
		model.addAttribute("pageName", "Account Balanace");
		return "page";
	}
	
	/**
	 * About page produces the controller's default Cache-Control header.
	 */
	@RequestMapping("/about.do")
	public String handleItemRequest(Model model) {
		model.addAttribute("pageName", "About");
		return "page";
	}
}

[spring-projects-issues]

Brian Clozel commented

Given the current timetable, this issue has been rescheduled to 4.x backlog but will be addressed as a high priority for the next version.

[spring-projects-issues]

Rossen Stoyanchev commented

Scott Rossillo, thanks for the reference to your project. In addition to this ticket there is also #13194 and we'll look to address both at the same time.

[spring-projects-issues]

marc schipperheyn commented

I really hope that you guys will at least also implement the "configuration approach" as per some of the earlier comments as opposed to the "hardwire in Java" which looks better but is more limiting.

The rationale for this is that a configuration style approach allows you to have different configurations based on a single platform (as is our case). E.g. a web page requires a logged in user (requires private) on one implementation but not on another (public suffices).

[spring-projects-issues]

Brian Clozel commented

Hi marc schipperheyn,

What do you mean by "configuration approach"? Is #13194 what you had in mind?
If so, yes, it is scheduled for 4.1.

[spring-projects-issues]

marc schipperheyn commented

No, in one of my earlier comments I talked about using a style such as used in the WebContentGenerator to express these kinds of configurations. This allows you to manage your settings without hardcoding them at the controller level such as #13194 suggests.

[spring-projects-issues]

Rossen Stoyanchev commented

I agree interceptor-style should be preferred for configuring caching. The main reason for even considering @CacheControl in my mind is to provide a more encapsulated alternative for frameworks or services built on Spring MVC like Spring Social.

I wonder if having an annotation isn't going too far. Perhaps we should change the scope for #13194 to be more along the lines of how we support not-modified with an etag in @MVC controller methods.

[spring-projects-issues]

Brian Clozel commented

Those changes will be available for WebContentGenerator.
Check out #16413, it lists all changes to be made in that space. Feel free to comment there as well if the description is incomplete.

[spring-projects-issues]

Christopher Smith commented

As a note, the ResourceHandlerRegistry#addResourceHandler builder syntax, which was extended in 4.1 with strategy-based resource resolvers, should have an easy mechanism to set Cache-Control: public in addition to max-age (with setCachePeriod). I'm slowly getting cache busting working with my entire pipeline, but there's no way to tell MVC that all those static resources are publicly cacheable.

[spring-projects-issues]

Brian Clozel commented

A new CacheControl builder class allows the "public" and "private" Cache-Control directives.

Such a property can be configured like this for handling resources:

@Override
public void addResourceHandlers(ResourceHandlerRegistry registry) {
    CacheControl cc = CacheControl.maxAge(10, TimeUnit.DAYS)
                                  .cachePublic();
    registry.addResourceHandler("/resources/**")
                .addResourceLocations("classpath:/resources/")
                .setCacheControl(cc);

See #16413 for more details.