Issue Details (XML | Word | Printable)

Key: SPR-4930
Type: Improvement Improvement
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Juergen Hoeller
Reporter: Darren Davison
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Spring Framework

@SessionAttributes cache control too severe for MSIE

Created: 18/Jun/08 10:00 AM   Updated: 24/Jul/08 09:37 AM   Resolved: 24/Jul/08 09:37 AM
Component/s: SpringWEB
Affects Version/s: 2.5.4
Fix Version/s: 2.5.6

Time Tracking:
Not Specified

File Attachments: 1. Text File SPR-4930.patch (5 kB)



 Description  « Hide

We have a controller that uses @SessionAttributes, and as such Spring always adds HTTP headers to prevent caching (see AnnotationMethodHandlerAdapter.handle() )

This is causing us a problem in that MSIE users invoke the controller, click links on the page generated by it, and then get the "This page has expired" warning from IE if they hit their back button. Interestingly, if the HTTP headers are set to expire after 1 second, MSIE happily shows the user the page from cache, even though it really HAS expired according to the protocol, so there's definitely (yet another) IE fault at play here too. Firefox and Safari don't exhibit this issue at all.

There is no way to override this behaviour in the AnnotationMethodHandlerAdapter class at present, so I'm wondering if the default behaviour can be made more flexible? Ideally this would be by way of an attribute on the @RequestMapping (or maybe a new annotation) which could be read by the handler adapter. If no value is supplied, it defaults to 0, thus preserving current system behaviour and backwards compatibility, otherwise it specifies the cache seconds to use.

Perhaps adding such an attribute to the RequestMapping/new annotation would provide more benefit in finer grained cache control for annotated controllers, falling back to that set on the handler adapter.

I'll write and test the patch if any of that sounds like it might be acceptable.

Or perhaps there's already some way to handle this that I missed..?



Darren Davison added a comment - 18/Jun/08 05:23 PM

attached is a patch that creates a simple @CacheControl annotation (permitting a single attribute specifying seconds to cache) and corresponding changes to AnnotationMethodHandlerAdapter to account for it.

It's not polished and duplicates a couple of lines of code from the invokeHandlerMethod() method which could be refactored.

I compiled this and tested in a deployment of our project code, using the annotation at both class and method level with expected results.

Is it feasible to get something like this into 2.5.5?


Juergen Hoeller added a comment - 03/Jul/08 09:22 AM

Darren, I can't really reproduce this behavior: I can't get IE to show me a "page has expired" page. Is this happening for you for any page rendered by an @SessionAttributes controller, i.e. when navigating to another page and then returning to the original one?

Making the behavior for @SessionAttributes controllers configurable is a good idea in any case, even if pages generated by those should never really be cached by a proxy or browser. I'll have a look at that for 2.5.6.

Juergen


Darren Davison added a comment - 03/Jul/08 01:35 PM

hi Juergen.. I can't say for sure that it affects any page, but we replicated the issue our users were reporting during UAT phases. We then proved that the behaviour was due to caching headers (by manipulating the HTTP streams between server and client with tools like fiddler), and then figured out by tracing logs and step-debugging that the @SessionAttributes annotation was the cause of the caching headers being set to those values that triggered the undesirable behaviour.

So we didn't investigate any further than that, but when I used a version of Spring compiled with the attached patch, and added @CacheControl annotations alongside @SessionAttributes in the controllers, the issue went away and we got desired behaviour in all browsers.

I could try to produce a simple test case web-app, but I think the annotation has merit in itself as you say. It could take additional attributes to exercise full control over all values for the Pragma, Expires and Cache-Control HTTP headers which would be really powerful I think. Happy to develop that patch further in this direction if you like.

Cheers.


Juergen Hoeller added a comment - 24/Jul/08 09:37 AM

For the time being (2.5.6), I've added a "cacheSecondsForSessionAttributeHandlers" property to AnnotationMethodHandlerAdapter, allowing to override the default value of 0 there. The headers used can be configured through WebContentGenerator's boolean properties, as inherited by AnnotationMethodHandlerAdapter, just like for the standard "cacheSeconds" setting. I suppose this should solve the basic problem that you experienced, without a need to patch?

I still think that a dedicated @CacheControl header is a worthy addition as well. However, that's rather a 3.0 thingie, in particular since we'll be tackling close HTTP integration in the context of our REST theme there anyway. This is likely to be restricted to cache seconds control, though, since the headers can be customized at the AnnotationMethodHandlerAdapter level anyway... I don't really see a need for customizing the HTTP header types used at the handler level?

Juergen