Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support of Cache-Control header is only partial #462

Open
NiccoMlt opened this issue Mar 5, 2024 · 0 comments
Open

Support of Cache-Control header is only partial #462

NiccoMlt opened this issue Mar 5, 2024 · 0 comments

Comments

@NiccoMlt
Copy link
Contributor

NiccoMlt commented Mar 5, 2024

While looking at the block code below to figure out if WebP images would be cached correctly:

switch (extension) {
case "png":
case "gif":
case "jpg":
case "jpeg":
case "js":
case "css":
case "woff2":
return true;
default:
return false;
}
}

we figured out that caching strategy implemented by isChached methods is a little naïve. Taking a look the the actual code:

public boolean isCacheable(HttpClientResponse response) {
HttpHeaders headers = response.responseHeaders();
String cacheControl = headers.get(HttpHeaderNames.CACHE_CONTROL, "").replaceAll(" ", "").toLowerCase();
if ((!cacheControl.isEmpty() && CACHE_CONTROL_CACHE_DISABLED_VALUES.stream().anyMatch(v -> cacheControl.contains(v)))
|| headers.contains(HttpHeaderNames.PRAGMA, HttpHeaderValues.NO_CACHE, true)
|| !isContentLengthCacheable(headers)) {
// never cache Pragma: no-cache, Cache-Control: nostore/no-cache
LOG.log(Level.FINER, "not cacheable {0}", response);
return false;
}

final HttpHeaders headers = request.getRequestHeaders();
final String cacheControl = headers.get(HttpHeaderNames.CACHE_CONTROL, "").replaceAll(" ", "").toLowerCase();
boolean ctrlF5 = cacheControl.contains(HttpHeaderValues.NO_CACHE + "");
if (ctrlF5) {
if (registerNoCacheStat) {
NO_CACHE_REQUESTS_COUNTER.inc();
}
return false;
}
if (this.currentConfiguration.isCacheDisabledForSecureRequestsWithoutPublic()
&& request.isSecure() && !cacheControl.contains(HttpHeaderValues.PUBLIC + "")) {
return false;
}
if ((!cacheControl.isEmpty() && CACHE_CONTROL_CACHE_DISABLED_VALUES.stream().anyMatch(v -> cacheControl.contains(v)))
|| headers.contains(HttpHeaderNames.PRAGMA, HttpHeaderValues.NO_CACHE, true)) {
// never cache Pragma: no-cache, Cache-Control: nostore/no-cache
return false;
}

it is especially evident that we don't actually support all the Cache-Control values.

Request Response
max-age max-age
max-stale -
min-fresh -
- s-maxage
no-cache no-cache
no-store no-store
no-transform no-transform
only-if-cached -
- must-revalidate
- proxy-revalidate
- must-understand
- private
- public
- immutable
- stale-while-revalidate
stale-if-error stale-if-error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant