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.

Sign up for GitHub

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

Jump to bottom

CacheFilter: Fully parse and handle request and response cache-control #9833

Closed
toddmgreer opened this issue Jan 25, 2020 · 0 comments · Fixed by #11727
Closed

CacheFilter: Fully parse and handle request and response cache-control #9833

toddmgreer opened this issue Jan 25, 2020 · 0 comments · Fixed by #11727
Labels
area/cache help wanted Needs help!

Comments

@toddmgreer
Copy link
Contributor

The cache-control header (RFC7234§5.2) supports a long list of directives, all of which need to be understood by CacheFilter (#7198). Currently, CacheFilter only parses and handles the private, no-cache, max-age, and s-maxage response directives, and ignores the rest. Some directives (max-stale, min-fresh, no-transform, only-if-cached, must-revalidate, public, proxy-revalidate), cause no harm (other than missed caching opportunities) if ignored. Others (max-age and no-cache request directives, and no-store request/response directive) can cause incorrect results to be served, and thus must be handled before CacheFilter can be generally usable with production traffic.

Current parsing code is in extensions/filters/http/cache/http_cache_utils.cc and cache_filter.cc.
@toddmgreer toddmgreer mentioned this issue Jan 29, 2020
Closed
@yosrym93 yosrym93 mentioned this issue Jun 15, 2020
Closed
@yosrym93 yosrym93 mentioned this issue Jun 25, 2020
Merged
htuch pushed a commit that referenced this issue Jul 22, 2020
@yosrym93
caching: Fully parse and handle request and response cache-control he…
b510dd0 
…aders (#11727)

Request and response cache-control headers are fully parsed in CacheHeadersUtils. isCacheableResponse in the CacheFilter now uses parsed cache-control headers to check if a received response can be cached. HttpCache now uses parsed cache-control headers to check if a cached response requires validation before it is served. Cache entries validation is not yet implemented.
Signed-off-by: Yosry Ahmed yosryahmed@google.com

Risk Level: Low
Testing: Unit testing
Docs Changes: N/A
Release Notes: N/A

Fixes #9833

Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
KBaichoo pushed a commit to KBaichoo/envoy that referenced this issue Jul 30, 2020
@yosrym93 @KBaichoo
caching: Fully parse and handle request and response cache-control he…
009a503 
…aders (envoyproxy#11727)

Request and response cache-control headers are fully parsed in CacheHeadersUtils. isCacheableResponse in the CacheFilter now uses parsed cache-control headers to check if a received response can be cached. HttpCache now uses parsed cache-control headers to check if a cached response requires validation before it is served. Cache entries validation is not yet implemented.
Signed-off-by: Yosry Ahmed yosryahmed@google.com

Risk Level: Low
Testing: Unit testing
Docs Changes: N/A
Release Notes: N/A

Fixes envoyproxy#9833

Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/cache help wanted Needs help!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

caching: Fully parse and handle request and response cache-control headers yosrym93/envoy
2 participants
@mattklein123 @toddmgreer