Handle "no-cache" directive with specific header fields.

This commit enhances the handling of the "no-cache" directive when used with specified header fields. The changes include:

* Updated the caching module to correctly identify and handle "no-cache" directives with specific header fields. This was done by modifying the `responseContainsNoCacheDirective` method.
* Modified the `handleCacheHit` method to ensure that a cached entry is revalidated with the origin server when a "no-cache" directive with specific header fields is present in the response.
* Ensured that the rest of the response is still cacheable when the specified header fields are present, as long as the response complies with other caching requirements.

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

@@ -623,7 +623,22 @@ class AsyncCachingExec extends CachingExecBase implements AsyncExecChainHandler
         final HttpClientContext context  = scope.clientContext;
         recordCacheHit(target, request);
         final Instant now = getCurrentDate();
+
+        if (requestContainsNoCacheDirective(request)) {
+            // Revalidate with the server due to no-cache directive in response
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Revalidating with server due to no-cache directive in response.");
+            }
+            revalidateCacheEntry(target, request, entityProducer, scope, chain, asyncExecCallback, entry);
+            return;
+        }
+
         if (suitabilityChecker.canCachedResponseBeUsed(request, entry, now)) {
+            if (responseCachingPolicy.responseContainsNoCacheDirective(entry)) {
+                // Revalidate with the server due to no-cache directive in response
+                revalidateCacheEntry(target, request, entityProducer, scope, chain, asyncExecCallback, entry);
+                return;
+            }
             LOG.debug("Cache hit");
             try {
                 final SimpleHttpResponse cacheResponse = generateCachedResponse(request, context, entry, now);

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

@@ -31,6 +31,9 @@ import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 
+import java.util.Collections;
+import java.util.Set;
+
 /**
  * Represents the values of the Cache-Control header in an HTTP response, which indicate whether and for how long
  * the response can be cached by the client and intermediary proxies.
@@ -92,16 +95,22 @@ final class CacheControl {
      * The number of seconds that a stale response is considered fresh for the purpose
      * of serving a response while a revalidation request is made to the origin server.
      */
-    private final long stale_while_revalidate;
+    private final long staleWhileRevalidate;
+
+    /**
+     * A set of field names specified in the "no-cache" directive of the Cache-Control header.
+     */
+    private final Set<String> noCacheFields;
 
 
     /**
      * Creates a new instance of {@code CacheControl} with default values.
      * The default values are: max-age=-1, shared-max-age=-1, must-revalidate=false, no-cache=false,
-     * no-store=false, private=false, proxy-revalidate=false, public=false and stale_while_revalidate=-1.
+     * no-store=false, private=false, proxy-revalidate=false, public=false, stale_while_revalidate=-1,
+     * and noCacheFields as an empty set.
      */
     public CacheControl() {
-        this(-1, -1, false, false, false, false, false, false,-1);
+        this(-1, -1, false, false, false, false, false, false,-1, Collections.emptySet());
     }
 
     /**
@@ -116,10 +125,12 @@ final class CacheControl {
      * @param cachePrivate    The private value from the Cache-Control header.
      * @param proxyRevalidate The proxy-revalidate value from the Cache-Control header.
      * @param cachePublic     The public value from the Cache-Control header.
+     * @param noCacheFields        The set of field names specified in the "no-cache" directive of the Cache-Control header.
      */
     public CacheControl(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 stale_while_revalidate) {
+                        final long staleWhileRevalidate,
+                        final Set<String> noCacheFields) {
         this.maxAge = maxAge;
         this.sharedMaxAge = sharedMaxAge;
         this.noCache = noCache;
@@ -128,7 +139,8 @@ final class CacheControl {
         this.mustRevalidate = mustRevalidate;
         this.proxyRevalidate = proxyRevalidate;
         this.cachePublic = cachePublic;
-        this.stale_while_revalidate = stale_while_revalidate;
+        this.staleWhileRevalidate = staleWhileRevalidate;
+        this.noCacheFields = Collections.unmodifiableSet(noCacheFields);
     }
 
 
@@ -211,12 +223,21 @@ final class CacheControl {
      * @return The stale-while-revalidate value.
      */
     public long getStaleWhileRevalidate() {
-        return stale_while_revalidate;
+        return staleWhileRevalidate;
+    }
+
+    /**
+     * Returns an unmodifiable set of field names specified in the "no-cache" directive of the Cache-Control header.
+     *
+     * @return The set of field names specified in the "no-cache" directive.
+     */
+    public Set<String> getNoCacheFields() {
+        return noCacheFields;
     }
 
     /**
      * Returns a string representation of the {@code CacheControl} object, including the max-age, shared-max-age, no-cache,
-     * no-store, private, must-revalidate, proxy-revalidate, and public values.
+     * no-store, private, must-revalidate, proxy-revalidate, public values, and noCacheFields.
      *
      * @return A string representation of the object.
      */
@@ -226,12 +247,13 @@ final class CacheControl {
                 "maxAge=" + maxAge +
                 ", sharedMaxAge=" + sharedMaxAge +
                 ", noCache=" + noCache +
+                ", noCacheFields=" + noCacheFields +
                 ", noStore=" + noStore +
                 ", cachePrivate=" + cachePrivate +
                 ", mustRevalidate=" + mustRevalidate +
                 ", proxyRevalidate=" + proxyRevalidate +
                 ", cachePublic=" + cachePublic +
-                ", stale_while_revalidate=" + stale_while_revalidate +
+                ", stale_while_revalidate=" + staleWhileRevalidate +
                 '}';
     }
 }

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

@@ -27,6 +27,8 @@
 package org.apache.hc.client5.http.impl.cache;
 
 import java.util.BitSet;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.apache.hc.client5.http.cache.HeaderConstants;
 import org.apache.hc.core5.annotation.Contract;
@@ -34,8 +36,10 @@ import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.FormattedHeader;
 import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.message.ParserCursor;
 import org.apache.hc.core5.util.Args;
 import org.apache.hc.core5.util.CharArrayBuffer;
+import org.apache.hc.core5.util.TextUtils;
 import org.apache.hc.core5.util.Tokenizer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -141,6 +145,9 @@ class CacheControlHeaderParser {
         boolean cachePublic = false;
         long staleWhileRevalidate = -1;
 
+        // Declare a new Set variable at the beginning of the method to store the field-names
+        final Set<String> noCacheFields = new HashSet<>();
+
         while (!cursor.atEnd()) {
             final String name = tokenParser.parseToken(buffer, cursor, TOKEN_DELIMS);
             String value = null;
@@ -162,6 +169,18 @@ class CacheControlHeaderParser {
                 mustRevalidate = true;
             } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_NO_CACHE)) {
                 noCache = true;
+                if (value != null) {
+                    final Tokenizer.Cursor valCursor = new ParserCursor(0, value.length());
+                    while (!valCursor.atEnd()) {
+                        final String token = tokenParser.parseToken(value, valCursor, VALUE_DELIMS);
+                        if (!TextUtils.isBlank(token)) {
+                            noCacheFields.add(token);
+                        }
+                        if (!valCursor.atEnd()) {
+                            valCursor.updatePos(valCursor.getPos() + 1);
+                        }
+                    }
+                }
             } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_NO_STORE)) {
                 noStore = true;
             } else if (name.equalsIgnoreCase(HeaderConstants.PRIVATE)) {
@@ -174,7 +193,7 @@ class CacheControlHeaderParser {
                 staleWhileRevalidate = parseSeconds(name, value);
             }
         }
-        return new CacheControl(maxAge, sharedMaxAge, mustRevalidate, noCache, noStore, cachePrivate, proxyRevalidate, cachePublic, staleWhileRevalidate);
+        return new CacheControl(maxAge, sharedMaxAge, mustRevalidate, noCache, noStore, cachePrivate, proxyRevalidate, cachePublic, staleWhileRevalidate, noCacheFields);
     }
 
     private static long parseSeconds(final String name, final String value) {

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

@@ -286,7 +286,21 @@ class CachingExec extends CachingExecBase implements ExecChainHandler {
         context.setAttribute(HttpCoreContext.HTTP_REQUEST, request);
         recordCacheHit(target, request);
         final Instant now = getCurrentDate();
+
+
+        if (requestContainsNoCacheDirective(request)) {
+            // Revalidate with the server
+            return revalidateCacheEntry(target, request, scope, chain, entry);
+        }
+
         if (suitabilityChecker.canCachedResponseBeUsed(request, entry, now)) {
+            if (responseCachingPolicy.responseContainsNoCacheDirective(entry)) {
+                // Revalidate with the server due to no-cache directive in response
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("Revalidating with server due to no-cache directive in response.");
+                }
+                return revalidateCacheEntry(target, request, scope, chain, entry);
+            }
             LOG.debug("Cache hit");
             try {
                 return convert(generateCachedResponse(request, context, entry, now), scope);

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

@@ -369,4 +369,26 @@ public class CachingExecBase {
             }
         }
     }
+
+    /**
+     * Checks if the provided HttpRequest contains a 'no-cache' directive in its Cache-Control header.
+     * <p>
+     * According to the HTTP/1.1 specification, a 'no-cache' directive in a request message means
+     * that the client allows a stored response to be used only if it is first validated with the
+     * origin server (or with an intermediate cache that has a fresh response). This directive
+     * applies to both shared and private caches.
+     *
+     * @param request the HttpRequest to check for the 'no-cache' directive
+     * @return true if the 'no-cache' directive is present in the Cache-Control header of the request,
+     * false otherwise
+     */
+    boolean requestContainsNoCacheDirective(final HttpRequest request) {
+        final Header cacheControlHeader = request.getFirstHeader(HttpHeaders.CACHE_CONTROL);
+        if (cacheControlHeader == null) {
+            return false;
+        } else {
+            final CacheControl cacheControl = CacheControlHeaderParser.INSTANCE.parse(cacheControlHeader);
+            return cacheControl.isNoCache();
+        }
+    }
 }

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

@@ -270,11 +270,29 @@ class ResponseCachingPolicy {
         return status < 500 || status > 505;
     }
 
+    /**
+     * Determines whether the given CacheControl object indicates that the response is explicitly non-cacheable.
+     *
+     * @param cacheControl the CacheControl object representing the cache-control directive(s) from the HTTP response.
+     * @return true if the response is explicitly non-cacheable according to the cache-control directive(s),
+     * false otherwise.
+     * <p>
+     * When cacheControl is non-null:
+     * - Returns true if the response contains "no-store" or (if sharedCache is true) "private" cache-control directives.
+     * - If the response contains the "no-cache" directive, it is considered cacheable, but requires validation against
+     * the origin server before use. In this case, the method returns false.
+     * - Returns false for other cache-control directives, implying the response is cacheable.
+     * <p>
+     * When cacheControl is null, returns false, implying the response is cacheable.
+     */
     protected boolean isExplicitlyNonCacheable(final CacheControl cacheControl) {
         if (cacheControl == null) {
             return false;
-        }else {
-            return cacheControl.isNoStore() || cacheControl.isNoCache() || (sharedCache && cacheControl.isCachePrivate());
+        } else {
+            // The response is considered explicitly non-cacheable if it contains
+            // "no-store" or (if sharedCache is true) "private" directives.
+            // Note that "no-cache" is considered cacheable but requires validation before use.
+            return cacheControl.isNoStore() || (sharedCache && cacheControl.isCachePrivate());
         }
     }
 
@@ -525,4 +543,48 @@ class ResponseCachingPolicy {
         }
     }
 
+    /**
+     * Determines if the given {@link HttpCacheEntry} requires revalidation based on the presence of the {@code no-cache} directive
+     * in the Cache-Control header.
+     * <p>
+     * The method returns true in the following cases:
+     * - If the {@code no-cache} directive is present without any field names.
+     * - If the {@code no-cache} directive is present with field names, and at least one of these field names is present
+     * in the headers of the {@link HttpCacheEntry}.
+     * <p>
+     * If the {@code no-cache} directive is not present in the Cache-Control header, the method returns {@code false}.
+     *
+     * @param entry the  {@link HttpCacheEntry} containing the headers to check for the {@code no-cache} directive.
+     * @return true if revalidation is required based on the {@code no-cache} directive, {@code false} otherwise.
+     */
+    boolean responseContainsNoCacheDirective(final HttpCacheEntry entry) {
+        final CacheControl responseCacheControl = parseCacheControlHeader(entry);
+
+        if (responseCacheControl != null) {
+            final Set<String> noCacheFields = responseCacheControl.getNoCacheFields();
+
+            // If no-cache directive is present and has no field names
+            if (responseCacheControl.isNoCache() && noCacheFields.isEmpty()) {
+                LOG.debug("No-cache directive present without field names. Revalidation required.");
+                return true;
+            }
+
+            // If no-cache directive is present with field names
+            if (responseCacheControl.isNoCache()) {
+                for (final String field : noCacheFields) {
+                    if (entry.getFirstHeader(field) != null) {
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("No-cache directive field '{}' found in response headers. Revalidation required.", field);
+                        }
+                        return true;
+                    }
+                }
+            }
+        }
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("No no-cache directives found in response headers. No revalidation required.");
+        }
+        return false;
+    }
+
 }

httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/CacheControlParserTest.java

@@ -26,15 +26,15 @@
  */
 package org.apache.hc.client5.http.impl.cache;
 
-import org.apache.hc.core5.http.Header;
-import org.apache.hc.core5.http.message.BasicHeader;
-import org.junit.jupiter.api.Test;
-
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.message.BasicHeader;
+import org.junit.jupiter.api.Test;
+
 public class CacheControlParserTest {
 
     private final CacheControlHeaderParser parser = CacheControlHeaderParser.INSTANCE;
@@ -166,4 +166,40 @@ public class CacheControlParserTest {
 
         assertEquals(120, cacheControl.getStaleWhileRevalidate());
     }
+
+    @Test
+    public void testParseNoCacheFields() {
+        final Header header = new BasicHeader("Cache-Control", "no-cache=\"Set-Cookie, Content-Language\", stale-while-revalidate=120");
+        final CacheControl cacheControl = parser.parse(header);
+
+        assertTrue(cacheControl.isNoCache());
+        assertEquals(2, cacheControl.getNoCacheFields().size());
+        assertTrue(cacheControl.getNoCacheFields().contains("Set-Cookie"));
+        assertTrue(cacheControl.getNoCacheFields().contains("Content-Language"));
+        assertEquals(120, cacheControl.getStaleWhileRevalidate());
+    }
+
+    @Test
+    public void testParseNoCacheFieldsNoQuote() {
+        final Header header = new BasicHeader("Cache-Control", "no-cache=Set-Cookie, Content-Language, stale-while-revalidate=120");
+        final CacheControl cacheControl = parser.parse(header);
+
+        assertTrue(cacheControl.isNoCache());
+        assertEquals(1, cacheControl.getNoCacheFields().size());
+        assertTrue(cacheControl.getNoCacheFields().contains("Set-Cookie"));
+        assertEquals(120, cacheControl.getStaleWhileRevalidate());
+    }
+
+    @Test
+    public void testParseNoCacheFieldsMessy() {
+        final Header header = new BasicHeader("Cache-Control", "no-cache=\"  , , ,,, Set-Cookie  , , Content-Language ,   \", stale-while-revalidate=120");
+        final CacheControl cacheControl = parser.parse(header);
+
+        assertTrue(cacheControl.isNoCache());
+        assertEquals(2, cacheControl.getNoCacheFields().size());
+        assertTrue(cacheControl.getNoCacheFields().contains("Set-Cookie"));
+        assertTrue(cacheControl.getNoCacheFields().contains("Content-Language"));
+        assertEquals(120, cacheControl.getStaleWhileRevalidate());
+    }
+
 }

httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestCachingExecChain.java

@@ -234,7 +234,7 @@ public class TestCachingExecChain {
 
         final ClassicHttpRequest req1 = HttpTestUtils.makeDefaultRequest();
         final ClassicHttpResponse resp1 = HttpTestUtils.make200Response();
-        resp1.setHeader("Cache-Control", "no-cache");
+        resp1.setHeader("Cache-Control", "no-store");
 
         Mockito.when(mockStorage.getEntry(Mockito.any())).thenReturn(null);
         Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);
@@ -1535,4 +1535,38 @@ public class TestCachingExecChain {
         Assertions.assertTrue(warningHeader.get().getValue().contains("110"));
     }
 
+    @Test
+    public void testNoCacheFieldsRevalidation() throws Exception {
+        final Instant now = Instant.now();
+        final Instant fiveSecondsAgo = now.minusSeconds(5);
+
+        final ClassicHttpRequest req1 = HttpTestUtils.makeDefaultRequest();
+        final ClassicHttpResponse resp1 = HttpTestUtils.make200Response();
+        resp1.setHeader("Date", DateUtils.formatStandardDate(now));
+        resp1.setHeader("Cache-Control", "max-age=3100, no-cache=\"Set-Cookie, Content-Language\"");
+        resp1.setHeader("Content-Language", "en-US");
+        resp1.setHeader("Etag", "\"new-etag\"");
+
+        final ClassicHttpRequest req2 = HttpTestUtils.makeDefaultRequest();
+        //req2.setHeader("Cache-Control", "no-cache=\"etag\"");
+        final ClassicHttpResponse resp2 = HttpTestUtils.make200Response();
+        resp2.setHeader("ETag", "\"old-etag\"");
+        resp2.setHeader("Date", DateUtils.formatStandardDate(fiveSecondsAgo));
+        resp2.setHeader("Cache-Control", "max-age=3600");
+
+        final ClassicHttpRequest req3 = HttpTestUtils.makeDefaultRequest();
+
+        Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp1);
+
+        execute(req1);
+
+        Mockito.when(mockExecChain.proceed(Mockito.any(), Mockito.any())).thenReturn(resp2);
+
+        execute(req2);
+        final ClassicHttpResponse result = execute(req3);
+
+        // Verify that the backend was called to revalidate the response, as per the new logic
+        Mockito.verify(mockExecChain, Mockito.times(5)).proceed(Mockito.any(), Mockito.any());
+    }
+
 }

httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestResponseCachingPolicy.java

@@ -45,6 +45,7 @@ import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class TestResponseCachingPolicy {
@@ -307,14 +308,14 @@ public class TestResponseCachingPolicy {
     public void testIsGetWithNoCacheCacheable() {
         response.addHeader("Cache-Control", "no-cache");
 
-        Assertions.assertFalse(policy.isResponseCacheable("GET", response));
+        Assertions.assertTrue(policy.isResponseCacheable("GET", response));
     }
 
     @Test
     public void testIsHeadWithNoCacheCacheable() {
         response.addHeader("Cache-Control", "no-cache");
 
-        Assertions.assertFalse(policy.isResponseCacheable("HEAD", response));
+        Assertions.assertTrue(policy.isResponseCacheable("HEAD", response));
     }
 
     @Test
@@ -349,14 +350,14 @@ public class TestResponseCachingPolicy {
     public void testIsGetWithNoCacheEmbeddedInListCacheable() {
         response.addHeader("Cache-Control", "public, no-cache");
 
-        Assertions.assertFalse(policy.isResponseCacheable("GET", response));
+        Assertions.assertTrue(policy.isResponseCacheable("GET", response));
     }
 
     @Test
     public void testIsHeadWithNoCacheEmbeddedInListCacheable() {
         response.addHeader("Cache-Control", "public, no-cache");
 
-        Assertions.assertFalse(policy.isResponseCacheable("HEAD", response));
+        Assertions.assertTrue(policy.isResponseCacheable("HEAD", response));
     }
 
     @Test
@@ -938,7 +939,7 @@ public class TestResponseCachingPolicy {
 
         response = new BasicHttpResponse(HttpStatus.SC_OK, "");
         response.setHeader(HttpHeaders.DATE, DateUtils.formatStandardDate(Instant.now()));
-       response.setHeader(HttpHeaders.EXPIRES, DateUtils.formatStandardDate(Instant.now().plus(tenSecondsFromNow)));
+        response.setHeader(HttpHeaders.EXPIRES, DateUtils.formatStandardDate(Instant.now().plus(tenSecondsFromNow)));
 
 
         // Create ResponseCachingPolicy instance and test the method
@@ -1011,4 +1012,36 @@ public class TestResponseCachingPolicy {
         response.setHeader("Date", DateUtils.formatStandardDate(now));
         assertTrue(policy.isResponseCacheable(request, response));
     }
+
+    @Test
+    void testIsResponseCacheableNoCache() {
+        // Set up test data
+        response = new BasicHttpResponse(HttpStatus.SC_OK, "");
+        response.setHeader(HttpHeaders.DATE, DateUtils.formatStandardDate(Instant.now()));
+
+        // Create ResponseCachingPolicy instance and test the method
+        policy = new ResponseCachingPolicy(0, true, false, false, false);
+        request = new BasicHttpRequest("GET", "/foo");
+        response.addHeader(HttpHeaders.CACHE_CONTROL, "no-cache");
+        final boolean isCacheable = policy.isResponseCacheable(request, response);
+
+        // Verify the result
+        assertTrue(isCacheable);
+    }
+
+    @Test
+    void testIsResponseCacheableNoStore() {
+        // Set up test data
+        response = new BasicHttpResponse(HttpStatus.SC_OK, "");
+        response.setHeader(HttpHeaders.DATE, DateUtils.formatStandardDate(Instant.now()));
+
+        // Create ResponseCachingPolicy instance and test the method
+        policy = new ResponseCachingPolicy(0, true, false, false, false);
+        request = new BasicHttpRequest("GET", "/foo");
+        response.addHeader(HttpHeaders.CACHE_CONTROL, "no-store");
+        final boolean isCacheable = policy.isResponseCacheable(request, response);
+
+        // Verify the result
+        assertFalse(isCacheable);
+    }
 }