simplify

apache · Jan 8, 2025 · 74ba76e · 74ba76e
1 parent 0f21d1a
commit 74ba76e
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 54 deletions.
diff --git a/...ent5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java b/...ent5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java
@@ -142,9 +142,13 @@ public boolean isResponseCacheable(final ResponseCacheControl cacheControl, fina
             return false;
         }
 
-        if (response.countHeaders(HttpHeaders.DATE) > 1) {
-            LOG.debug("Multiple Date headers");
-            return false;
+        if (sharedCache) {
+            if (request.containsHeader(HttpHeaders.AUTHORIZATION) &&
+                    cacheControl.getSharedMaxAge() == -1 &&
+                    !(cacheControl.isPublic() || cacheControl.isMustRevalidate())) {
+                LOG.debug("Request contains private credentials");
+                return false;
+            }
         }
 
         // See if the response is tainted
@@ -153,6 +157,19 @@ public boolean isResponseCacheable(final ResponseCacheControl cacheControl, fina
             return false;
         }
 
+        if (response.countHeaders(HttpHeaders.DATE) > 1) {
+            LOG.debug("Multiple Date headers");
+            return false;
+        }
+
+        final Instant responseDate = DateUtils.parseStandardDate(response, HttpHeaders.DATE);
+        final Instant responseExpires = DateUtils.parseStandardDate(response, HttpHeaders.EXPIRES);
+
+        if (expiresHeaderLessOrEqualToDateHeaderAndNoCacheControl(cacheControl, responseDate, responseExpires)) {
+            LOG.debug("Expires header less or equal to Date header and no cache control directives");
+            return false;
+        }
+
         // Treat responses with `Vary: *` as essentially non-cacheable.
         final Iterator<String> it = MessageSupport.iterateTokens(response, HttpHeaders.VARY);
         while (it.hasNext()) {
@@ -165,31 +182,6 @@ public boolean isResponseCacheable(final ResponseCacheControl cacheControl, fina
             }
         }
 
-        if (sharedCache) {
-            if (request.containsHeader(HttpHeaders.AUTHORIZATION)) {
-                if (cacheControl.getSharedMaxAge() != -1 ||
-                        cacheControl.isMustRevalidate() ||
-                        cacheControl.isPublic()) {
-                    LOG.debug("Request with Authorization header can be cached due to must-revalidate, s-maxage, or public directive");
-                    return true;
-                } else {
-                    LOG.debug("Request contains private credentials and cannot be cached without must-revalidate, s-maxage, or public directive");
-                    return false;
-                }
-            }
-        }
-
-
-        final Instant responseDate = DateUtils.parseStandardDate(response, HttpHeaders.DATE);
-        final Instant responseExpires = DateUtils.parseStandardDate(response, HttpHeaders.EXPIRES);
-
-        if (expiresHeaderLessOrEqualToDateHeaderAndNoCacheControl(cacheControl, responseDate, responseExpires)) {
-            LOG.debug("Expires header less or equal to Date header and no cache control directives");
-            return false;
-        }
-
-
-
         return isExplicitlyCacheable(cacheControl, response) || isHeuristicallyCacheable(cacheControl, code, responseDate, responseExpires);
     }
 
@@ -256,9 +248,6 @@ protected boolean isExplicitlyCacheable(final ResponseCacheControl cacheControl,
         if (response.containsHeader(HttpHeaders.EXPIRES)) {
             return true;
         }
-        if (cacheControl.getMaxAge() == 0 && cacheControl.isMustRevalidate()) {
-            return true;  // Cacheable but immediately stale, must be revalidated
-        }
         if (cacheControl.getMaxAge() > 0) {
             return true;
         }

diff --git a/...-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestResponseCachingPolicy.java b/...-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestResponseCachingPolicy.java
@@ -945,29 +945,13 @@ void testImmutableAndFreshResponseIsCacheable() {
         Assertions.assertTrue(policy.isResponseCacheable(responseCacheControl, request, response));
     }
 
-    @Test
-    void testMustRevalidateWithAuthorizationIsCacheable() {
-        request = new BasicHttpRequest("GET", "/resource");
-        request.setHeader(HttpHeaders.AUTHORIZATION, "Basic dXNlcjpwYXNzd2Q=");
-        response.setHeader("Cache-Control", "must-revalidate, max-age=0");
-        responseCacheControl = ResponseCacheControl.builder()
-                .setMustRevalidate(true)
-                .setMaxAge(0)
-                .build();
-
-        final boolean isCacheable = policy.isResponseCacheable(responseCacheControl, request, response);
-        Assertions.assertTrue(isCacheable,
-                "Response with must-revalidate and Authorization header should be cacheable.");
-    }
-
     @Test
     void testPublicWithAuthorizationIsCacheable() {
         request = new BasicHttpRequest("GET", "/resource");
         request.setHeader(HttpHeaders.AUTHORIZATION, "Basic dXNlcjpwYXNzd2Q=");
-        response.setHeader("Cache-Control", "public, max-age=0");
+        response.setHeader("Cache-Control", "public");
         responseCacheControl = ResponseCacheControl.builder()
                 .setCachePublic(true)
-                .setSharedMaxAge(0)
                 .build();
 
         final boolean isCacheable = policy.isResponseCacheable(responseCacheControl, request, response);
@@ -993,9 +977,8 @@ void testSMaxageWithAuthorizationIsCacheable() {
     void testNoDirectivesWithAuthorizationNotCacheable() {
         request = new BasicHttpRequest("GET", "/resource");
         request.setHeader(HttpHeaders.AUTHORIZATION, "Basic dXNlcjpwYXNzd2Q=");
-        response.setHeader("Cache-Control", "max-age=0");
+        response.setHeader("Cache-Control", "");
         responseCacheControl = ResponseCacheControl.builder()
-                .setMaxAge(0)
                 .build();
 
         final boolean isCacheable = policy.isResponseCacheable(responseCacheControl, request, response);
@@ -1004,17 +987,17 @@ void testNoDirectivesWithAuthorizationNotCacheable() {
     }
 
     @Test
-    void testMustRevalidateWithMaxAgeZeroIsCacheable() {
+    void testMustRevalidateWithAuthorizationIsCacheable() {
         request = new BasicHttpRequest("GET", "/resource");
-        response.setHeader("Cache-Control", "must-revalidate, max-age=0");
+        request.setHeader(HttpHeaders.AUTHORIZATION, "Basic dXNlcjpwYXNzd2Q=");
+        response.setHeader("Cache-Control", "must-revalidate");
         responseCacheControl = ResponseCacheControl.builder()
                 .setMustRevalidate(true)
-                .setMaxAge(0)
                 .build();
 
         final boolean isCacheable = policy.isResponseCacheable(responseCacheControl, request, response);
         Assertions.assertTrue(isCacheable,
-                "Response with must-revalidate and max-age=0 should be cacheable but immediately stale.");
+                "Response with must-revalidate and Authorization header should be cacheable in shared cache.");
     }
 
 }