From e7b313e130010a982ac77da6653e8ea52fd7f22f Mon Sep 17 00:00:00 2001 From: Mohammed Hassan Date: Sat, 14 Nov 2026 15:05:54 +0530 Subject: [PATCH] do not store responses to authenticated requests in the shared cache --- .../http/impl/cache/AsyncCachingExec.java | 3 +- .../client5/http/impl/cache/CachingExec.java | 3 +- .../http/impl/cache/CachingExecBase.java | 14 ++ .../impl/cache/ResponseCachingPolicy.java | 15 +- .../TestCachingExecAuthenticatedResponse.java | 168 ++++++++++++++++++ 5 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecAuthenticatedResponse.java diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java index d3f8346ddd..4eda6d6c7f 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/AsyncCachingExec.java @@ -752,7 +752,8 @@ public void cancelled() { final ResponseCacheControl responseCacheControl = CacheControlHeaderParser.INSTANCE.parse(backendResponse); context.setResponseCacheControl(responseCacheControl); final boolean cacheable = responseCachingPolicy.isResponseCacheable( - requestCacheControl, responseCacheControl, request, backendResponse); + requestCacheControl, responseCacheControl, request, backendResponse, + isTargetAuthenticated(context, scope.route)); if (cacheable) { storeRequestIfModifiedSinceFor304Response(request, backendResponse); if (LOG.isDebugEnabled()) { diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java index 116e994240..9fbc71bcd6 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExec.java @@ -552,7 +552,8 @@ ClassicHttpResponse handleBackendResponse( final ResponseCacheControl responseCacheControl = CacheControlHeaderParser.INSTANCE.parse(backendResponse); context.setResponseCacheControl(responseCacheControl); final boolean cacheable = responseCachingPolicy.isResponseCacheable( - requestCacheControl, responseCacheControl, request, backendResponse); + requestCacheControl, responseCacheControl, request, backendResponse, + isTargetAuthenticated(context, scope.route)); if (cacheable) { storeRequestIfModifiedSinceFor304Response(request, backendResponse); if (LOG.isDebugEnabled()) { diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java index 3e2db95f0c..5307ba6e8f 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CachingExecBase.java @@ -29,10 +29,13 @@ import java.time.Instant; import java.util.concurrent.atomic.AtomicLong; +import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; +import org.apache.hc.client5.http.auth.AuthExchange; import org.apache.hc.client5.http.cache.HttpCacheContext; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.cache.ResourceIOException; +import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.core5.http.EntityDetails; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHeaders; @@ -118,6 +121,17 @@ public long getCacheUpdates() { return cacheUpdates.get(); } + /** + * Determines whether the protocol layer below the cache authenticated the request + * to the route target. The {@code Authorization} header generated from the + * credentials provider or the auth cache is added to the request instance + * executed by the protocol layer and is not visible to the cache. + */ + static boolean isTargetAuthenticated(final HttpClientContext context, final HttpRoute route) { + final AuthExchange authExchange = context.getAuthExchanges().get(route.getTargetHost()); + return authExchange != null && authExchange.getState() == AuthExchange.State.SUCCESS; + } + SimpleHttpResponse generateCachedResponse( final HttpRequest request, final HttpCacheEntry entry, diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java index 9b829bbae2..3591effe22 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java @@ -91,6 +91,19 @@ public ResponseCachingPolicy( * @return {@code true} if response is cacheable */ public boolean isResponseCacheable(final RequestCacheControl requestCacheControl, final ResponseCacheControl cacheControl, final HttpRequest request, final HttpResponse response) { + return isResponseCacheable(requestCacheControl, cacheControl, request, response, false); + } + + /** + * Determine if the {@link HttpResponse} gotten from the origin is a + * cacheable response. + * + * @param requestCacheControl the cache control directives of the request + * @param authenticated {@code true} if the request was authenticated by the protocol + * layer with credentials not present in the given request + * @return {@code true} if response is cacheable + */ + boolean isResponseCacheable(final RequestCacheControl requestCacheControl, final ResponseCacheControl cacheControl, final HttpRequest request, final HttpResponse response, final boolean authenticated) { if (requestCacheControl.isNoStore()) { if (LOG.isDebugEnabled()) { LOG.debug("Response is not cacheable as the request enforced no-store"); @@ -149,7 +162,7 @@ public boolean isResponseCacheable(final RequestCacheControl requestCacheControl } if (sharedCache) { - if (request.containsHeader(HttpHeaders.AUTHORIZATION) && + if ((authenticated || request.containsHeader(HttpHeaders.AUTHORIZATION)) && cacheControl.getSharedMaxAge() == -1 && !(cacheControl.isPublic() || cacheControl.isMustRevalidate())) { LOG.debug("Request contains private credentials"); diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecAuthenticatedResponse.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecAuthenticatedResponse.java new file mode 100644 index 0000000000..f15953fef9 --- /dev/null +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecAuthenticatedResponse.java @@ -0,0 +1,168 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ +package org.apache.hc.client5.http.impl.cache; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.time.Instant; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpServer; + +import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; +import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder; +import org.apache.hc.client5.http.async.methods.SimpleRequestProducer; +import org.apache.hc.client5.http.async.methods.SimpleResponseConsumer; +import org.apache.hc.client5.http.cache.HttpCacheContext; +import org.apache.hc.client5.http.classic.methods.HttpGet; +import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; +import org.apache.hc.client5.http.impl.auth.CredentialsProviderBuilder; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.utils.DateUtils; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.io.entity.EntityUtils; +import org.apache.hc.core5.io.CloseMode; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * A shared cache must not store a response to a request that the protocol layer + * authenticated with credentials from the credentials provider. + */ +class TestCachingExecAuthenticatedResponse { + + private HttpServer server; + private ExecutorService executorService; + private AtomicInteger originHits; + private HttpHost target; + + @BeforeEach + void setUp() throws IOException { + originHits = new AtomicInteger(0); + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/", this::handleOrigin); + executorService = Executors.newCachedThreadPool(); + server.setExecutor(executorService); + server.start(); + target = new HttpHost("http", "localhost", server.getAddress().getPort()); + } + + @AfterEach + void tearDown() { + server.stop(0); + executorService.shutdownNow(); + } + + private HttpCacheContext authenticatedContext() { + final HttpCacheContext context = HttpCacheContext.create(); + context.setCredentialsProvider(CredentialsProviderBuilder.create() + .add(target, "alice", "secret".toCharArray()) + .build()); + return context; + } + + @Test + void testAuthenticatedResponseNotServedToAnotherContext() throws Exception { + try (final CloseableHttpClient client = CachingHttpClients.custom() + .setResourceFactory(HeapResourceFactory.INSTANCE) + .build()) { + final String first = client.execute(target, new HttpGet("/private"), authenticatedContext(), + response -> response.getCode() + " " + EntityUtils.toString(response.getEntity())); + Assertions.assertEquals("200 private data of alice", first); + + final HttpCacheContext anonymous = HttpCacheContext.create(); + final int code = client.execute(target, new HttpGet("/private"), anonymous, response -> { + EntityUtils.consume(response.getEntity()); + return response.getCode(); + }); + Assertions.assertEquals(401, code); + Assertions.assertEquals(3, originHits.get()); + } + } + + @Test + void testAuthenticatedResponseNotServedToAnotherContextAsyncViaProxy() throws Exception { + // The origin server doubles as a plain HTTP proxy, so the request is sent in absolute form + // and the protocol layer authenticates a copy of the request passed down by the cache + final CloseableHttpAsyncClient client = CachingHttpAsyncClients.custom() + .setResourceFactory(HeapResourceFactory.INSTANCE) + .setProxy(new HttpHost("http", "127.0.0.1", server.getAddress().getPort())) + .build(); + client.start(); + try { + final SimpleHttpResponse first = client.execute( + SimpleRequestProducer.create(SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/private") + .build()), + SimpleResponseConsumer.create(), + authenticatedContext(), + null).get(1, TimeUnit.MINUTES); + Assertions.assertEquals(200, first.getCode()); + Assertions.assertEquals("private data of alice", first.getBodyText()); + + final SimpleHttpResponse second = client.execute( + SimpleRequestProducer.create(SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/private") + .build()), + SimpleResponseConsumer.create(), + HttpCacheContext.create(), + null).get(1, TimeUnit.MINUTES); + Assertions.assertEquals(401, second.getCode()); + Assertions.assertEquals(3, originHits.get()); + } finally { + client.close(CloseMode.IMMEDIATE); + } + } + + private void handleOrigin(final HttpExchange exchange) throws IOException { + originHits.incrementAndGet(); + exchange.getResponseHeaders().add("Date", DateUtils.formatStandardDate(Instant.now())); + final String authorization = exchange.getRequestHeaders().getFirst("Authorization"); + if (authorization == null) { + exchange.getResponseHeaders().add("WWW-Authenticate", "Basic realm=\"test\""); + exchange.sendResponseHeaders(401, -1); + exchange.close(); + return; + } + final byte[] body = "private data of alice".getBytes(StandardCharsets.US_ASCII); + exchange.getResponseHeaders().add("Cache-Control", "max-age=60"); + exchange.sendResponseHeaders(200, body.length); + try (final OutputStream out = exchange.getResponseBody()) { + out.write(body); + } + } +}