diff --git a/cachecontrol/controller.py b/cachecontrol/controller.py index 5eca8de7..8d80e67f 100644 --- a/cachecontrol/controller.py +++ b/cachecontrol/controller.py @@ -382,7 +382,8 @@ def cache_response( cache_url = self.cache_url(request.url) logger.debug('Updating cache with response from "%s"', cache_url) - # Delete it from the cache if we happen to have it stored there + # Don't cache a no-store response. Drop a stored entry only when it + # is the same variant; a different Vary variant stays. no_store = False if "no-store" in cc: no_store = True @@ -390,10 +391,19 @@ def cache_response( if "no-store" in cc_req: no_store = True logger.debug('Request header has "no-store"') - if no_store and self.cache.get(cache_url): - logger.debug('Purging existing cache entry to honor "no-store"') - self.cache.delete(cache_url) if no_store: + # no-store applies to this response. A different Vary variant + # stored under the same URL is not this response. + cached_data = self.cache.get(cache_url) + if cached_data is not None: + same_variant = self.serializer.stored_variant_matches( + request, cached_data + ) + if same_variant is False: + logger.debug("Keeping cached response for a different Vary variant") + else: + logger.debug('Purging existing cache entry to honor "no-store"') + self.cache.delete(cache_url) return # https://tools.ietf.org/html/rfc7234#section-4.1: diff --git a/cachecontrol/serialize.py b/cachecontrol/serialize.py index 83bce073..7ecb2057 100644 --- a/cachecontrol/serialize.py +++ b/cachecontrol/serialize.py @@ -132,6 +132,35 @@ def prepare_response( return HTTPResponse(body=body, preload_content=False, **cached["response"]) + def stored_variant_matches( + self, request: PreparedRequest, data: object + ) -> bool | None: + """Return whether ``request`` selects the stored response. + + False means the entry is a different Vary variant. None means the + entry is not a current cache record. This uses the same Vary check + as ``prepare_response``. + """ + if not isinstance(data, bytes): + return None + prefix = f"cc={self.serde_version},".encode() + if not data.startswith(prefix): + return None + try: + cached = msgpack.loads(data[len(prefix) :], raw=False) + except (ValueError, TypeError): + return None + if not isinstance(cached, dict): + return None + vary = cached.get("vary", {}) + if not isinstance(vary, dict) or "*" in vary: + return None + + for header, value in vary.items(): + if request.headers.get(header, None) != value: + return False + return True + def _loads_v4( self, request: PreparedRequest, diff --git a/tests/test_cache_control.py b/tests/test_cache_control.py index 6e3f21bd..05eadf3c 100644 --- a/tests/test_cache_control.py +++ b/tests/test_cache_control.py @@ -113,6 +113,57 @@ def test_cache_response_no_store(self): cc.cache_response(self.req(), resp) assert not cc.cache.get(cache_url) + def test_no_store_keeps_different_vary_variant(self): + cache = DictCache() + cc = CacheController(cache) + now = time.strftime(TIME_FMT, time.gmtime()) + json_req = DummyRequest(self.url, {"Accept": "application/json"}) + json_resp = DummyResponse( + 200, + { + "cache-control": "max-age=3600", + "date": now, + "vary": "Accept", + }, + ) + cc.cache_response(json_req, json_resp, b"json") + cached = cc.cache.get(self.url) + assert cached + + html_req = DummyRequest(self.url, {"Accept": "text/html"}) + html_resp = DummyResponse( + 200, + {"cache-control": "no-store", "vary": "Accept"}, + ) + cc.cache_response(html_req, html_resp, b"html") + + assert cc.cache.get(self.url) == cached + assert cc.serializer.loads(json_req, cached) is not None + assert cc.serializer.loads(html_req, cached) is None + + def test_no_store_purges_same_vary_variant(self): + cache = DictCache() + cc = CacheController(cache) + now = time.strftime(TIME_FMT, time.gmtime()) + req = DummyRequest(self.url, {"Accept": "application/json"}) + resp = DummyResponse( + 200, + { + "cache-control": "max-age=3600", + "date": now, + "vary": "Accept", + }, + ) + cc.cache_response(req, resp, b"json") + assert cc.cache.get(self.url) + + cc.cache_response( + req, + DummyResponse(200, {"cache-control": "no-store", "vary": "Accept"}), + b"html", + ) + assert cc.cache.get(self.url) is None + def test_cache_control_directives_case_insensitive(self, cc): now = time.strftime(TIME_FMT, time.gmtime()) resp = self.resp({"cache-control": "No-Store, max-age=3600", "date": now})