Polishing CacheControl behavior

This commit builds on the previous commit and ensures that
"must-understand" is only used with "no-store". This check is performed
at runtime as a staged interface/builder would be a major breaking
change for a behavior that is highlighted as "SHOULD" in the
specification.

This commit also performs similar runtime checks for:
* cache-public + cache-private
* cache-public + no-store

See gh-36918
This commit is contained in:
Brian Clozel
2026-08-28 15:49:57 +02:00
parent d9e240b37d
commit 5d6a56fe4a
2 changed files with 31 additions and 2 deletions
@@ -21,6 +21,7 @@ import java.util.concurrent.TimeUnit;
import org.jspecify.annotations.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
/**
@@ -171,13 +172,16 @@ public class CacheControl {
* Add a "must-understand" directive.
* <p>This directive limits caching of the response to caches that
* understand and conform to the requirements for the response status code.
* The {@link #noStore()} directive should also be set as a fallback for
* caches that do not implement the "must-understand" directive.
* <p>The {@link #noStore()} directive must have been set beforehand, as a
* fallback for caches that do not implement the "must-understand" directive.
* @return {@code this}, to facilitate method chaining
* @throws IllegalStateException if the "no-store" directive has not been set
* @since 7.1
* @see <a href="https://www.rfc-editor.org/rfc/rfc9111#section-5.2.2.3">rfc9111 section 5.2.2.3</a>
*/
public CacheControl mustUnderstand() {
Assert.state(this.noStore, "The \"no-store\" directive should be set as a fallback, " +
"use CacheControl.noStore().mustUnderstand() instead");
this.mustUnderstand = true;
return this;
}
@@ -214,9 +218,14 @@ public class CacheControl {
* even if the response would normally be non-cacheable or cacheable
* only within a private cache.
* @return {@code this}, to facilitate method chaining
* @throws IllegalStateException if the "private" or "no-store" directive has already been set
* @see <a href="https://tools.ietf.org/html/rfc7234#section-5.2.2.5">rfc7234 section 5.2.2.5</a>
*/
public CacheControl cachePublic() {
Assert.state(!this.cachePrivate, "The \"private\" directive has already been set, " +
"which is mutually exclusive with \"public\"");
Assert.state(!this.noStore, "The \"no-store\" directive has already been set, " +
"which contradicts \"public\"");
this.cachePublic = true;
return this;
}
@@ -226,9 +235,12 @@ public class CacheControl {
* <p>This directive indicates that the response message is intended
* for a single user and MUST NOT be stored by a shared cache.
* @return {@code this}, to facilitate method chaining
* @throws IllegalStateException if the "public" directive has already been set
* @see <a href="https://tools.ietf.org/html/rfc7234#section-5.2.2.6">rfc7234 section 5.2.2.6</a>
*/
public CacheControl cachePrivate() {
Assert.state(!this.cachePublic, "The \"public\" directive has already been set, " +
"which is mutually exclusive with \"private\"");
this.cachePrivate = true;
return this;
}
@@ -22,6 +22,7 @@ import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
/**
@@ -84,6 +85,22 @@ class CacheControlTests {
assertThat(cc.getHeaderValue()).isEqualTo("no-store, must-understand");
}
@Test
void mustUnderstandWithoutNoStoreRejected() {
assertThatIllegalStateException().isThrownBy(() -> CacheControl.maxAge(1, TimeUnit.HOURS).mustUnderstand());
}
@Test
void cachePublicAndCachePrivateRejected() {
assertThatIllegalStateException().isThrownBy(() -> CacheControl.empty().cachePrivate().cachePublic());
assertThatIllegalStateException().isThrownBy(() -> CacheControl.empty().cachePublic().cachePrivate());
}
@Test
void cachePublicWithNoStoreRejected() {
assertThatIllegalStateException().isThrownBy(() -> CacheControl.noStore().cachePublic());
}
@Test
void staleIfError() {
CacheControl cc = CacheControl.maxAge(1, TimeUnit.HOURS).staleIfError(2, TimeUnit.HOURS);