Skip to content

Honor qualified Cache-Control: private="field" in a shared cache (RFC9111 section 5.2.2.7) #868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
arturobernalg wants to merge 1 commit into from
Closed

Honor qualified Cache-Control: private="field" in a shared cache (RFC9111 section 5.2.2.7) #868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/ResponseCacheControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ public final class ResponseCacheControl implements CacheControl {
*/
private final Set<String> noCacheFields;

/**
* A set of field names specified in the "private" directive of the Cache-Control header.
*/
private final Set<String> privateFields;

private final boolean undefined;

/**
Expand All @@ -128,13 +133,15 @@ public final class ResponseCacheControl implements CacheControl {
* @param staleWhileRevalidate The stale-while-revalidate value from the Cache-Control header.
* @param staleIfError The stale-if-error value from the Cache-Control header.
* @param noCacheFields The set of field names specified in the "no-cache" directive of the Cache-Control header.
* @param privateFields The set of field names specified in the "private" directive of the Cache-Control header.
* @param mustUnderstand The must-understand value from the Cache-Control header.
* @param immutable The immutable value from the Cache-Control header.
*/
ResponseCacheControl(final long maxAge, final long sharedMaxAge, final boolean mustRevalidate, final boolean noCache,
final boolean noStore, final boolean cachePrivate, final boolean proxyRevalidate,
final boolean cachePublic, final long staleWhileRevalidate, final long staleIfError,
final Set<String> noCacheFields, final boolean mustUnderstand, final boolean immutable) {
final Set<String> noCacheFields, final Set<String> privateFields, final boolean mustUnderstand,
final boolean immutable) {
this.maxAge = maxAge;
this.sharedMaxAge = sharedMaxAge;
this.noCache = noCache;
Expand All @@ -146,6 +153,7 @@ public final class ResponseCacheControl implements CacheControl {
this.staleWhileRevalidate = staleWhileRevalidate;
this.staleIfError = staleIfError;
this.noCacheFields = noCacheFields != null ? Collections.unmodifiableSet(noCacheFields) : Collections.emptySet();
this.privateFields = privateFields != null ? Collections.unmodifiableSet(privateFields) : Collections.emptySet();
this.undefined = maxAge == -1 &&
sharedMaxAge == -1 &&
!noCache &&
Expand Down Expand Up @@ -272,6 +280,16 @@ public Set<String> getNoCacheFields() {
return noCacheFields;
}

/**
* Returns an unmodifiable set of field names specified in the "private" directive of the Cache-Control header.
*
* @return The set of field names specified in the "private" directive.
* @since 5.7
*/
public Set<String> getPrivateFields() {
return privateFields;
}

/**
* Returns the 'immutable' Cache-Control directive status.
*
Expand Down Expand Up @@ -356,6 +374,7 @@ public static class Builder {
private long staleWhileRevalidate = -1;
private long staleIfError = -1;
private Set<String> noCacheFields;
private Set<String> privateFields;
private boolean mustUnderstand;
private boolean immutable;

Expand Down Expand Up @@ -467,6 +486,21 @@ public Builder setNoCacheFields(final String... noCacheFields) {
return this;
}

public Set<String> getPrivateFields() {
return privateFields;
}

public Builder setPrivateFields(final Set<String> privateFields) {
this.privateFields = privateFields;
return this;
}

public Builder setPrivateFields(final String... privateFields) {
this.privateFields = new HashSet<>();
this.privateFields.addAll(Arrays.asList(privateFields));
return this;
}

public boolean isMustUnderstand() {
return mustUnderstand;
}
Expand All @@ -487,7 +521,7 @@ public Builder setImmutable(final boolean immutable) {

public ResponseCacheControl build() {
return new ResponseCacheControl(maxAge, sharedMaxAge, mustRevalidate, noCache, noStore, cachePrivate, proxyRevalidate,
cachePublic, staleWhileRevalidate, staleIfError, noCacheFields, mustUnderstand, immutable);
cachePublic, staleWhileRevalidate, staleIfError, noCacheFields, privateFields, mustUnderstand, immutable);
}

}
Expand Down
49 changes: 36 additions & 13 deletions httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ class CachingAsyncDataConsumer implements AsyncDataConsumer {
private final String exchangeId;
private final AsyncExecCallback fallback;
private final HttpResponse backendResponse;
private final ResponseCacheControl responseCacheControl;
private final EntityDetails entityDetails;
private final AtomicBoolean writtenThrough;
private final AtomicReference<ByteArrayBuffer> bufferRef;
Expand All @@ -596,10 +597,12 @@ class CachingAsyncDataConsumer implements AsyncDataConsumer {
final String exchangeId,
final AsyncExecCallback fallback,
final HttpResponse backendResponse,
final ResponseCacheControl responseCacheControl,
final EntityDetails entityDetails) {
this.exchangeId = exchangeId;
this.fallback = fallback;
this.backendResponse = backendResponse;
this.responseCacheControl = responseCacheControl;
this.entityDetails = entityDetails;
this.writtenThrough = new AtomicBoolean(false);
this.bufferRef = new AtomicReference<>(entityDetails != null ? new ByteArrayBuffer(1024) : null);
Expand Down Expand Up @@ -741,7 +744,7 @@ public void cancelled() {
LOG.debug("{} caching backend response", exchangeId);
}
final CachingAsyncDataConsumer cachingDataConsumer = new CachingAsyncDataConsumer(
exchangeId, asyncExecCallback, backendResponse, entityDetails);
exchangeId, asyncExecCallback, backendResponse, responseCacheControl, entityDetails);
cachingConsumerRef.set(cachingDataConsumer);
return cachingDataConsumer;
}
Expand All @@ -756,14 +759,15 @@ public void handleInformationResponse(final HttpResponse response) throws HttpEx
asyncExecCallback.handleInformationResponse(response);
}

void triggerNewCacheEntryResponse(final HttpResponse backendResponse, final Instant responseDate, final ByteArrayBuffer buffer) {
void triggerNewCacheEntryResponse(final HttpResponse backendResponse, final ResponseCacheControl responseCacheControl,
final Instant responseDate, final ByteArrayBuffer buffer) {
final String exchangeId = scope.exchangeId;
final HttpCacheContext context = HttpCacheContext.cast(scope.clientContext);
final CancellableDependency operation = scope.cancellableDependency;
operation.setDependency(responseCache.store(
target,
request,
backendResponse,
responseToStore(responseCacheControl, backendResponse),
buffer,
requestDate,
responseDate,
Expand All @@ -776,6 +780,7 @@ public void completed(final CacheHit hit) {
}
try {
final SimpleHttpResponse cacheResponse = responseGenerator.generateResponse(request, hit.entry);
restorePrivateFields(cacheResponse, responseCacheControl, backendResponse);
context.setCacheEntry(hit.entry);
triggerResponse(cacheResponse, scope, asyncExecCallback);
} catch (final ResourceIOException ex) {
Expand Down Expand Up @@ -808,6 +813,19 @@ void triggerCachedResponse(final HttpCacheEntry entry) {
}
}

void triggerCachedResponse(final HttpCacheEntry entry, final ResponseCacheControl responseCacheControl,
final HttpResponse originResponse) {
final HttpCacheContext context = HttpCacheContext.cast(scope.clientContext);
try {
final SimpleHttpResponse cacheResponse = responseGenerator.generateResponse(request, entry);
restorePrivateFields(cacheResponse, responseCacheControl, originResponse);
context.setCacheEntry(entry);
triggerResponse(cacheResponse, scope, asyncExecCallback);
} catch (final ResourceIOException ex) {
asyncExecCallback.failed(ex);
}
}

@Override
public void completed() {
final String exchangeId = scope.exchangeId;
Expand All @@ -817,6 +835,7 @@ public void completed() {
return;
}
final HttpResponse backendResponse = cachingDataConsumer.backendResponse;
final ResponseCacheControl responseCacheControl = cachingDataConsumer.responseCacheControl;
final ByteArrayBuffer buffer = cachingDataConsumer.bufferRef.getAndSet(null);

// Handle 304 Not Modified responses
Expand All @@ -831,10 +850,10 @@ public void completed(final CacheMatch result) {
LOG.debug("{} existing cache entry found, updating cache entry", exchangeId);
}
responseCache.update(
hit,
hitToStore(responseCacheControl, hit),
target,
request,
backendResponse,
responseToStore(responseCacheControl, backendResponse),
requestDate,
responseDate,
new FutureCallback<CacheHit>() {
Expand All @@ -844,7 +863,7 @@ public void completed(final CacheHit updated) {
if (LOG.isDebugEnabled()) {
LOG.debug("{} cache entry updated, generating response from updated entry", exchangeId);
}
triggerCachedResponse(updated.entry);
triggerCachedResponse(updated.entry, responseCacheControl, backendResponse);
}
@Override
public void failed(final Exception cause) {
Expand All @@ -864,7 +883,7 @@ public void cancelled() {

});
} else {
triggerNewCacheEntryResponse(backendResponse, responseDate, buffer);
triggerNewCacheEntryResponse(backendResponse, responseCacheControl, responseDate, buffer);
}
}

Expand Down Expand Up @@ -893,7 +912,7 @@ public void completed(final CacheMatch result) {
}
triggerCachedResponse(hit.entry);
} else {
triggerNewCacheEntryResponse(backendResponse, responseDate, buffer);
triggerNewCacheEntryResponse(backendResponse, responseCacheControl, responseDate, buffer);
}
}

Expand All @@ -909,7 +928,7 @@ public void cancelled() {

}));
} else {
triggerNewCacheEntryResponse(backendResponse, responseDate, buffer);
triggerNewCacheEntryResponse(backendResponse, responseCacheControl, responseDate, buffer);
}
}
}
Expand Down Expand Up @@ -1062,11 +1081,12 @@ void revalidateCacheEntry(

void triggerUpdatedCacheEntryResponse(final HttpResponse backendResponse, final Instant responseDate) {
final CancellableDependency operation = scope.cancellableDependency;
final ResponseCacheControl backendCacheControl = CacheControlHeaderParser.INSTANCE.parse(backendResponse);
operation.setDependency(responseCache.update(
hit,
hitToStore(backendCacheControl, hit),
target,
request,
backendResponse,
responseToStore(backendCacheControl, backendResponse),
requestDate,
responseDate,
new FutureCallback<CacheHit>() {
Expand All @@ -1075,6 +1095,7 @@ void triggerUpdatedCacheEntryResponse(final HttpResponse backendResponse, final
public void completed(final CacheHit updated) {
try {
final SimpleHttpResponse cacheResponse = generateCachedResponse(request, updated.entry, responseDate);
restorePrivateFields(cacheResponse, backendCacheControl, backendResponse);
context.setCacheEntry(updated.entry);
triggerResponse(cacheResponse, scope, asyncExecCallback);
} catch (final ResourceIOException ex) {
Expand Down Expand Up @@ -1422,11 +1443,12 @@ void updateVariantCacheEntry(final HttpResponse backendResponse, final Instant r
context.setCacheResponseStatus(CacheResponseStatus.VALIDATED);
cacheUpdates.getAndIncrement();

final ResponseCacheControl backendCacheControl = CacheControlHeaderParser.INSTANCE.parse(backendResponse);
operation.setDependency(responseCache.storeFromNegotiated(
match,
hitToStore(backendCacheControl, match),
target,
request,
backendResponse,
responseToStore(backendCacheControl, backendResponse),
requestDate,
responseDate,
new FutureCallback<CacheHit>() {
Expand All @@ -1435,6 +1457,7 @@ void updateVariantCacheEntry(final HttpResponse backendResponse, final Instant r
public void completed(final CacheHit hit) {
try {
final SimpleHttpResponse cacheResponse = generateCachedResponse(request, hit.entry, responseDate);
restorePrivateFields(cacheResponse, backendCacheControl, backendResponse);
context.setCacheEntry(hit.entry);
triggerResponse(cacheResponse, scope, asyncExecCallback);
} catch (final ResourceIOException ex) {
Expand Down
14 changes: 14 additions & 0 deletions ...5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControlHeaderParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,20 @@ public final ResponseCacheControl parseResponse(final Iterator<Header> headerIte
builder.setNoStore(true);
} else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_PRIVATE)) {
builder.setCachePrivate(true);
if (value != null) {
final Tokenizer.Cursor valCursor = new ParserCursor(0, value.length());
final Set<String> privateFields = new HashSet<>();
while (!valCursor.atEnd()) {
final String token = tokenParser.parseToken(value, valCursor, VALUE_DELIMS);
if (!TextUtils.isBlank(token)) {
privateFields.add(token);
}
if (!valCursor.atEnd()) {
valCursor.updatePos(valCursor.getPos() + 1);
}
}
builder.setPrivateFields(privateFields);
}
} else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_PROXY_REVALIDATE)) {
builder.setProxyRevalidate(true);
} else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_PUBLIC)) {
Expand Down
Loading
Loading