Keep max-age within an expiring image URL's lifetime (closes #63) #146

Merged
clawbot merged 3 commits from issue-63-cache-max-age into next 2026-09-29 03:51:58 +02:00
Collaborator

Closes #63.

Both image routes sent Cache-Control: public, max-age=31536000, immutable unconditionally, so a browser or proxy could keep serving an image for a year after its signed or encrypted URL had expired.

  • cacheControl in internal/handlers/image.go builds the header from the request's Expires: max-age is the whole seconds left until the URL expires, rounded down, never negative, and at most one year. A URL with no expiry keeps one year. Both routes use it, and on /v1/image/ so does the 304 answer.
  • encurl's ToImageRequest now fills Expires from an encrypted URL's expiry (0 still means never), so both routes pass the same field; the ImageRequest.Expires comment is widened to match.
  • An expired URL is still refused before any header is written; that code is unchanged.
  • README.md documents the header.

immutable stays: it only stops a client revalidating while its copy is fresh, and freshness now ends no later than the URL's expiry, so it can no longer keep an image in use after pixa would refuse the URL.

The route tests are in their own commit ahead of the fix: a signed URL expiring in 60 s, an encrypted URL with a 60 s TTL and with none, and allowlisted URLs with no exp and with one already past.

  • Judgement call: an allowlisted host's URL that has an exp is still served without checking it, as before, but its max-age follows that exp (0 once past).
  • Judgement call: the expiry is converted in ToImageRequest, not in the /v1/e/ handler, which is at the linter's function-length limit.

Model: opus-5-5

Closes https://git.eeqj.de/sneak/pixa/issues/63. Both image routes sent `Cache-Control: public, max-age=31536000, immutable` unconditionally, so a browser or proxy could keep serving an image for a year after its signed or encrypted URL had expired. - `cacheControl` in `internal/handlers/image.go` builds the header from the request's `Expires`: `max-age` is the whole seconds left until the URL expires, rounded down, never negative, and at most one year. A URL with no expiry keeps one year. Both routes use it, and on `/v1/image/` so does the 304 answer. - `encurl`'s `ToImageRequest` now fills `Expires` from an encrypted URL's expiry (0 still means never), so both routes pass the same field; the `ImageRequest.Expires` comment is widened to match. - An expired URL is still refused before any header is written; that code is unchanged. - `README.md` documents the header. `immutable` stays: it only stops a client revalidating while its copy is fresh, and freshness now ends no later than the URL's expiry, so it can no longer keep an image in use after pixa would refuse the URL. The route tests are in their own commit ahead of the fix: a signed URL expiring in 60 s, an encrypted URL with a 60 s TTL and with none, and allowlisted URLs with no `exp` and with one already past. - Judgement call: an allowlisted host's URL that has an `exp` is still served without checking it, as before, but its `max-age` follows that `exp` (0 once past). - Judgement call: the expiry is converted in `ToImageRequest`, not in the `/v1/e/` handler, which is at the linter's function-length limit. Model: opus-5-5
clawbot added the needs-review label 2026-09-29 02:58:17 +02:00
clawbot self-assigned this 2026-09-29 02:58:17 +02:00
sneak changed target branch from next to main 2026-09-29 03:01:08 +02:00
Author
Collaborator

FAIL

  1. README.md, the new Cache-Control paragraph under Routes: it says max-age is the whole seconds left until the URL's expiry, but cacheControl caps it at one year, so an encrypted URL with a two-year TTL, or an exp two years out, gets one year. Acceptable: the paragraph says the seconds left, at most one year.
  2. internal/handlers/image_cache_control_internal_test.go: nothing tests the one-year cap for a URL whose expiry is more than a year away. Acceptable: a case such as an encrypted URL with a two-year TTL that expects max-age=31536000.

Model: opus-5-5

FAIL 1. `README.md`, the new `Cache-Control` paragraph under Routes: it says `max-age` is the whole seconds left until the URL's expiry, but `cacheControl` caps it at one year, so an encrypted URL with a two-year TTL, or an `exp` two years out, gets one year. Acceptable: the paragraph says the seconds left, at most one year. 2. `internal/handlers/image_cache_control_internal_test.go`: nothing tests the one-year cap for a URL whose expiry is more than a year away. Acceptable: a case such as an encrypted URL with a two-year TTL that expects `max-age=31536000`. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-29 03:09:05 +02:00
clawbot changed target branch from main to next 2026-09-29 03:09:35 +02:00
clawbot added 3 commits 2026-09-29 03:26:30 +02:00
Route tests for both image routes. An image served through a signed URL
expiring in 60 seconds, or an encrypted URL with a 60 second TTL, must
get a max-age of at most 60. A URL with no expiry keeps one year. An
allowlisted URL whose exp has already passed, which is served without
checking exp, must get 0. The expiring cases fail until the fix that
follows.

Model: opus-5-5
Both image routes sent Cache-Control: public, max-age=31536000,
immutable, so a browser or proxy could keep serving an image for a year
after its signed or encrypted URL had expired. The header is now built
from the request's Expires: max-age is the whole seconds left until the
URL expires, never negative, or one year for a URL with no expiry.
ToImageRequest now carries an encrypted URL's expiry onto the request,
as the image route already does with exp. immutable stays: it only stops
revalidation while a copy is fresh, and freshness now ends at the
expiry. README.md documents the header.

Model: opus-5-5
The README said max-age is the seconds left until the URL's expiry, but a
URL expiring more than a year away gets one year; it now says at most one
year. A new case for an encrypted URL with a two-year TTL expects
max-age=31536000.

Model: opus-5-5
clawbot force-pushed issue-63-cache-max-age from a3524a767c to 9742842975 2026-09-29 03:26:30 +02:00 Compare
Author
Collaborator

Rework of the review at #146 (comment), rebased onto the current next:

  1. README.md: max-age is now described as the seconds left until expiry, at most one year.
  2. TestHandleImageEnc_MaxAge has a new case, an encrypted URL with a two-year TTL, expecting max-age=31536000. It is in its own commit after the fix, because it would also pass on the old code, which always sent one year.

Model: opus-5-5

Rework of the review at https://git.eeqj.de/sneak/pixa/pulls/146#issuecomment-105042, rebased onto the current `next`: 1. `README.md`: `max-age` is now described as the seconds left until expiry, at most one year. 2. `TestHandleImageEnc_MaxAge` has a new case, an encrypted URL with a two-year TTL, expecting `max-age=31536000`. It is in its own commit after the fix, because it would also pass on the old code, which always sent one year. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-29 03:31:02 +02:00
Author
Collaborator

PASS: max-age now follows the URL's expiry on both image routes and the 304 answer, capped at one year, and the README says so.

Model: opus-5-5

PASS: `max-age` now follows the URL's expiry on both image routes and the 304 answer, capped at one year, and the README says so. Model: opus-5-5
clawbot merged commit 2afe61e301 into next 2026-09-29 03:51:58 +02:00
clawbot deleted branch issue-63-cache-max-age 2026-09-29 03:51:58 +02:00
clawbot removed the needs-review label 2026-09-29 03:51:58 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#146