WebRequest.checkNotModified() issue...

[allenru]

The current (Spring 3) version of WebRequest.checkNotModified(long lastModifiedDate) combines both the "check" and the "set" steps when deciding whether or not a request is cached/modified. I think the method mistakenly assumes that the lastModifiedDate can always be computed in a persistent manner, which is typically not the case.

An example is helpful:

@Controller
public class ExampleController {

	private long lastModified = -1;  //not persistent, gets reset on server restart...
	
	@RequestMapping(value = "/example", method = RequestMethod.GET)
	public String example(WebRequest webRequest) {
		if (webRequest.checkNotModified(lastModified)) {
			// shortcut exit - no further processing necessary
			return null;
		}
		// further request processing, actually building content
		lastModified = System.currentTimeMillis();
		return "movie";
	}
	
}

This code will rarely result in a HTTP 304 response.

• On the first request, checkNotModified checks the header (no value) and passed in last modified date (-1) and sets nothing in the response header since the last modified date is -1.
• On the second request, checkNotModified checks the header (no value) and passed in last modified date (now at time_1) and sets the response header to a date of time_1. It still returns false though, since the response 'was modified' based on the request's timestamp (no value < time_1). As a result, the "further processing" occurs, and lastModified is locally moved forward to time_2.
• On the third request, checkNotModified checks the header (now at time_1) and passed in last modified date (now at time_2) and sets the response header to a date of time_2. It still returns false though, since the response 'was modified' based on the request's timestamp (time_1 < time_2). As a result, the "further processing" occurs, and lastModified is moved forward to time_3.
• This last pattern repeats for each request, incrementing the time on each request but the response time is always behind the local last modified time.

(Note, last modified dates are rounded down to the nearest second. As a result it is possible to get a time_1 and time_2 that are == after dropping the milliseconds. Thus, in rare cases, it will appear to work.)


I think (looking for advice here) that the two steps need to be broken apart so that controller code would look more like the following:

	@RequestMapping(value = "/example", method = RequestMethod.GET)
	public String example(WebRequest webRequest) {
		if (webRequest.isNotModified(lastModified)) {
			// shortcut exit - no further processing necessary
			return null;
		}
		// further request processing, actually building content
		lastModified = System.currentTimeMillis();
		webRequest.setLastModified(lastModified);
		return "movie";
	}

Thoughts?

[Marten Deinum]

Not sure if that is a problem, from my pov it acts just like it should and is the flaw in your logic (or in mine ).

You should set the header on the first request and not update the last modified timestamp on each incoming request that way your resource is always modified. The check is to see if your resource has been modified since the last request not to check if the request is modified.

Imagine you are sending a text file and as the last modifed date you send the file date. The next request has that date and checks against the file date it will send a 304 as it isn't modified. Now use touch to change the date and the next request sees that the file has changed since the last modified date and thus gets the new file.

So basically your last modified date should only change if it the content actually changed and you simply update the timestamp on each incoming request.