From 74ba76e58576ed5f20a0f35f82b279bcbba9a4f5 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Wed, 8 Jan 2025 22:55:50 +0100 Subject: [PATCH] simplify --- .../impl/cache/ResponseCachingPolicy.java | 51 ++++++++----------- .../impl/cache/TestResponseCachingPolicy.java | 29 +++-------- 2 files changed, 26 insertions(+), 54 deletions(-) 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 f406fcf06..9672fe2dc 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 @@ -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 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/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestResponseCachingPolicy.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestResponseCachingPolicy.java index 9bd036555..bdddf7cfe 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestResponseCachingPolicy.java +++ b/httpclient5-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."); } } \ No newline at end of file