Issue
Expected behaviour:
If-None-Match has precedence when If-None-Match is used in combination with If-Modified-Since.
The function checkNotModified in org.springframework.web.context.request.ServletWebRequest even references the expected order of precedence with the following comment:
// Evaluate conditions in order of precedence.
// See https://tools.ietf.org/html/rfc7232#section-6
Observed behaviour
The response is updated with status NOT MODIFIED when checkNotModified is called from handleRequest in org.springframework.web.servlet.resource.ResourceHttpRequestHandler whenlastModifiedTimestamp indicates that the resources is not modified.
The response is not updated again when checkNotModified is called from updateResponse in org.springframework.web.filter.ShallowEtagHeaderFilter with an etag value indicating that the resource is modified.
As a result, the server returns 304 NOT MODIFIED for an invalid etag when both the If-None-Match and If-Modified-Since-header is set.
Problem
After rollback to an earlier deployment, clients will send a lastModifiedTimestamp newer than the content on the server and an invalid etag. Since If-None-Match doesn't have precedence as expected, the client recieves a 304 NOT MODIFIED when a 200 OK was expected.
Work-around
Problem can be mitigated by not using If-Modified-Since. E.g. utilizing setUseLastModified(false) on a resource handler.
Reproduction
Send a GET-request with an invalid etag and lastModifiedTimestamp in the future.
Question
Does Spring handle the combination of If-Modified-Since and If-None-Match correctly, or could this be a bug?
Update 1
(Thanks Kevin)
It seems to be something wrong on my end, since the implementation details locally and in the spring-project differs.
- I mismatched the main and 5.3.x branch. The local implementation matches the implementation on the 5.3.x branch.
Update 2
checkNotModified is covered by the test IfNoneMatchAndIfNotModifiedSinceShouldNotMatchWhenDifferentETag. However, this test covers the case where checkNotModified is called with both eTag and lastModifiedTimestamp. In the case described, checkNotModified is called sequentially by overloaded methods. The overloaded method call with lastModifiedTimestamp alters the response status to 304 Not Modified. This does not seem to be handled by the ShallowEtagHeaderFilter where isEligbleForEtag returns false if the status code is 304, and the call to checkNotModified from updateResponse in shallowEtagHeaderFilter does not update the response from 304 to 200 OK if the etag has changed.
Update 3
Solution
The behavior you're describing has already been reported and supposedly fixed. They also have a test case meant to capture the exact situation you describe (no ETag match and future last modified should return 200): IfNoneMatchAndIfNotModifiedSinceShouldNotMatchWhenDifferentETag.
However, as you say, handleRequest does not actually call checkNotModified with both arguments. And you're seeing this behavior on a current version of the software.
So it sounds like a real bug, and I would say the next step is to file a bug report with Spring Framework.
Answered By - Kevin Christopher Henry
Answer Checked By - Mary Flores (JavaFixing Volunteer)
