Refactor CacheControl parser to handle multiple headers.

This commit refactors the CacheControl parsing logic to handle multiple "Cache-Control" headers. The previous implementation treated each header independently, returning an array of CacheControl objects. This caused issues when headers had directives that should be combined into a single CacheControl object.
The updated implementation combines all directives from all headers into a single CacheControl object, ensuring accurate representation of the caching directives.

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

@@ -28,6 +28,7 @@ package org.apache.hc.client5.http.impl.cache;
 
 import java.util.BitSet;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.Set;
 
 import org.apache.hc.client5.http.cache.HeaderConstants;
@@ -50,7 +51,7 @@ import org.slf4j.LoggerFactory;
  * This class is thread-safe and has a singleton instance ({@link #INSTANCE}).
  * </p>
  * <p>
- * The {@link #parse(Header)} method takes an HTTP header and returns a {@link CacheControl} object containing
+ * The {@link #parse(Iterator)} method takes an HTTP header and returns a {@link CacheControl} object containing
  * the relevant caching directives. The header can be either a {@link FormattedHeader} object, which contains a
  * pre-parsed {@link CharArrayBuffer}, or a plain {@link Header} object, in which case the value will be parsed and
  * stored in a new {@link CharArrayBuffer}.
@@ -113,28 +114,14 @@ class CacheControlHeaderParser {
      * <p>
      * "s-maxage" (-1).</p>
      *
-     * @param header the header to parse, cannot be {@code null}
+     * @param headerIterator the header to parse, cannot be {@code null}
      * @return a new {@link CacheControl} instance containing the relevant caching directives parsed from the header
      * @throws IllegalArgumentException if the input header is {@code null}
      */
-    public final CacheControl parse(final Header header) {
+    public final CacheControl parse(final Iterator<Header> headerIterator) {
-        Args.notNull(header, "Header");
+        Args.notNull(headerIterator, "headerIterator");
-
-        final CharArrayBuffer buffer;
-        final Tokenizer.Cursor cursor;
-        if (header instanceof FormattedHeader) {
-            buffer = ((FormattedHeader) header).getBuffer();
-            cursor = new Tokenizer.Cursor(((FormattedHeader) header).getValuePos(), buffer.length());
-        } else {
-            final String s = header.getValue();
-            if (s == null) {
-                return new CacheControl();
-            }
-            buffer = new CharArrayBuffer(s.length());
-            buffer.append(s);
-            cursor = new Tokenizer.Cursor(0, buffer.length());
-        }
 
+        // Initialize variables to hold the Cache-Control directives
         long maxAge = -1;
         long sharedMaxAge = -1;
         boolean noCache = false;
@@ -144,55 +131,76 @@ class CacheControlHeaderParser {
         boolean proxyRevalidate = false;
         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()) {
+        // Iterate over each header
-            final String name = tokenParser.parseToken(buffer, cursor, TOKEN_DELIMS);
+        while (headerIterator.hasNext()) {
-            String value = null;
+            final Header header = headerIterator.next();
-            if (!cursor.atEnd()) {
+            final CharArrayBuffer buffer;
-                final int valueDelim = buffer.charAt(cursor.getPos());
+            final Tokenizer.Cursor cursor;
-                cursor.updatePos(cursor.getPos() + 1);
+            if (header instanceof FormattedHeader) {
-                if (valueDelim == EQUAL_CHAR) {
+                buffer = ((FormattedHeader) header).getBuffer();
-                    value = tokenParser.parseValue(buffer, cursor, VALUE_DELIMS);
+                cursor = new Tokenizer.Cursor(((FormattedHeader) header).getValuePos(), buffer.length());
-                    if (!cursor.atEnd()) {
+            } else {
-                        cursor.updatePos(cursor.getPos() + 1);
+                final String s = header.getValue();
-                    }
+                if (s == null) {
+                    return new CacheControl();
                 }
+                buffer = new CharArrayBuffer(s.length());
+                buffer.append(s);
+                cursor = new Tokenizer.Cursor(0, buffer.length());
             }
-            if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_S_MAX_AGE)) {
+
-                sharedMaxAge = parseSeconds(name, value);
+            // Parse the header
-            } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_MAX_AGE)) {
+            while (!cursor.atEnd()) {
-                maxAge = parseSeconds(name, value);
+                final String name = tokenParser.parseToken(buffer, cursor, TOKEN_DELIMS);
-            } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_MUST_REVALIDATE)) {
+                String value = null;
-                mustRevalidate = true;
+                if (!cursor.atEnd()) {
-            } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_NO_CACHE)) {
+                    final int valueDelim = buffer.charAt(cursor.getPos());
-                noCache = true;
+                    cursor.updatePos(cursor.getPos() + 1);
-                if (value != null) {
+                    if (valueDelim == EQUAL_CHAR) {
-                    final Tokenizer.Cursor valCursor = new ParserCursor(0, value.length());
+                        value = tokenParser.parseValue(buffer, cursor, VALUE_DELIMS);
-                    while (!valCursor.atEnd()) {
+                        if (!cursor.atEnd()) {
-                        final String token = tokenParser.parseToken(value, valCursor, VALUE_DELIMS);
+                            cursor.updatePos(cursor.getPos() + 1);
-                        if (!TextUtils.isBlank(token)) {
-                            noCacheFields.add(token);
-                        }
-                        if (!valCursor.atEnd()) {
-                            valCursor.updatePos(valCursor.getPos() + 1);
                         }
                     }
                 }
-            } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_NO_STORE)) {
+                // Update the Cache-Control directives based on the current header
-                noStore = true;
+                if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_S_MAX_AGE)) {
-            } else if (name.equalsIgnoreCase(HeaderConstants.PRIVATE)) {
+                    sharedMaxAge = parseSeconds(name, value);
-                cachePrivate = true;
+                } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_MAX_AGE)) {
-            } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_PROXY_REVALIDATE)) {
+                    maxAge = parseSeconds(name, value);
-                proxyRevalidate = true;
+                } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_MUST_REVALIDATE)) {
-            } else if (name.equalsIgnoreCase(HeaderConstants.PUBLIC)) {
+                    mustRevalidate = true;
-                cachePublic = true;
+                } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_NO_CACHE)) {
-            } else if (name.equalsIgnoreCase(HeaderConstants.STALE_WHILE_REVALIDATE)) {
+                    noCache = true;
-                staleWhileRevalidate = parseSeconds(name, value);
+                    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)) {
+                    cachePrivate = true;
+                } else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_PROXY_REVALIDATE)) {
+                    proxyRevalidate = true;
+                } else if (name.equalsIgnoreCase(HeaderConstants.PUBLIC)) {
+                    cachePublic = true;
+                } else if (name.equalsIgnoreCase(HeaderConstants.STALE_WHILE_REVALIDATE)) {
+                    staleWhileRevalidate = parseSeconds(name, value);
+                }
             }
         }
+        // Return a single CacheControl object with the combined directives
         return new CacheControl(maxAge, sharedMaxAge, mustRevalidate, noCache, noStore, cachePrivate, proxyRevalidate, cachePublic, staleWhileRevalidate, noCacheFields);
     }
 

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

@@ -383,11 +383,11 @@ public class CachingExecBase {
      * false otherwise
      */
     boolean requestContainsNoCacheDirective(final HttpRequest request) {
-        final Header cacheControlHeader = request.getFirstHeader(HttpHeaders.CACHE_CONTROL);
+        final Iterator<Header> it = request.headerIterator(HttpHeaders.CACHE_CONTROL);
-        if (cacheControlHeader == null) {
+        if (it == null || !it.hasNext()) {
             return false;
         } else {
-            final CacheControl cacheControl = CacheControlHeaderParser.INSTANCE.parse(cacheControlHeader);
+            final CacheControl cacheControl = CacheControlHeaderParser.INSTANCE.parse(it);
             return cacheControl.isNoCache();
         }
     }

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

@@ -535,11 +535,11 @@ class ResponseCachingPolicy {
      * @return a CacheControl instance with the parsed directives or default values if the header is not present
      */
     private CacheControl parseCacheControlHeader(final MessageHeaders messageHeaders) {
-        final Header cacheControlHeader = messageHeaders.getFirstHeader(HttpHeaders.CACHE_CONTROL);
+        final Iterator<Header> it = messageHeaders.headerIterator(HttpHeaders.CACHE_CONTROL);
-        if (cacheControlHeader == null) {
+        if (it == null || !it.hasNext()) {
             return null;
         } else {
-            return CacheControlHeaderParser.INSTANCE.parse(cacheControlHeader);
+            return CacheControlHeaderParser.INSTANCE.parse(it);
         }
     }
 

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

@@ -35,6 +35,9 @@ import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.message.BasicHeader;
 import org.junit.jupiter.api.Test;
 
+import java.util.Arrays;
+import java.util.Collections;
+
 public class CacheControlParserTest {
 
     private final CacheControlHeaderParser parser = CacheControlHeaderParser.INSTANCE;
@@ -42,69 +45,69 @@ public class CacheControlParserTest {
     @Test
     public void testParseMaxAgeZero() {
         final Header header = new BasicHeader("Cache-Control", "max-age=0 , this = stuff;");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
         assertEquals(0L, cacheControl.getMaxAge());
     }
 
     @Test
     public void testParseSMaxAge() {
         final Header header = new BasicHeader("Cache-Control", "s-maxage=3600");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
         assertEquals(3600L, cacheControl.getSharedMaxAge());
     }
 
     @Test
     public void testParseInvalidCacheValue() {
         final Header header = new BasicHeader("Cache-Control", "max-age=invalid");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
         assertEquals(-1L, cacheControl.getMaxAge());
     }
 
     @Test
     public void testParseInvalidHeader() {
         final Header header = new BasicHeader("Cache-Control", "max-age");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
         assertEquals(-1L, cacheControl.getMaxAge());
     }
 
     @Test
     public void testParseNullHeader() {
         final Header header = null;
-        assertThrows(NullPointerException.class, () -> parser.parse(header));
+        assertThrows(NullPointerException.class, () -> parser.parse(Collections.singletonList(header).iterator()));
     }
 
     @Test
     public void testParseEmptyHeader() {
         final Header header = new BasicHeader("Cache-Control", "");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
         assertEquals(-1L, cacheControl.getMaxAge());
     }
 
     @Test
     public void testParseCookieEmptyValue() {
         final Header header = new BasicHeader("Cache-Control", "max-age=;");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
         assertEquals(-1L, cacheControl.getMaxAge());
     }
 
     @Test
     public void testParseNoCache() {
         final Header header = new BasicHeader(" Cache-Control", "no-cache");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
         assertEquals(-1L, cacheControl.getMaxAge());
     }
 
     @Test
     public void testParseNoDirective() {
         final Header header = new BasicHeader(" Cache-Control", "");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
         assertEquals(-1L, cacheControl.getMaxAge());
     }
 
     @Test
     public void testGarbage() {
         final Header header = new BasicHeader("Cache-Control", ",,= blah,");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
         assertEquals(-1L, cacheControl.getMaxAge());
     }
 
@@ -112,7 +115,7 @@ public class CacheControlParserTest {
     @Test
     public void testParseMultipleDirectives() {
         final Header header = new BasicHeader("Cache-Control", "max-age=604800, stale-while-revalidate=86400, s-maxage=3600, must-revalidate, private");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
 
         assertAll("Must all pass",
                 () -> assertEquals(604800L, cacheControl.getMaxAge()),
@@ -125,7 +128,7 @@ public class CacheControlParserTest {
     @Test
     public void testParseMultipleDirectives2() {
         final Header header = new BasicHeader("Cache-Control", "max-age=604800, stale-while-revalidate=86400, must-revalidate, private, s-maxage=3600");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
 
         assertAll("Must all pass",
                 () -> assertEquals(604800L, cacheControl.getMaxAge()),
@@ -138,7 +141,7 @@ public class CacheControlParserTest {
     @Test
     public void testParsePublic() {
         final Header header = new BasicHeader("Cache-Control", "public");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
 
         assertTrue(cacheControl.isPublic());
     }
@@ -146,7 +149,7 @@ public class CacheControlParserTest {
     @Test
     public void testParsePrivate() {
         final Header header = new BasicHeader("Cache-Control", "private");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
 
         assertTrue(cacheControl.isCachePrivate());
     }
@@ -154,7 +157,7 @@ public class CacheControlParserTest {
     @Test
     public void testParseNoStore() {
         final Header header = new BasicHeader("Cache-Control", "no-store");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
 
         assertTrue(cacheControl.isNoStore());
     }
@@ -162,7 +165,7 @@ public class CacheControlParserTest {
     @Test
     public void testParseStaleWhileRevalidate() {
         final Header header = new BasicHeader("Cache-Control", "max-age=3600, stale-while-revalidate=120");
-        final CacheControl cacheControl = parser.parse(header);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
 
         assertEquals(120, cacheControl.getStaleWhileRevalidate());
     }
@@ -170,7 +173,7 @@ public class CacheControlParserTest {
     @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);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
 
         assertTrue(cacheControl.isNoCache());
         assertEquals(2, cacheControl.getNoCacheFields().size());
@@ -182,7 +185,7 @@ public class CacheControlParserTest {
     @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);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
 
         assertTrue(cacheControl.isNoCache());
         assertEquals(1, cacheControl.getNoCacheFields().size());
@@ -193,7 +196,7 @@ public class CacheControlParserTest {
     @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);
+        final CacheControl cacheControl = parser.parse(Collections.singletonList(header).iterator());
 
         assertTrue(cacheControl.isNoCache());
         assertEquals(2, cacheControl.getNoCacheFields().size());
@@ -202,4 +205,25 @@ public class CacheControlParserTest {
         assertEquals(120, cacheControl.getStaleWhileRevalidate());
     }
 
+
+    @Test
+    public void testParseMultipleHeaders() {
+        // Create headers
+        final Header header1 = new BasicHeader("Cache-Control", "max-age=3600, no-store");
+        final Header header2 = new BasicHeader("Cache-Control", "private, must-revalidate");
+        final Header header3 = new BasicHeader("Cache-Control", "max-age=3600");
+        final Header header4 = new BasicHeader("Cache-Control", "no-store");
+        final Header header5 = new BasicHeader("Cache-Control", "private");
+        final Header header6 = new BasicHeader("Cache-Control", "must-revalidate");
+
+        // Parse headers
+        final CacheControl cacheControl1 = parser.parse(Arrays.asList(header1, header2).iterator());
+        final CacheControl cacheControl2 = parser.parse(Arrays.asList(header3, header4, header5, header6).iterator());
+
+        // Validate Cache-Control directives
+        assertEquals(cacheControl1.getMaxAge(), cacheControl2.getMaxAge());
+        assertEquals(cacheControl1.isNoStore(), cacheControl2.isNoStore());
+        assertEquals(cacheControl1.isCachePrivate(), cacheControl2.isCachePrivate());
+        assertEquals(cacheControl1.isMustRevalidate(), cacheControl2.isMustRevalidate());
+    }
 }

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

@@ -381,7 +381,7 @@ public class TestResponseCachingPolicy {
         response.addHeader("Cache-Control", "max-age=20");
         response.addHeader("Cache-Control", "public, no-store");
 
-        Assertions.assertTrue(policy.isResponseCacheable("GET", response));
+        Assertions.assertFalse(policy.isResponseCacheable("GET", response));
     }
 
     @Test
@@ -389,7 +389,7 @@ public class TestResponseCachingPolicy {
         response.addHeader("Cache-Control", "max-age=20");
         response.addHeader("Cache-Control", "public, no-store");
 
-        Assertions.assertTrue(policy.isResponseCacheable("HEAD", response));
+        Assertions.assertFalse(policy.isResponseCacheable("HEAD", response));
     }
 
     @Test