Affects: 5.2.0.RELEASE

Inside a @Controller I set the ETag and Cache-control header:

String etag = ...
if (webRequest.checkNotModified( etag ))
    return null;
response.setHeader( HttpHeaders.CACHE_CONTROL, CacheControl.maxAge( 30, TimeUnit.DAYS ).cachePublic().getHeaderValue() );

ShallowETagHeaderFilter overwrites ETag because isEligibleForEtag method returns true:

String cacheControl = response.getHeader(HttpHeaders.CACHE_CONTROL);
return (cacheControl == null || !cacheControl.contains(DIRECTIVE_NO_STORE));

ShallowEtagHeaderFilter should first check if an ETag is already set and skip it.

Comment From: rstoyanchev

I cannot find any calls to WebRequest#checkNotModified(String) in ShallowETagHeaderFilter. Can you please point to the source.

Comment From: cdalexndr

@rstoyanchev sorry, wrong code snippet. Updated original post.

Comment From: rstoyanchev

This was supposed to have been addressed as part of #22797 via https://github.com/spring-projects/spring-framework/commit/1a97a26eb7ed21f5c12f0d89aa67ad518d213e89 but I now see omissions:

  1. disableContentCachingIfNecessary should be invoked before the response is flushed.
  2. there is an inverted condition, if (!isRequestNotModified(webRequest)) that seems wrong.

I will prepare an update and maybe you can give it a try?

Comment From: rstoyanchev

After taking a closer look, only the 2nd point from my previous comment is an issue and that matches the scenario here, (i.e. null return value handling for requestNotModified with an eTag).

Comment From: mickaeltr

Hello,

As I explained in #22797 I am still having an issue with the ShallowEtagHeaderFilter overwriting ETag (and Content-Length), so I created a project with unit tests that demonstrate this: https://github.com/mickaeltr/Spring-ShallowEtagHeaderFilter-issue/blob/master/src/test/java/com/example/demo/DemoControllerTest.java

Thank you