Skip to content

Commit

Permalink
Fix HTTPCLIENT-2354 by updating ResponseCachingPolicy to allow cachin…
Browse files Browse the repository at this point in the history
…g of responses with "must-revalidate, max-age=0" in shared caches with Authorization headers. The change aligns with RFC 9111 Section 5.2.2.2, ensuring responses with "must-revalidate," "s-maxage," or "public" directives are cacheable. This addresses cases where responses with Authorization headers were unnecessarily excluded from caching.
  • Loading branch information
arturobernalg committed Jan 8, 2025
1 parent 3c82807 commit 843ff21
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public boolean isResponseCacheable(final ResponseCacheControl cacheControl, fina
if (sharedCache) {
if (request.containsHeader(HttpHeaders.AUTHORIZATION) &&
cacheControl.getSharedMaxAge() == -1 &&
!cacheControl.isPublic()) {
!(cacheControl.isPublic() || cacheControl.isMustRevalidate())) {
LOG.debug("Request contains private credentials");
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -944,4 +944,60 @@ void testImmutableAndFreshResponseIsCacheable() {

Assertions.assertTrue(policy.isResponseCacheable(responseCacheControl, request, response));
}

@Test
void testPublicWithAuthorizationIsCacheable() {
request = new BasicHttpRequest("GET", "/resource");
request.setHeader(HttpHeaders.AUTHORIZATION, "Basic dXNlcjpwYXNzd2Q=");
response.setHeader("Cache-Control", "public");
responseCacheControl = ResponseCacheControl.builder()
.setCachePublic(true)
.build();

final boolean isCacheable = policy.isResponseCacheable(responseCacheControl, request, response);
Assertions.assertTrue(isCacheable,
"Response with public directive and Authorization header should be cacheable in shared cache.");
}

@Test
void testSMaxageWithAuthorizationIsCacheable() {
request = new BasicHttpRequest("GET", "/resource");
request.setHeader(HttpHeaders.AUTHORIZATION, "Basic dXNlcjpwYXNzd2Q=");
response.setHeader("Cache-Control", "s-maxage=60");
responseCacheControl = ResponseCacheControl.builder()
.setSharedMaxAge(60)
.build();

final boolean isCacheable = policy.isResponseCacheable(responseCacheControl, request, response);
Assertions.assertTrue(isCacheable,
"Response with s-maxage and Authorization header should be cacheable in shared cache.");
}

@Test
void testNoDirectivesWithAuthorizationNotCacheable() {
request = new BasicHttpRequest("GET", "/resource");
request.setHeader(HttpHeaders.AUTHORIZATION, "Basic dXNlcjpwYXNzd2Q=");
response.setHeader("Cache-Control", "");
responseCacheControl = ResponseCacheControl.builder()
.build();

final boolean isCacheable = policy.isResponseCacheable(responseCacheControl, request, response);
Assertions.assertFalse(isCacheable,
"Response without must-revalidate, public, or s-maxage should not be cacheable with Authorization header.");
}

@Test
void testMustRevalidateWithAuthorizationIsCacheable() {
request = new BasicHttpRequest("GET", "/resource");
request.setHeader(HttpHeaders.AUTHORIZATION, "Basic dXNlcjpwYXNzd2Q=");
response.setHeader("Cache-Control", "must-revalidate");
responseCacheControl = ResponseCacheControl.builder()
.setMustRevalidate(true)
.build();

final boolean isCacheable = policy.isResponseCacheable(responseCacheControl, request, response);
Assertions.assertTrue(isCacheable,
"Response with must-revalidate and Authorization header should be cacheable in shared cache.");
}

}

0 comments on commit 843ff21

Please sign in to comment.