rbrooks/Circa

Watch

Fork 0

Media responses are never cacheable; full re-download every time #85

New issue

Closed

opened 2026-07-28 06:00:29 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-07-28 06:00:29 +00:00

Severity: HIGH

The bug

Media endpoints return a bare FileResponse (backend/app/api/routes/photos.py:168-191).

Verified against the installed Starlette source and empirically on the dev server: Starlette sets
ETag and Last-Modified but has no conditional-request handling and sets no
Cache-Control. A request with a matching If-None-Match receives 200 and the full body
again — measured at 2,000,000 bytes re-downloaded rather than a 304.

Why it matters

A reviewer works through thousands of photos, revisiting and flipping front/back. Every view
re-transfers multi-megabyte originals. This is the largest recurring byte cost in the application.

The irony: storage is content-addressed by SHA-256, so the bytes for a given photo id and side
never change — aggressive immutable caching is trivially correct here, and the opportunity is
entirely unused.

Fix

Set Cache-Control: private, max-age=31536000, immutable on media responses.
Implement the If-None-Match → 304 check in the handler.
Consider using photo.sha256 as the ETag rather than Starlette's mtime-derived one, since it is
the true content identity.

Coordinate with the media security headers issue in v0.1.1 — both touch the same response.

Done when

A repeat request with a matching If-None-Match receives 304 with no body
Media responses carry an immutable Cache-Control
Navigating back to a previously viewed photo issues no image bytes

References

backend/app/api/routes/photos.py:168-191
Installed Starlette 0.49.4 responses.py (handles if-none-match: False)

## Severity: HIGH ## The bug Media endpoints return a bare `FileResponse` (`backend/app/api/routes/photos.py:168-191`). Verified against the installed Starlette source and empirically on the dev server: Starlette sets `ETag` and `Last-Modified` but has **no conditional-request handling** and sets no `Cache-Control`. A request with a matching `If-None-Match` receives **200 and the full body again** — measured at 2,000,000 bytes re-downloaded rather than a 304. ## Why it matters A reviewer works through thousands of photos, revisiting and flipping front/back. Every view re-transfers multi-megabyte originals. This is the largest recurring byte cost in the application. The irony: storage is content-addressed by SHA-256, so the bytes for a given photo id and side never change — aggressive immutable caching is trivially correct here, and the opportunity is entirely unused. ## Fix - Set `Cache-Control: private, max-age=31536000, immutable` on media responses. - Implement the `If-None-Match` → 304 check in the handler. - Consider using `photo.sha256` as the ETag rather than Starlette's mtime-derived one, since it is the true content identity. Coordinate with the media security headers issue in v0.1.1 — both touch the same response. ## Done when - [ ] A repeat request with a matching `If-None-Match` receives 304 with no body - [ ] Media responses carry an immutable `Cache-Control` - [ ] Navigating back to a previously viewed photo issues no image bytes ## References - `backend/app/api/routes/photos.py:168-191` - Installed Starlette 0.49.4 `responses.py` (`handles if-none-match: False`)

claude-bot added this to the v0.2.0 milestone

2026-07-28 06:00:29 +00:00

claude-bot added the

backend

enhancement

labels

2026-07-28 06:00:29 +00:00

claude-bot referenced this issue

2026-07-28 06:07:50 +00:00

Performance optimization and progress tracking at collection scale #48

rbrooks referenced this issue from a commit

2026-08-03 16:20:49 +00:00

Stop re-downloading every photograph on every view

rbrooks closed this issue

2026-08-03 16:20:49 +00:00

claude-bot commented

2026-08-03 16:21:14 +00:00

Author

Done in 20d5e4c. backend/tests/test_media_caching.py (16) plus one browser test in the e2e suite.

Confirmed on the current Starlette, not the audited one

The repo is now on Starlette 1.3.1 rather than 0.49.4, and it is unchanged:

handles if-none-match: False
sets cache-control:    False
sets etag:             True

So the diagnosis holds exactly: an ETag is emitted and then ignored.

The fix

Cache-Control: private, max-age=31536000, immutable, and an If-None-Match check in the handler that returns 304 with the validators and no body.

Three decisions worth stating:

Why immutable is safe here — the irony the issue points out is the licence for it. Storage keys are content-addressed by SHA-256, so the bytes behind a key cannot change; replacing a scan produces a different key, never different bytes at the same one. That is a property of the design, not an assumption about how people will behave.

private, not public — a shared proxy holding family photographs would be serving them to people the collection never admitted. The bandwidth is not worth that.

The ETag is the content identity, as suggested — but derived from the storage key rather than photo.sha256, because the key already contains the hash and it works for the back scan too (which has no column of its own). Starlette's own ETag is built from path, size and mtime, so a restore from backup or a copy between machines mints a new validator for identical bytes and every reviewer re-downloads the entire archive. Two rows sharing content — duplicates, #91 — now share a validator, so opening the duplicate of a photo already viewed costs nothing.

The security bit

The conditional check sits after authorization. A 304 is an answer: served before the session is resolved it would confirm both that the photo exists and that the caller once held its bytes. Two tests hold that up — an anonymous caller with a valid ETag still gets 401, and a viewer holding an ETag for an unapproved photo still gets 404.

Front and back are asserted to have different validators, because sharing one would mean flipping a photo over returned a 304 and the browser showed the front again — a caching bug that presents as the wrong photograph.

If-None-Match handling covers the three forms the header actually takes: *, a comma-separated list, and the W/ weak prefix.

Done when

A repeat request with a matching If-None-Match receives 304 with no body
Media responses carry an immutable Cache-Control
Navigating back to a previously viewed photo issues no image bytes — asserted in a real browser in the e2e suite, since whether a browser honours the policy is not something a unit test can answer

Verified load-bearing: reverting the policy to no-store fails exactly that browser test.

Coordinated with the media security headers as asked — X-Content-Type-Options and Content-Disposition are asserted on both the 200 and the 304, since a caching change is exactly the kind of edit that drops one of them.

1044 passed, 8 skipped; 7 e2e; ruff clean.

Done in 20d5e4c. `backend/tests/test_media_caching.py` (16) plus one browser test in the e2e suite. ## Confirmed on the current Starlette, not the audited one The repo is now on Starlette 1.3.1 rather than 0.49.4, and it is unchanged: ``` handles if-none-match: False sets cache-control: False sets etag: True ``` So the diagnosis holds exactly: an `ETag` is emitted and then ignored. ## The fix `Cache-Control: private, max-age=31536000, immutable`, and an `If-None-Match` check in the handler that returns 304 with the validators and no body. Three decisions worth stating: **Why `immutable` is safe here** — the irony the issue points out is the licence for it. Storage keys are content-addressed by SHA-256, so the bytes behind a key *cannot* change; replacing a scan produces a different key, never different bytes at the same one. That is a property of the design, not an assumption about how people will behave. **`private`, not `public`** — a shared proxy holding family photographs would be serving them to people the collection never admitted. The bandwidth is not worth that. **The ETag is the content identity, as suggested** — but derived from the *storage key* rather than `photo.sha256`, because the key already contains the hash and it works for the back scan too (which has no column of its own). Starlette's own ETag is built from path, size and mtime, so a restore from backup or a copy between machines mints a new validator for identical bytes and every reviewer re-downloads the entire archive. Two rows sharing content — duplicates, #91 — now share a validator, so opening the duplicate of a photo already viewed costs nothing. ## The security bit **The conditional check sits after authorization.** A 304 is an answer: served before the session is resolved it would confirm both that the photo exists and that the caller once held its bytes. Two tests hold that up — an anonymous caller with a valid ETag still gets 401, and a viewer holding an ETag for an unapproved photo still gets 404. Front and back are asserted to have *different* validators, because sharing one would mean flipping a photo over returned a 304 and the browser showed the front again — a caching bug that presents as the wrong photograph. `If-None-Match` handling covers the three forms the header actually takes: `*`, a comma-separated list, and the `W/` weak prefix. ## Done when - [x] A repeat request with a matching `If-None-Match` receives 304 with no body - [x] Media responses carry an immutable `Cache-Control` - [x] Navigating back to a previously viewed photo issues no image bytes — asserted in a real browser in the e2e suite, since whether a *browser* honours the policy is not something a unit test can answer Verified load-bearing: reverting the policy to `no-store` fails exactly that browser test. Coordinated with the media security headers as asked — `X-Content-Type-Options` and `Content-Disposition` are asserted on both the 200 and the 304, since a caching change is exactly the kind of edit that drops one of them. **1044 passed, 8 skipped**; 7 e2e; ruff clean.

claude-bot referenced this issue

2026-08-03 17:32:00 +00:00

No response compression for API JSON #92