Standardize Cache-Control Header Parsing Across SDK and Proxy - #34751
Standardize Cache-Control Header Parsing Across SDK and Proxy#34751ParvGatecha wants to merge 1 commit into
Conversation
Greptile SummaryStandardizes Cache-Control handling through a shared parser
Confidence Score: 3/5The PR should not merge until valued Current parsing allows cache reuse despite a valid valued Files Needing Attention: litellm/litellm_core_utils/cache_control_utils.py, litellm/caching/caching.py, litellm/caching/caching_handler.py, litellm/proxy/litellm_pre_call_utils.py
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/cache_control_utils.py | Adds the shared parser, but mishandles valued no-cache and collapses distinct lifetime directives using order-dependent semantics |
| litellm/caching/caching.py | Adds header-driven cache bypass, but only recognizes boolean no-cache |
| litellm/caching/caching_handler.py | Enriches cache options before reads and writes, while inheriting the parser's directive-value defects |
| litellm/proxy/litellm_pre_call_utils.py | Reuses parsed directives for proxy TTL and cache settings, exposing incorrect normalized lifetimes downstream |
| tests/test_litellm/litellm_core_utils/test_cache_control_utils.py | Covers common parser forms but omits valued no-cache and conflicting lifetime directives |
| tests/test_litellm/caching/test_caching_handler.py | Verifies basic header enrichment and explicit-cache precedence but not the failing edge cases |
Reviews (1): Last reviewed commit: "feat(caching): standardize HTTP Cache-Co..." | Re-trigger Greptile
| except ValueError: | ||
| parsed[raw_key] = raw_val | ||
| else: | ||
| parsed[raw_key] = raw_val |
There was a problem hiding this comment.
Valued no-cache bypass is ignored
When a default-on cache receives a valued directive such as Cache-Control: no-cache="Authorization", the parser stores the value as a string while both cache-read guards recognize only the boolean True, causing the lookup to proceed and a stale cached response to be returned instead of invoking the provider.
Knowledge Base Used: Response Caching
| if raw_key in ("max-age", "s-maxage", "s-max-age", "ttl"): | ||
| try: | ||
| int_val = int(raw_val) | ||
| parsed["s-maxage"] = int_val | ||
| parsed["s-max-age"] = int_val | ||
| parsed["ttl"] = int_val |
There was a problem hiding this comment.
Cache lifetime precedence is order-dependent
When a header contains different s-maxage and max-age values with max-age last, the parser overwrites every normalized lifetime key with max-age, causing the shared cache to use the wrong expiration and either evict entries prematurely or retain stale responses.
Knowledge Base Used: Response Caching
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Standardize Cache-Control Header Parsing Across SDK and Proxy
TLDR
Problem this solves:
How it solves it:
Relevant issues
N/A
Linear ticket
N/A
Pre-Submission checklist
Screenshots / Proof of Fix
Manual verification
The following scenarios were verified after the change:
Cache-Control: no-cachecorrectly bypasses cache usage.Cache-Control: no-storeis respected during cache storage decisions.Cache-Control: max-age=<value>is parsed consistently across SDK and Proxy.No-Cache) are parsed correctly.Type
🧹 Refactoring
Changes
parse_cache_control_header()utility underlitellm_core_utils.Final Attestation