If-Unmodified-Since header check removes Last-Modified and Etag headers from response, even if condition passes

[molnarp]

Affects: 5.3.23

When handling the response, HttpEntityMethodProcessor calls isResourceNotModified() which removes the Last-Modified and Etag headers, set already in the response:
https://github.com/spring-projects/spring-framework/blob/5.3.x/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java#L247

Subsequently, when WebServletRequest.checkNotModified() is invoked, it calls validateIfUnmodifiedSince(). If the header is present and populated with a valid value, this will return true, which will then go to a branch which invariably returns from the method. Further down, the two headers are readded, but those are never reached.

https://github.com/spring-projects/spring-framework/blob/5.3.x/spring-web/src/main/java/org/springframework/web/context/request/ServletWebRequest.java#L219

Therefore, even if the If-Unmodified-Since condition passes, those headers are removed from the response.

[molnarp]

As far as I can tell, even though the code has changed in the main branch, the issue is still present:
https://github.com/spring-projects/spring-framework/blob/main/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java#L266

Why does the isResourceNotModified() method remove those headers in the first place?

[bclozel]

In these cases, a sample application demonstrating the problem would help a lot.
Could you share a small project (typically a small web app created with start.spring.io m) that we can git clone or download? Ideally, with a readme that lists a curl request, the expected result and what you got instead.

Pointing to various places in the codebase helps, but it's too early for that and we need consider the use case as a whole first.
Thanks!

[molnarp]

Hello Brian,
here you go: https://github.com/molnarp/http-conditional-response-test/blob/main/src/test/java/com/example/demo/FruitResourceTest.java#L36

[bclozel]

Thanks for the sample @molnarp , this is now fixed and will be released with our next maintenance version in November.