ShallowETagHeaderFilter overwrites ETag

[cdalexndr]

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.

[rstoyanchev]

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

[cdalexndr]

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

[rstoyanchev]

This was supposed to have been addressed as part of #22797 via 1a97a26 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?

[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).

[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