Skip to content

Commit 7e9ca32

Browse files
arturobernalgarturobernalg
committed
Honor qualified Cache-Control: private="field" in a shared cache (RFC 9111 section 5.2.2.7)
A shared cache now stores a response that carries a qualified private directive with the named header fields removed from the stored copy, instead of treating the whole response as non-cacheable, while the response returned to the caller retains those fields. The fields are removed from freshly stored entries, from entries updated by a 304 revalidation, and from the root entry of a Vary response. A bare private directive still makes the whole response non-storable by a shared cache, and multiple qualified private directives accumulate their field names.
1 parent a890ec7 commit 7e9ca32

13 files changed

Lines changed: 376 additions & 23 deletions

‎httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/ResponseCacheControl.java‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ public final class ResponseCacheControl implements CacheControl {
104104
*/
105105
private final Set<String> noCacheFields;
106106

107+
/**
108+
* A set of field names specified in the "private" directive of the Cache-Control header.
109+
*/
110+
private final Set<String> privateFields;
111+
107112
private final boolean undefined;
108113

109114
/**
@@ -128,13 +133,15 @@ public final class ResponseCacheControl implements CacheControl {
128133
* @param staleWhileRevalidate The stale-while-revalidate value from the Cache-Control header.
129134
* @param staleIfError The stale-if-error value from the Cache-Control header.
130135
* @param noCacheFields The set of field names specified in the "no-cache" directive of the Cache-Control header.
136+
* @param privateFields The set of field names specified in the "private" directive of the Cache-Control header.
131137
* @param mustUnderstand The must-understand value from the Cache-Control header.
132138
* @param immutable The immutable value from the Cache-Control header.
133139
*/
134140
ResponseCacheControl(final long maxAge, final long sharedMaxAge, final boolean mustRevalidate, final boolean noCache,
135141
final boolean noStore, final boolean cachePrivate, final boolean proxyRevalidate,
136142
final boolean cachePublic, final long staleWhileRevalidate, final long staleIfError,
137-
final Set<String> noCacheFields, final boolean mustUnderstand, final boolean immutable) {
143+
final Set<String> noCacheFields, final Set<String> privateFields, final boolean mustUnderstand,
144+
final boolean immutable) {
138145
this.maxAge = maxAge;
139146
this.sharedMaxAge = sharedMaxAge;
140147
this.noCache = noCache;
@@ -146,6 +153,7 @@ public final class ResponseCacheControl implements CacheControl {
146153
this.staleWhileRevalidate = staleWhileRevalidate;
147154
this.staleIfError = staleIfError;
148155
this.noCacheFields = noCacheFields != null ? Collections.unmodifiableSet(noCacheFields) : Collections.emptySet();
156+
this.privateFields = privateFields != null ? Collections.unmodifiableSet(privateFields) : Collections.emptySet();
149157
this.undefined = maxAge == -1 &&
150158
sharedMaxAge == -1 &&
151159
!noCache &&
@@ -272,6 +280,16 @@ public Set<String> getNoCacheFields() {
272280
return noCacheFields;
273281
}
274282

283+
/**
284+
* Returns an unmodifiable set of field names specified in the "private" directive of the Cache-Control header.
285+
*
286+
* @return The set of field names specified in the "private" directive.
287+
* @since 5.7
288+
*/
289+
public Set<String> getPrivateFields() {
290+
return privateFields;
291+
}
292+
275293
/**
276294
* Returns the 'immutable' Cache-Control directive status.
277295
*
@@ -356,6 +374,7 @@ public static class Builder {
356374
private long staleWhileRevalidate = -1;
357375
private long staleIfError = -1;
358376
private Set<String> noCacheFields;
377+
private Set<String> privateFields;
359378
private boolean mustUnderstand;
360379
private boolean immutable;
361380

@@ -467,6 +486,30 @@ public Builder setNoCacheFields(final String... noCacheFields) {
467486
return this;
468487
}
469488

489+
/**
490+
* @since 5.7
491+
*/
492+
public Set<String> getPrivateFields() {
493+
return privateFields;
494+
}
495+
496+
/**
497+
* @since 5.7
498+
*/
499+
public Builder setPrivateFields(final Set<String> privateFields) {
500+
this.privateFields = privateFields;
501+
return this;
502+
}
503+
504+
/**
505+
* @since 5.7
506+
*/
507+
public Builder setPrivateFields(final String... privateFields) {
508+
this.privateFields = new HashSet<>();
509+
this.privateFields.addAll(Arrays.asList(privateFields));
510+
return this;
511+
}
512+
470513
public boolean isMustUnderstand() {
471514
return mustUnderstand;
472515
}
@@ -487,7 +530,7 @@ public Builder setImmutable(final boolean immutable) {
487530

488531
public ResponseCacheControl build() {
489532
return new ResponseCacheControl(maxAge, sharedMaxAge, mustRevalidate, noCache, noStore, cachePrivate, proxyRevalidate,
490-
cachePublic, staleWhileRevalidate, staleIfError, noCacheFields, mustUnderstand, immutable);
533+
cachePublic, staleWhileRevalidate, staleIfError, noCacheFields, privateFields, mustUnderstand, immutable);
491534
}
492535

493536
}

‎httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpAsyncCache.java‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,27 @@ class BasicHttpAsyncCache implements HttpAsyncCache {
6969
private final HttpCacheEntryFactory cacheEntryFactory;
7070
private final CacheKeyGenerator cacheKeyGenerator;
7171
private final HttpAsyncCacheStorage storage;
72+
private final boolean sharedCache;
7273

7374
public BasicHttpAsyncCache(
7475
final ResourceFactory resourceFactory,
7576
final HttpCacheEntryFactory cacheEntryFactory,
7677
final HttpAsyncCacheStorage storage,
77-
final CacheKeyGenerator cacheKeyGenerator) {
78+
final CacheKeyGenerator cacheKeyGenerator,
79+
final boolean sharedCache) {
7880
this.resourceFactory = resourceFactory;
7981
this.cacheEntryFactory = cacheEntryFactory;
8082
this.cacheKeyGenerator = cacheKeyGenerator;
8183
this.storage = storage;
84+
this.sharedCache = sharedCache;
85+
}
86+
87+
public BasicHttpAsyncCache(
88+
final ResourceFactory resourceFactory,
89+
final HttpCacheEntryFactory cacheEntryFactory,
90+
final HttpAsyncCacheStorage storage,
91+
final CacheKeyGenerator cacheKeyGenerator) {
92+
this(resourceFactory, cacheEntryFactory, storage, cacheKeyGenerator, CacheConfig.DEFAULT.isSharedCache());
8293
}
8394

8495
public BasicHttpAsyncCache(
@@ -227,7 +238,7 @@ Cancellable storeInternal(final String cacheKey, final HttpCacheEntry entry, fin
227238
LOG.debug("Store entry in cache: {}", cacheKey);
228239
}
229240

230-
return storage.putEntry(cacheKey, entry, new FutureCallback<Boolean>() {
241+
return storage.putEntry(cacheKey, sharedCache ? BasicHttpCache.stripPrivateFields(entry) : entry, new FutureCallback<Boolean>() {
231242

232243
@Override
233244
public void completed(final Boolean result) {
@@ -366,7 +377,8 @@ public void completed(final Boolean result) {
366377
existing -> {
367378
final Set<String> variantMap = existing != null ? new HashSet<>(existing.getVariants()) : new HashSet<>();
368379
variantMap.add(variantKey);
369-
return cacheEntryFactory.createRoot(entry, variantMap);
380+
return cacheEntryFactory.createRoot(
381+
sharedCache ? BasicHttpCache.stripPrivateFields(entry) : entry, variantMap);
370382
},
371383
new CallbackContribution<Boolean>(callback) {
372384

‎httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java‎

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.ArrayList;
3232
import java.util.Collections;
3333
import java.util.HashSet;
34+
import java.util.Iterator;
3435
import java.util.List;
3536
import java.util.Set;
3637

@@ -43,13 +44,16 @@
4344
import org.apache.hc.client5.http.cache.Resource;
4445
import org.apache.hc.client5.http.cache.ResourceFactory;
4546
import org.apache.hc.client5.http.cache.ResourceIOException;
47+
import org.apache.hc.client5.http.cache.ResponseCacheControl;
4648
import org.apache.hc.client5.http.validator.ETag;
4749
import org.apache.hc.client5.http.validator.ValidatorType;
50+
import org.apache.hc.core5.http.Header;
4851
import org.apache.hc.core5.http.HttpHeaders;
4952
import org.apache.hc.core5.http.HttpHost;
5053
import org.apache.hc.core5.http.HttpResponse;
5154
import org.apache.hc.core5.http.HttpStatus;
5255
import org.apache.hc.core5.http.Method;
56+
import org.apache.hc.core5.http.message.HeaderGroup;
5357
import org.apache.hc.core5.util.ByteArrayBuffer;
5458
import org.slf4j.Logger;
5559
import org.slf4j.LoggerFactory;
@@ -62,16 +66,27 @@ class BasicHttpCache implements HttpCache {
6266
private final HttpCacheEntryFactory cacheEntryFactory;
6367
private final CacheKeyGenerator cacheKeyGenerator;
6468
private final HttpCacheStorage storage;
69+
private final boolean sharedCache;
6570

6671
public BasicHttpCache(
6772
final ResourceFactory resourceFactory,
6873
final HttpCacheEntryFactory cacheEntryFactory,
6974
final HttpCacheStorage storage,
70-
final CacheKeyGenerator cacheKeyGenerator) {
75+
final CacheKeyGenerator cacheKeyGenerator,
76+
final boolean sharedCache) {
7177
this.resourceFactory = resourceFactory;
7278
this.cacheEntryFactory = cacheEntryFactory;
7379
this.cacheKeyGenerator = cacheKeyGenerator;
7480
this.storage = storage;
81+
this.sharedCache = sharedCache;
82+
}
83+
84+
public BasicHttpCache(
85+
final ResourceFactory resourceFactory,
86+
final HttpCacheEntryFactory cacheEntryFactory,
87+
final HttpCacheStorage storage,
88+
final CacheKeyGenerator cacheKeyGenerator) {
89+
this(resourceFactory, cacheEntryFactory, storage, cacheKeyGenerator, CacheConfig.DEFAULT.isSharedCache());
7590
}
7691

7792
public BasicHttpCache(
@@ -86,7 +101,8 @@ public BasicHttpCache(final ResourceFactory resourceFactory, final HttpCacheStor
86101
}
87102

88103
public BasicHttpCache(final CacheConfig config) {
89-
this(new HeapResourceFactory(), new BasicHttpCacheStorage(config));
104+
this(new HeapResourceFactory(), HttpCacheEntryFactory.INSTANCE, new BasicHttpCacheStorage(config),
105+
new CacheKeyGenerator(), config.isSharedCache());
90106
}
91107

92108
public BasicHttpCache() {
@@ -95,14 +111,49 @@ public BasicHttpCache() {
95111

96112
void storeInternal(final String cacheKey, final HttpCacheEntry entry) {
97113
try {
98-
storage.putEntry(cacheKey, entry);
114+
storage.putEntry(cacheKey, sharedCache ? stripPrivateFields(entry) : entry);
99115
} catch (final ResourceIOException ex) {
100116
if (LOG.isWarnEnabled()) {
101117
LOG.warn("I/O error storing cache entry with key {}", cacheKey);
102118
}
103119
}
104120
}
105121

122+
/**
123+
* Returns a copy of the entry with the header fields named by a qualified {@code private} directive
124+
* removed, or the entry unchanged when it carries no such directive. A shared cache must not store
125+
* those fields, while the remainder of the response stays cacheable (RFC 9111 section 5.2.2.7).
126+
*/
127+
static HttpCacheEntry stripPrivateFields(final HttpCacheEntry entry) {
128+
final ResponseCacheControl cacheControl = CacheControlHeaderParser.INSTANCE.parse(entry);
129+
final Set<String> privateFields = cacheControl.getPrivateFields();
130+
if (!cacheControl.isCachePrivate() || privateFields.isEmpty()) {
131+
return entry;
132+
}
133+
final HeaderGroup responseHeaders = new HeaderGroup();
134+
for (final Iterator<Header> it = entry.headerIterator(); it.hasNext(); ) {
135+
responseHeaders.addHeader(it.next());
136+
}
137+
for (final String field : privateFields) {
138+
responseHeaders.removeHeaders(field);
139+
}
140+
final HeaderGroup requestHeaders = new HeaderGroup();
141+
for (final Iterator<Header> it = entry.requestHeaderIterator(); it.hasNext(); ) {
142+
requestHeaders.addHeader(it.next());
143+
}
144+
return new HttpCacheEntry(
145+
entry.getRequestInstant(),
146+
entry.getResponseInstant(),
147+
entry.getRequestMethod(),
148+
entry.getRequestURI(),
149+
requestHeaders,
150+
entry.getRequestContent(),
151+
entry.getStatus(),
152+
responseHeaders,
153+
entry.getResource(),
154+
entry.hasVariants() ? entry.getVariants() : null);
155+
}
156+
106157
void updateInternal(final String cacheKey, final HttpCacheCASOperation casOperation) {
107158
try {
108159
storage.updateEntry(cacheKey, casOperation);
@@ -210,7 +261,7 @@ CacheHit store(final String rootKey, final String variantKey, final HttpCacheEnt
210261
updateInternal(rootKey, existing -> {
211262
final Set<String> variants = existing != null ? new HashSet<>(existing.getVariants()) : new HashSet<>();
212263
variants.add(variantKey);
213-
return cacheEntryFactory.createRoot(entry, variants);
264+
return cacheEntryFactory.createRoot(sharedCache ? stripPrivateFields(entry) : entry, variants);
214265
});
215266
return new CacheHit(rootKey, variantCacheKey, entry);
216267
}

‎httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControlHeaderParser.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,8 @@ public void parse(final Iterator<Header> headerIterator, final BiConsumer<String
162162
public final ResponseCacheControl parseResponse(final Iterator<Header> headerIterator) {
163163
Args.notNull(headerIterator, "headerIterator");
164164
final ResponseCacheControl.Builder builder = ResponseCacheControl.builder();
165+
final Set<String> privateFields = new HashSet<>();
166+
final boolean[] barePrivate = {false};
165167
parse(headerIterator, (name, value) -> {
166168
if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_S_MAX_AGE)) {
167169
builder.setSharedMaxAge(parseSeconds(name, value));
@@ -189,6 +191,22 @@ public final ResponseCacheControl parseResponse(final Iterator<Header> headerIte
189191
builder.setNoStore(true);
190192
} else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_PRIVATE)) {
191193
builder.setCachePrivate(true);
194+
if (value != null) {
195+
final Tokenizer.Cursor valCursor = new ParserCursor(0, value.length());
196+
while (!valCursor.atEnd()) {
197+
final String token = tokenParser.parseToken(value, valCursor, VALUE_DELIMS);
198+
if (!TextUtils.isBlank(token)) {
199+
privateFields.add(token);
200+
}
201+
if (!valCursor.atEnd()) {
202+
valCursor.updatePos(valCursor.getPos() + 1);
203+
}
204+
}
205+
} else {
206+
// A bare (unqualified) private directive makes the whole response non-storable by
207+
// a shared cache, regardless of any qualified private directives.
208+
barePrivate[0] = true;
209+
}
192210
} else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_PROXY_REVALIDATE)) {
193211
builder.setProxyRevalidate(true);
194212
} else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_PUBLIC)) {
@@ -203,6 +221,11 @@ public final ResponseCacheControl parseResponse(final Iterator<Header> headerIte
203221
builder.setImmutable(true);
204222
}
205223
});
224+
// Accumulated qualified private field names apply only when no bare private directive is present;
225+
// a bare private leaves the field set empty so a shared cache treats the whole response as private.
226+
if (!barePrivate[0] && !privateFields.isEmpty()) {
227+
builder.setPrivateFields(privateFields);
228+
}
206229
return builder.build();
207230
}
208231

‎httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingH2AsyncClientBuilder.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,8 @@ protected void customizeExecChain(final NamedElementChain<AsyncExecChainHandler>
142142
resourceFactoryCopy,
143143
HttpCacheEntryFactory.INSTANCE,
144144
storageCopy,
145-
CacheKeyGenerator.INSTANCE);
145+
CacheKeyGenerator.INSTANCE,
146+
config.isSharedCache());
146147

147148
DefaultAsyncCacheRevalidator cacheRevalidator = null;
148149
if (config.getAsynchronousWorkers() > 0) {

‎httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpAsyncClientBuilder.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ protected void customizeExecChain(final NamedElementChain<AsyncExecChainHandler>
146146
resourceFactoryCopy,
147147
HttpCacheEntryFactory.INSTANCE,
148148
storageCopy,
149-
CacheKeyGenerator.INSTANCE);
149+
CacheKeyGenerator.INSTANCE,
150+
config.isSharedCache());
150151

151152
DefaultAsyncCacheRevalidator cacheRevalidator = null;
152153
if (config.getAsynchronousWorkers() > 0) {

‎httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingHttpClientBuilder.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ protected void customizeExecChain(final NamedElementChain<ExecChainHandler> exec
140140
resourceFactoryCopy,
141141
HttpCacheEntryFactory.INSTANCE,
142142
storageCopy,
143-
CacheKeyGenerator.INSTANCE);
143+
CacheKeyGenerator.INSTANCE,
144+
config.isSharedCache());
144145

145146
DefaultCacheRevalidator cacheRevalidator = null;
146147
if (config.getAsynchronousWorkers() > 0) {

‎httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,9 @@ public boolean isResponseCacheable(final RequestCacheControl requestCacheControl
146146
return false;
147147
}
148148
// Status code is in a recognized range; treat no-store as overridden.
149-
if (sharedCache && cacheControl.isCachePrivate()) {
149+
// A qualified "private" directive (one that names specific fields) still permits a shared
150+
// cache to store the remainder of the response, so only bare "private" is non-cacheable.
151+
if (sharedCache && cacheControl.isCachePrivate() && cacheControl.getPrivateFields().isEmpty()) {
150152
LOG.debug("Response is private and cannot be cached by a shared cache");
151153
return false;
152154
}
@@ -253,9 +255,11 @@ protected boolean isExplicitlyNonCacheable(final ResponseCacheControl cacheContr
253255
return false;
254256
}
255257
// The response is considered explicitly non-cacheable if it contains
256-
// "no-store" or (if sharedCache is true) "private" directives.
257-
// Note that "no-cache" is considered cacheable but requires validation before use.
258-
return cacheControl.isNoStore() || sharedCache && cacheControl.isCachePrivate();
258+
// "no-store" or (if sharedCache is true) a bare "private" directive.
259+
// Note that "no-cache" is considered cacheable but requires validation before use, and that a
260+
// qualified "private" directive naming specific fields leaves the remainder cacheable.
261+
return cacheControl.isNoStore()
262+
|| sharedCache && cacheControl.isCachePrivate() && cacheControl.getPrivateFields().isEmpty();
259263
}
260264

261265
protected boolean isExplicitlyCacheable(final ResponseCacheControl cacheControl, final HttpResponse response) {

0 commit comments

Comments
 (0)