From 032788c7a9e404adb880250792ea68ba488487c4 Mon Sep 17 00:00:00 2001 From: VenishPaneliya <141703684+VenishPaneliya@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:50:17 +0530 Subject: [PATCH] fix(controller): normalize the cache key when reading from the cache Writes go through self.cache_url(), which normalizes the URL and drops the fragment, but _load_from_cache() looked the entry up under the raw request.url. A request whose URL carries a fragment therefore stored an entry it could never read back again: the response was re-fetched on every call while the cache entry sat there unused. cached_request(), cache_response() and update_cached_response() all key on self.cache_url(request.url) already, including both cache.delete() calls in cached_request(), so the read was the only path left unnormalized. It normalized too until 11fbcfe unified the low-level cache loading code. --- cachecontrol/controller.py | 4 ++-- tests/test_regressions.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/cachecontrol/controller.py b/cachecontrol/controller.py index 03b22185..dc034d10 100644 --- a/cachecontrol/controller.py +++ b/cachecontrol/controller.py @@ -149,8 +149,8 @@ def _load_from_cache(self, request: PreparedRequest) -> HTTPResponse | None: if "Range" in request.headers: return None - cache_url = request.url - assert cache_url is not None + assert request.url is not None + cache_url = self.cache_url(request.url) cache_data = self.cache.get(cache_url) if cache_data is None: logger.debug("No cache entry available") diff --git a/tests/test_regressions.py b/tests/test_regressions.py index 78e8aaad..be7c74a8 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -21,6 +21,21 @@ def test_file_cache_recognizes_consumed_file_handle(self, url): s.close() +class TestCacheKeyNormalization: + def test_fragment_in_url_still_hits_the_cache(self, url): + """The cache is keyed on the normalized URL, which drops the fragment. + + Reads have to normalize too, otherwise a request for a URL carrying a + fragment stores an entry it can never look up again. + """ + s = CacheControl(Session()) + the_url = url + "cache_60#section" + s.get(the_url) + r = s.get(the_url) + assert r.from_cache + s.close() + + def test_getattr_during_gc(): s = CallbackFileWrapper(None, None) # normal behavior: