Behavior of checkNotModified(String etag, long lastModifiedTimestamp) does not match HTTP recommendations [SPR-14224]

[spring-projects-issues]

Sebastiaan van Erk opened SPR-14224 and commented

According to RFC2616 https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html if a request has the If-None-Match or If-Modified-Since headers are set on a request (that is, either one or the other, but not both), a 304 Not Modified can be returned if the the ETag matches or the resource is has not been modified since the specified date.

According to the same specification, if both headers are set, the behavior of the server is undefined. In other words, a client should never use both, as it does not make sense (the server could return "418 I'm a teapot" if it felt like it).

Thus for the checkNotModified(String etag, long lastModifiedTimestamp) method to be useful, it should:

• Check the etag if the If-None-Match header is the only header present.
• Check the last modified timestamp if the If-Modified-Since header is the only header present.
• Do something sane if both are present (which they shouldn't be). In my opinion, the most sane option is to check the etag, because it's generally stronger than the last modified date.

However, currently the implementation of the method returns false unless both headers are present (a case which should never happen). Thus effectively it always returns false.

---

Affects: 4.2.5

Issue Links:

• ServletWebRequest.isEtagNotModified does not support commas and spaces in client ETags [SPR-14216] #18790 ServletWebRequest.isEtagNotModified does not support commas and spaces in client ETags

Referenced from: commits a50ea80

[spring-projects-issues]

Brian Clozel commented

Hi Sebastiaan van Erk,

It seems the latest version of this RFC7232 states otherwise:

(Clients) SHOULD send both validators in cache validation requests if both an entity-tag and a Last-Modified value have been provided by the origin server

see Section 2.4

Also, the same RFC precisely describes how origin servers should react to multiple conditional headers in Section 6, "Precedence".

Let me know if you think that the current behavior does not match the latest spec.

[spring-projects-issues]

Sebastiaan van Erk commented

Hi Brian,

It seems that RFC7232 does indeed seem to override RFC2616 in terms of the behavior when multiple preconditions are present. However, the Precedence section seems be exactly in accordance with the behavior I sketched above, and not with the current implementation. That is:

1. In the case of only a If-None-Match header, it goes: step 1 (does not apply), step 2 (does not apply), step 3 (applies, respond based on the If-None-Match header, except also still consider If-Range if the If-None-Match precondition passes, i.e., go to step 5 in that case, but respond 304/412 otherwise).
2. In the case of only a If-Modified-Since header, it goes: step 1 (does not apply), step 2 (does not apply), step 3 (does not apply), step 4 (applies, respond based on the If-Modified-Since, except also still consider If-Range if the If-Modified-Since precondition passes, i.e., go to step 5 in that case).
3. In the case of both If-None-Match and If-Modified-Since, it goes: step 1 (does not apply), step 2 (does not apply), step 3 (applies, check none-match, if it passes, go to step 5 again for the If-Range check, but if it fails respond with 304 or 412). Note that it never reaches step 4 in this case to check the present If-Modified-Since. In any case, step 4 would never apply because as part of the condition for step 4 to apply it explicitly states that the If-None-Match header is not present. So the If-Modified-Since header is not checked in the case.

In other words, if one of the two is present, use that header; if both are present the If-None-Match takes precedence.

Best regards,
Sebastiaan

[spring-projects-issues]

Brian Clozel commented

Good catch!
This has been fixed in master and should be available shortly as a SNAPSHOT version.

Thanks!

[spring-projects-issues]

Sebastiaan van Erk commented

Thanks for the quick fix! :)

Best regards,
Sebastiaan