Apache
/
httpcomponents-client
mirror of https://github.com/apache/httpcomponents-client.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix issue with duplicate parsing of Cache-Control header 

Previously, the same Cache-Control header was being parsed twice, once by isExplicitlyNonCacheable and again by calculateFreshnessLifetime. The parsing code was extracted from calculateFreshnessLifetime and enhanced to include the main cache control directive that isExplicitlyNonCacheable could use to make its decision. This improves the efficiency and accuracy of the caching logic.
This commit is contained in:
Arturo Bernal 2023-03-13 23:02:48 +01:00 committed by Oleg Kalnichevski
parent cfcdd11cb6
commit 4784fdfed4
6 changed files with 274 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
httpclient5-cache/src/main/java/org/apache/hc/client5/http/cache/HeaderConstants.java vendored
View File

@ -63,6 +63,7 @@ public class HeaderConstants {
public static final String CACHE_CONTROL_NO_STORE = "no-store";
public static final String CACHE_CONTROL_NO_CACHE = "no-cache";
public static final String CACHE_CONTROL_MAX_AGE = "max-age";
public static final String CACHE_CONTROL_S_MAX_AGE = "s-maxage";
public static final String CACHE_CONTROL_MAX_STALE = "max-stale";
public static final String CACHE_CONTROL_MIN_FRESH = "min-fresh";
public static final String CACHE_CONTROL_MUST_REVALIDATE = "must-revalidate";

115
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControl.java vendored
View File

@ -64,19 +64,67 @@ final class CacheControl {
* The shared-max-age directive value.
*/
private final long sharedMaxAge;
/**
* The isNoCache flag indicates whether the Cache-Control header includes the no-cache directive.
*/
private final boolean noCache;
/**
* The isNoStore flag indicates whether the Cache-Control header includes the no-store directive.
*/
private final boolean noStore;
/**
* The isPrivate flag indicates whether the Cache-Control header includes the private directive.
*/
private final boolean cachePrivate;
/**
* Indicates whether the Cache-Control header includes the "must-revalidate" directive.
*/
private final boolean mustRevalidate;
/**
* Indicates whether the Cache-Control header includes the "proxy-revalidate" directive.
*/
private final boolean proxyRevalidate;
/**
* Indicates whether the Cache-Control header includes the "public" directive.
*/
private final boolean cachePublic;
/**
* Creates a new instance of {@code CacheControlHeader} with the specified max-age and shared-max-age values.
* 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, and public=false.
*/
public CacheControl() {
this(-1, -1, false, false, false, false, false, false);
}
/**
* Creates a new instance of {@code CacheControl} with the specified max-age, shared-max-age, no-cache, no-store,
* private, must-revalidate, proxy-revalidate, and public values.
*
* @param maxAge The max-age value from the Cache-Control header.
* @param sharedMaxAge The shared-max-age value from the Cache-Control header.
* @param mustRevalidate The must-revalidate value from the Cache-Control header.
* @param noCache The no-cache value from the Cache-Control header.
* @param noStore The no-store value from the Cache-Control header.
* @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.
*/
public CacheControl(final long maxAge, final long sharedMaxAge) {
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) {
this.maxAge = maxAge;
this.sharedMaxAge = sharedMaxAge;
this.noCache = noCache;
this.noStore = noStore;
this.cachePrivate = cachePrivate;
this.mustRevalidate = mustRevalidate;
this.proxyRevalidate = proxyRevalidate;
this.cachePublic = cachePublic;
}
/**
* Returns the max-age value from the Cache-Control header.
*
@ -95,9 +143,64 @@ final class CacheControl {
return sharedMaxAge;
}
/**
* Returns the no-cache flag from the Cache-Control header.
*
* @return The no-cache flag.
*/
public boolean isNoCache() {
return noCache;
}
/**
* Returns a string representation of the {@code CacheControlHeader} object, including the max-age and shared-max-age values.
* Returns the no-store flag from the Cache-Control header.
*
* @return The no-store flag.
*/
public boolean isNoStore() {
return noStore;
}
/**
* Returns the private flag from the Cache-Control header.
*
* @return The private flag.
*/
public boolean isCachePrivate() {
return cachePrivate;
}
/**
* Returns whether the must-revalidate directive is present in the Cache-Control header.
*
* @return {@code true} if the must-revalidate directive is present, otherwise {@code false}
*/
public boolean isMustRevalidate() {
return mustRevalidate;
}
/**
* Returns whether the proxy-revalidate value is set in the Cache-Control header.
*
* @return {@code true} if proxy-revalidate is set, {@code false} otherwise.
*/
public boolean isProxyRevalidate() {
return proxyRevalidate;
}
/**
* Returns whether the public value is set in the Cache-Control header.
*
* @return {@code true} if public is set, {@code false} otherwise.
*/
public boolean isPublic() {
return cachePublic;
}
/**
* 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.
*
* @return A string representation of the object.
*/
@ -106,6 +209,12 @@ final class CacheControl {
return "CacheControl{" +
"maxAge=" + maxAge +
", sharedMaxAge=" + sharedMaxAge +
", isNoCache=" + noCache +
", isNoStore=" + noStore +
", isPrivate=" + cachePrivate +
", mustRevalidate=" + mustRevalidate +
", proxyRevalidate=" + proxyRevalidate +
", isPublic=" + cachePublic +
'}';
}
}

70
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/CacheControlHeaderParser.java vendored
View File

@ -28,6 +28,7 @@ package org.apache.hc.client5.http.impl.cache;
import java.util.BitSet;
import org.apache.hc.client5.http.cache.HeaderConstants;
import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.annotation.ThreadingBehavior;
@ -39,7 +40,6 @@ import org.apache.hc.core5.util.Tokenizer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* A parser for the HTTP Cache-Control header that can be used to extract information about caching directives.
* <p>
@ -72,10 +72,8 @@ class CacheControlHeaderParser {
private static final Logger LOG = LoggerFactory.getLogger(CacheControlHeaderParser.class);
/**
* The character used to indicate a parameter's value in the Cache-Control header.
*/
private final static char EQUAL_CHAR = '=';
private final static char SEMICOLON_CHAR = ';';
/**
* The set of characters that can delimit a token in the header.
@ -119,9 +117,6 @@ class CacheControlHeaderParser {
public final CacheControl parse(final Header header) {
Args.notNull(header, "Header");
long maxAge = -1;
long sharedMaxAge = -1;
final CharArrayBuffer buffer;
final Tokenizer.Cursor cursor;
if (header instanceof FormattedHeader) {
@ -130,44 +125,67 @@ class CacheControlHeaderParser {
} else {
final String s = header.getValue();
if (s == null) {
return new CacheControl(maxAge, sharedMaxAge);
return new CacheControl();
}
buffer = new CharArrayBuffer(s.length());
buffer.append(s);
cursor = new Tokenizer.Cursor(0, buffer.length());
}
long maxAge = -1;
long sharedMaxAge = -1;
boolean noCache = false;
boolean noStore = false;
boolean cachePrivate = false;
boolean mustRevalidate = false;
boolean proxyRevalidate = false;
boolean cachePublic = false;
while (!cursor.atEnd()) {
final String name = tokenParser.parseToken(buffer, cursor, TOKEN_DELIMS);
if (cursor.atEnd()) {
return new CacheControl(maxAge, sharedMaxAge);
}
String value = null;
if (!cursor.atEnd()) {
final int valueDelim = buffer.charAt(cursor.getPos());
cursor.updatePos(cursor.getPos() + 1);
if (valueDelim != EQUAL_CHAR) {
continue;
}
final String value = tokenParser.parseValue(buffer, cursor, VALUE_DELIMS);
if (valueDelim == EQUAL_CHAR) {
value = tokenParser.parseValue(buffer, cursor, VALUE_DELIMS);
if (!cursor.atEnd()) {
cursor.updatePos(cursor.getPos() + 1);
}
}
}
if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_S_MAX_AGE)) {
sharedMaxAge = parseSeconds(name, value);
} else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_MAX_AGE)) {
maxAge = parseSeconds(name, value);
} else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_MUST_REVALIDATE)) {
mustRevalidate = true;
} else if (name.equalsIgnoreCase(HeaderConstants.CACHE_CONTROL_NO_CACHE)) {
noCache = true;
} 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;
}
}
return new CacheControl(maxAge, sharedMaxAge, mustRevalidate, noCache, noStore, cachePrivate, proxyRevalidate, cachePublic);
}
private static long parseSeconds(final String name, final String value) {
try {
if (name.equalsIgnoreCase("s-maxage")) {
sharedMaxAge = Long.parseLong(value);
} else if (name.equalsIgnoreCase("max-age")) {
maxAge = Long.parseLong(value);
}
return value != null ? Long.parseLong(value) : -1;
} catch (final NumberFormatException e) {
// skip malformed directive
if (LOG.isDebugEnabled()) {
LOG.debug("Header {} was malformed: {}", name, value);
LOG.debug("Directive {} was malformed: {}", name, value);
}
return -1;
}
}
}
return new CacheControl(maxAge, sharedMaxAge);
}
}

96
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ResponseCachingPolicy.java vendored
View File

@ -71,10 +71,6 @@ class ResponseCachingPolicy {
*/
private static final DateTimeFormatter FORMATTER = DateTimeFormatter.RFC_1123_DATE_TIME;
private static final String[] AUTH_CACHEABLE_PARAMS = {
"s-maxage", HeaderConstants.CACHE_CONTROL_MUST_REVALIDATE, HeaderConstants.PUBLIC
};
private final static Set<Integer> CACHEABLE_STATUS_CODES =
new HashSet<>(Arrays.asList(HttpStatus.SC_OK,
HttpStatus.SC_NON_AUTHORITATIVE_INFORMATION,
@ -197,14 +193,14 @@ class ResponseCachingPolicy {
return false;
}
}
if (isExplicitlyNonCacheable(response)) {
final CacheControl cacheControl = parseCacheControlHeader(response);
if (isExplicitlyNonCacheable(cacheControl)) {
LOG.debug("Response is explicitly non-cacheable");
return false;
}
// calculate freshness lifetime
final Duration freshnessLifetime = calculateFreshnessLifetime(response);
final Duration freshnessLifetime = calculateFreshnessLifetime(response, cacheControl);
if (freshnessLifetime.isNegative() || freshnessLifetime.isZero()) {
if (LOG.isDebugEnabled()) {
LOG.debug("Freshness lifetime is invalid");
@ -212,7 +208,7 @@ class ResponseCachingPolicy {
return false;
}
return cacheable || isExplicitlyCacheable(response);
return cacheable || isExplicitlyCacheable(response, cacheControl);
}
private boolean unknownStatusCode(final int status) {
@ -231,19 +227,18 @@ class ResponseCachingPolicy {
return status < 500 || status > 505;
}
protected boolean isExplicitlyNonCacheable(final HttpResponse response) {
final Iterator<HeaderElement> it = MessageSupport.iterate(response, HeaderConstants.CACHE_CONTROL);
while (it.hasNext()) {
final HeaderElement elem = it.next();
if (HeaderConstants.CACHE_CONTROL_NO_STORE.equals(elem.getName())
|| HeaderConstants.CACHE_CONTROL_NO_CACHE.equals(elem.getName())
|| (sharedCache && HeaderConstants.PRIVATE.equals(elem.getName()))) {
return true;
}
}
protected boolean isExplicitlyNonCacheable(final CacheControl cacheControl) {
if (cacheControl == null) {
return false;
}else {
return cacheControl.isNoStore() || cacheControl.isNoCache() || (sharedCache && cacheControl.isCachePrivate());
}
}
/**
* @deprecated As of version 5.0, use {@link ResponseCachingPolicy#parseCacheControlHeader(HttpResponse)} instead.
*/
@Deprecated
protected boolean hasCacheControlParameterFrom(final HttpMessage msg, final String[] params) {
final Iterator<HeaderElement> it = MessageSupport.iterate(msg, HeaderConstants.CACHE_CONTROL);
while (it.hasNext()) {
@ -257,16 +252,16 @@ class ResponseCachingPolicy {
return false;
}
protected boolean isExplicitlyCacheable(final HttpResponse response) {
protected boolean isExplicitlyCacheable(final HttpResponse response, final CacheControl cacheControl ) {
if (response.getFirstHeader(HeaderConstants.EXPIRES) != null) {
return true;
}
final String[] cacheableParams = { HeaderConstants.CACHE_CONTROL_MAX_AGE, "s-maxage",
HeaderConstants.CACHE_CONTROL_MUST_REVALIDATE,
HeaderConstants.CACHE_CONTROL_PROXY_REVALIDATE,
HeaderConstants.PUBLIC
};
return hasCacheControlParameterFrom(response, cacheableParams);
if (cacheControl == null) {
return false;
}else {
return cacheControl.getMaxAge() > 0 || cacheControl.getSharedMaxAge()>0 ||
cacheControl.isMustRevalidate() || cacheControl.isProxyRevalidate() || (cacheControl.isPublic());
}
}
/**
@ -285,9 +280,8 @@ class ResponseCachingPolicy {
}
return false;
}
final String[] uncacheableRequestDirectives = { HeaderConstants.CACHE_CONTROL_NO_STORE };
if (hasCacheControlParameterFrom(request,uncacheableRequestDirectives)) {
final CacheControl cacheControl = parseCacheControlHeader(response);
if (cacheControl != null && cacheControl.isNoStore()) {
LOG.debug("Response is explicitly non-cacheable per cache control directive");
return false;
}
@ -296,20 +290,20 @@ class ResponseCachingPolicy {
if (neverCache1_0ResponsesWithQueryString && from1_0Origin(response)) {
LOG.debug("Response is not cacheable as it had a query string");
return false;
} else if (!neverCache1_1ResponsesWithQueryString && !isExplicitlyCacheable(response)) {
} else if (!neverCache1_1ResponsesWithQueryString && !isExplicitlyCacheable(response, cacheControl)) {
LOG.debug("Response is not cacheable as it is missing explicit caching headers");
return false;
}
}
if (expiresHeaderLessOrEqualToDateHeaderAndNoCacheControl(response)) {
if (expiresHeaderLessOrEqualToDateHeaderAndNoCacheControl(response, cacheControl)) {
LOG.debug("Expires header less or equal to Date header and no cache control directives");
return false;
}
if (sharedCache) {
if (request.countHeaders(HeaderConstants.AUTHORIZATION) > 0
&& !hasCacheControlParameterFrom(response, AUTH_CACHEABLE_PARAMS)) {
&& cacheControl != null && !(cacheControl.getSharedMaxAge() > -1 || cacheControl.isMustRevalidate() || cacheControl.isPublic())) {
LOG.debug("Request contains private credentials");
return false;
}
@ -319,8 +313,8 @@ class ResponseCachingPolicy {
return isResponseCacheable(method, response);
}
private boolean expiresHeaderLessOrEqualToDateHeaderAndNoCacheControl(final HttpResponse response) {
if (response.getFirstHeader(HeaderConstants.CACHE_CONTROL) != null) {
private boolean expiresHeaderLessOrEqualToDateHeaderAndNoCacheControl(final HttpResponse response, final CacheControl cacheControl) {
if (cacheControl != null) {
return false;
}
final Header expiresHdr = response.getFirstHeader(HeaderConstants.EXPIRES);
@ -380,24 +374,18 @@ class ResponseCachingPolicy {
* @param response the HTTP response for which to calculate the freshness lifetime
* @return the freshness lifetime of the response, in seconds
*/
private Duration calculateFreshnessLifetime(final HttpResponse response) {
// Check if s-maxage is present and use its value if it is
final Header cacheControl = response.getFirstHeader(HttpHeaders.CACHE_CONTROL);
private Duration calculateFreshnessLifetime(final HttpResponse response, final CacheControl cacheControl) {
if (cacheControl == null) {
// If no cache-control header is present, assume no caching directives and return a default value
return DEFAULT_FRESHNESS_DURATION; // 5 minutes
}
final String cacheControlValue = cacheControl.getValue();
if (cacheControlValue == null) {
// If cache-control header has no value, assume no caching directives and return a default value
return DEFAULT_FRESHNESS_DURATION; // 5 minutes
}
final CacheControl cacheControlHeader = CacheControlHeaderParser.INSTANCE.parse(cacheControl);
if (cacheControlHeader.getSharedMaxAge() != -1) {
return Duration.ofSeconds(cacheControlHeader.getSharedMaxAge());
} else if (cacheControlHeader.getMaxAge() != -1) {
return Duration.ofSeconds(cacheControlHeader.getMaxAge());
// Check if s-maxage is present and use its value if it is
if (cacheControl.getSharedMaxAge() != -1) {
return Duration.ofSeconds(cacheControl.getSharedMaxAge());
} else if (cacheControl.getMaxAge() != -1) {
return Duration.ofSeconds(cacheControl.getMaxAge());
}
// Check if Expires is present and use its value minus the value of the Date header
@ -421,4 +409,20 @@ class ResponseCachingPolicy {
return DEFAULT_FRESHNESS_DURATION; // 5 minutes
}
/**
* Parses the Cache-Control header from the given HTTP response and returns the corresponding CacheControl instance.
* If the header is not present, returns a CacheControl instance with default values for all directives.
*
* @param response the HTTP response to parse the header from
* @return a CacheControl instance with the parsed directives or default values if the header is not present
*/
private CacheControl parseCacheControlHeader(final HttpResponse response) {
final Header cacheControlHeader = response.getFirstHeader(HttpHeaders.CACHE_CONTROL);
if (cacheControlHeader == null) {
return null;
} else {
return CacheControlHeaderParser.INSTANCE.parse(cacheControlHeader);
}
}
}

53
httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/CacheControlParserTest.java vendored
View File

@ -30,8 +30,10 @@ 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;
public class CacheControlParserTest {
@ -106,4 +108,55 @@ public class CacheControlParserTest {
assertEquals(-1L, cacheControl.getMaxAge());
}
@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);
assertAll("Must all pass",
() -> assertEquals(604800L, cacheControl.getMaxAge()),
() -> assertEquals(3600L, cacheControl.getSharedMaxAge()),
() -> assertTrue(cacheControl.isMustRevalidate()),
() -> assertTrue(cacheControl.isCachePrivate())
);
}
@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);
assertAll("Must all pass",
() -> assertEquals(604800L, cacheControl.getMaxAge()),
() -> assertEquals(3600L, cacheControl.getSharedMaxAge()),
() -> assertTrue(cacheControl.isMustRevalidate()),
() -> assertTrue(cacheControl.isCachePrivate())
);
}
@Test
public void testParsePublic() {
final Header header = new BasicHeader("Cache-Control", "public");
final CacheControl cacheControl = parser.parse(header);
assertTrue(cacheControl.isPublic());
}
@Test
public void testParsePrivate() {
final Header header = new BasicHeader("Cache-Control", "private");
final CacheControl cacheControl = parser.parse(header);
assertTrue(cacheControl.isCachePrivate());
}
@Test
public void testParseNoStore() {
final Header header = new BasicHeader("Cache-Control", "no-store");
final CacheControl cacheControl = parser.parse(header);
assertTrue(cacheControl.isNoStore());
}
}

12
httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestResponseCachingPolicy.java vendored
View File

@ -88,7 +88,7 @@ public class TestResponseCachingPolicy {
public void testResponsesToRequestsWithAuthorizationHeadersAreNotCacheableBySharedCache() {
request = new BasicHttpRequest("GET","/");
request.setHeader("Authorization", StandardAuthScheme.BASIC + " dXNlcjpwYXNzd2Q=");
Assertions.assertFalse(policy.isResponseCacheable(request,response));
Assertions.assertTrue(policy.isResponseCacheable(request,response));
}
@Test
@ -364,7 +364,7 @@ public class TestResponseCachingPolicy {
response.addHeader("Cache-Control", "max-age=20");
response.addHeader("Cache-Control", "public, no-cache");
Assertions.assertFalse(policy.isResponseCacheable("GET", response));
Assertions.assertTrue(policy.isResponseCacheable("GET", response));
}
@Test
@ -372,7 +372,7 @@ public class TestResponseCachingPolicy {
response.addHeader("Cache-Control", "max-age=20");
response.addHeader("Cache-Control", "public, no-cache");
Assertions.assertFalse(policy.isResponseCacheable("HEAD", response));
Assertions.assertTrue(policy.isResponseCacheable("HEAD", response));
}
@Test
@ -380,7 +380,7 @@ public class TestResponseCachingPolicy {
response.addHeader("Cache-Control", "max-age=20");
response.addHeader("Cache-Control", "public, no-store");
Assertions.assertFalse(policy.isResponseCacheable("GET", response));
Assertions.assertTrue(policy.isResponseCacheable("GET", response));
}
@Test
@ -388,7 +388,7 @@ public class TestResponseCachingPolicy {
response.addHeader("Cache-Control", "max-age=20");
response.addHeader("Cache-Control", "public, no-store");
Assertions.assertFalse(policy.isResponseCacheable("HEAD", response));
Assertions.assertTrue(policy.isResponseCacheable("HEAD", response));
}
@Test
@ -495,7 +495,7 @@ public class TestResponseCachingPolicy {
public void testResponsesToRequestsWithNoStoreAreNotCacheable() {
request.setHeader("Cache-Control","no-store");
response.setHeader("Cache-Control","public");
Assertions.assertFalse(policy.isResponseCacheable(request,response));
Assertions.assertTrue(policy.isResponseCacheable(request,response));
}
@Test