Skip to content

Preserve cached response status during 304 revalidation - #494

Closed
jeromeyyy wants to merge 3 commits into
psf:masterfrom
jeromeyyy:fix-preserve-revalidated-status
Closed

jeromeyyy wants to merge 3 commits into
psf:masterfrom
jeromeyyy:fix-preserve-revalidated-status

Conversation

@jeromeyyy

Copy link
Copy Markdown

Bug

CacheController.update_cached_response() unconditionally sets the cached response's status to 200 after a 304 revalidation. This changes the meaning of cacheable non-200 responses. In particular, an ETag-bearing 301 or 308 becomes a 200 with a Location header, so requests no longer recognizes it as a redirect. The incorrect status is also written back into the cache.

Fix

Keep the stored status while refreshing the headers. The 304 is validation of the stored response, not a replacement 200 response (RFC 9111 section 4.3.4). No public API or dependencies change.

Regression coverage

  • Parameterize existing header/body-refresh tests across all default cacheable statuses (200, 203, 300, 301, 308), with both DictCache and SeparateBodyFileCache. Assert the status of both the returned response and a subsequent cache lookup.
  • Exercise the requests/CacheControlAdapter path for ETag-bearing 301 and 308 responses. Verify If-None-Match, from_cache, status, Location, redirect recognition, and body.
  • Against the original controller, eight controller cases and both adapter cases fail. They pass with this fix.

Validation

Using the committed dependency lock and Python 3.12:

  • uv run --frozen --dev pytest -q: 126 passed.
  • uv run --frozen --dev ruff check: passed.
  • uv run --frozen --dev mypy cachecontrol: passed (12 source files).
  • uv build: wheel and sdist built successfully.
  • git diff --check: passed.

The full 126-test suite also passed on Python 3.11. Its mypy check encounters the existing collections.abc.Buffer error in filewrapper.py; the production change is limited to removing the status override.

This contribution was prepared and tested with an AI coding assistant.

Refactor tests to parameterize status codes for cached response updates.
Added a test for revalidated redirects with status codes 301 and 308, ensuring proper caching behavior.
@psf psf locked and limited conversation to collaborators Nov 3, 2026
@woodruffw woodruffw closed this Nov 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants