From 69e5799362e19bdf07dbad3589aea32b48183a3c Mon Sep 17 00:00:00 2001 From: Alhuda Khan Date: Thu, 8 Oct 2026 18:05:54 +0530 Subject: [PATCH] skip private and Vary: * responses in HttpCacheInterceptor --- http-cache/README.md | 8 +++- .../feign/cache/HttpCacheInterceptor.java | 33 +++++++++++--- .../feign/cache/HttpCacheInterceptorTest.java | 44 +++++++++++++++++++ 3 files changed, 77 insertions(+), 8 deletions(-) diff --git a/http-cache/README.md b/http-cache/README.md index c8670f533..daad48a8a 100644 --- a/http-cache/README.md +++ b/http-cache/README.md @@ -54,5 +54,9 @@ Caveats first 200; on a 304 the cached object is returned as-is. Mutations on the returned object propagate to subsequent callers — return immutable values from your decoder if that matters. -- `Cache-Control` directives beyond `no-store` are not interpreted; freshness windows - (`max-age`, etc.) are not enforced. Every cached entry triggers revalidation. +- Freshness windows (`max-age`, etc.) are not enforced. Every cached entry triggers + revalidation. +- A store is shared by every caller of the client, so responses marked + `Cache-Control: no-store`, `Cache-Control: private` or `Vary: *` are never stored. + Other `Vary` field names are not matched against the request, so include them in + `keyFn` when a response varies on a header the default key ignores. diff --git a/http-cache/src/main/java/feign/cache/HttpCacheInterceptor.java b/http-cache/src/main/java/feign/cache/HttpCacheInterceptor.java index be9e2cfb9..d6b8d3d9d 100644 --- a/http-cache/src/main/java/feign/cache/HttpCacheInterceptor.java +++ b/http-cache/src/main/java/feign/cache/HttpCacheInterceptor.java @@ -35,7 +35,8 @@ * returns the previously decoded value from {@link HttpCacheStore} without re-decoding. * *

Successful responses (2xx) carrying an {@code ETag} or {@code Last-Modified} header are - * stored. Responses with {@code Cache-Control: no-store} are skipped. + * stored, unless the response says it cannot be held in a cache shared by several callers: {@code + * Cache-Control: no-store}, {@code Cache-Control: private} and {@code Vary: *} are skipped. * *

Default scope is HTTP {@code GET}, {@code HEAD}, and {@code QUERY}; override via {@link * #cacheable(Function)}. @@ -49,6 +50,8 @@ public final class HttpCacheInterceptor implements MethodInterceptor { private static final Pattern NO_STORE = Pattern.compile("(?i)\\bno-store\\b"); + private static final Pattern PRIVATE = Pattern.compile("(?i)\\bprivate\\b"); + private final HttpCacheStore store; private final Function keyFn; private final Function cacheable; @@ -118,7 +121,7 @@ private void maybeStore(String key, Object result, Response response) { return; } Map> headers = response.headers(); - if (containsNoStore(headers)) { + if (!shareable(headers)) { return; } String etag = firstHeader(headers, "ETag"); @@ -144,13 +147,31 @@ private static Boolean defaultCacheable(RequestTemplate template) { || "QUERY".equalsIgnoreCase(method); } - private static boolean containsNoStore(Map> headers) { + /** + * Whether a {@link HttpCacheStore}, which every caller of the client shares, may hold this + * response. {@code no-store} and {@code private} mark a response that a shared cache must not + * store (RFC 9111 sections 5.2.2.5 and 5.2.2.7), and {@code Vary: *} marks one that never matches + * a later request (RFC 9111 section 4.1); the default key covers the method, url and body only, + * so storing any of them hands the entry and its validator to the next caller of the same method + * and url. + */ + private static boolean shareable(Map> headers) { for (String value : Util.valuesOrEmpty(headers, "Cache-Control")) { - if (value != null && NO_STORE.matcher(value).find()) { - return true; + if (value != null && (NO_STORE.matcher(value).find() || PRIVATE.matcher(value).find())) { + return false; + } + } + for (String value : Util.valuesOrEmpty(headers, "Vary")) { + if (value == null) { + continue; + } + for (String field : value.split(",")) { + if ("*".equals(field.trim())) { + return false; + } } } - return false; + return true; } private static String firstHeader(Map> headers, String name) { diff --git a/http-cache/src/test/java/feign/cache/HttpCacheInterceptorTest.java b/http-cache/src/test/java/feign/cache/HttpCacheInterceptorTest.java index 3607f6bd5..1efe7214f 100644 --- a/http-cache/src/test/java/feign/cache/HttpCacheInterceptorTest.java +++ b/http-cache/src/test/java/feign/cache/HttpCacheInterceptorTest.java @@ -185,6 +185,50 @@ void noStoreCacheControlPreventsStorage() throws Exception { assertThat(store.size()).isZero(); } + @Test + void privateCacheControlPreventsStorage() throws Exception { + server.enqueue( + new MockResponse() + .setHeader("ETag", "\"v1\"") + .setHeader("Cache-Control", "max-age=60, private") + .setBody("payload")); + + api().fetch("42"); + + assertThat(store.size()).isZero(); + } + + @Test + void varyAsteriskPreventsStorage() throws Exception { + server.enqueue( + new MockResponse() + .setHeader("ETag", "\"v1\"") + .setHeader("Vary", "Accept-Encoding, *") + .setBody("payload")); + + api().fetch("42"); + + assertThat(store.size()).isZero(); + } + + @Test + void privateResponseIsNotRevalidatedForASecondCaller() throws Exception { + server.enqueue( + new MockResponse() + .setHeader("ETag", "\"v1\"") + .setHeader("Cache-Control", "private") + .setBody("caller-a-data")); + server.enqueue(new MockResponse().setResponseCode(304)); + + Api api = api(); + assertThat(api.fetch("42")).isEqualTo("caller-a-data"); + assertThatThrownBy(() -> api.fetch("42")).isInstanceOf(FeignException.class); + + server.takeRequest(); // caller a + RecordedRequest second = server.takeRequest(); + assertThat(second.getHeader("If-None-Match")).isNull(); + } + @Test void serverErrorPropagatesAndDoesNotEvictExistingEntry() throws Exception { server.enqueue(new MockResponse().setHeader("ETag", "\"v1\"").setBody("payload-1"));