Response header gaps: no Vary, X-Request-ID not returned or propagated, /v1/e/ lacks ETag/HEAD/304 #84

New Issue

2026-08-09T03:49:48+02:00

clawbot commented

2026-08-09 03:49:48 +02:00

Verified against main at 61f42e6. Bundles three small, related response-header gaps from TODO.md plus one asymmetry found during the survey.

1. No Vary header anywhere. grep for Vary returns nothing. Harmless today, but it becomes a correctness bug the moment format=auto lands (filed separately): serving content negotiated on Accept without Vary: Accept means a shared cache will hand a WebP to a client that cannot display it. Add it as part of whichever change first makes a response depend on a request header — and add Vary: Accept defensively now if that is soon.

2. X-Request-ID is generated and logged but never returned or propagated. chi's RequestID middleware is installed (internal/server/routes.go:19) and the ID reaches the logs (internal/middleware/middleware.go:90,96), but it is never written to the response, so a client cannot quote an ID when reporting a problem — which is the main reason to have one. It is also not forwarded upstream (internal/httpfetcher/httpfetcher.go:206-207 sets only User-Agent and Accept), so a request cannot be traced across the proxy boundary. Additionally the per-image log lines (internal/handlers/image.go:161-168, internal/imgcache/service.go:270-280,338-351) do not carry the request ID, so they cannot be correlated with the access log line for the same request.

3. /v1/e/ is missing response features /v1/image/ has. internal/handlers/imageenc.go:69-85 sets Content-Type, Content-Length, Cache-Control, and X-Pixa-Cache — but no ETag, no If-None-Match/304 handling, and internal/server/routes.go:60 registers only Get, no Head. /v1/image/ has all three (image.go:129-147, routes.go:55-56). Encrypted URLs are the ones most likely to be embedded in pages and re-requested, so they benefit from revalidation at least as much.

Definition of done

X-Request-ID is set on every response.
The request ID is forwarded on upstream fetches, and included in the per-image log lines so they correlate with the access log.
/v1/e/ sets ETag, handles If-None-Match with a 304, and the route is registered for HEAD as well as GET — reusing /v1/image/'s implementation rather than duplicating it.
Vary is emitted wherever a response varies on a request header; if nothing varies yet, state that explicitly in the PR and leave it to the format=auto work.
Failing tests first for each: response carries the ID; /v1/e/ returns 304 on a matching If-None-Match; HEAD /v1/e/... returns headers with no body.
make check green.

Note

Split into separate commits (one per item) — they are independent, and item 3 is the only one touching handler logic.

Verified against `main` at `61f42e6`. Bundles three small, related response-header gaps from `TODO.md` plus one asymmetry found during the survey. **1. No `Vary` header anywhere.** `grep` for `Vary` returns nothing. Harmless today, but it becomes a correctness bug the moment `format=auto` lands (filed separately): serving content negotiated on `Accept` without `Vary: Accept` means a shared cache will hand a WebP to a client that cannot display it. Add it as part of whichever change first makes a response depend on a request header — and add `Vary: Accept` defensively now if that is soon. **2. `X-Request-ID` is generated and logged but never returned or propagated.** `chi`'s `RequestID` middleware is installed (`internal/server/routes.go:19`) and the ID reaches the logs (`internal/middleware/middleware.go:90,96`), but it is never written to the response, so a client cannot quote an ID when reporting a problem — which is the main reason to have one. It is also not forwarded upstream (`internal/httpfetcher/httpfetcher.go:206-207` sets only User-Agent and Accept), so a request cannot be traced across the proxy boundary. Additionally the per-image log lines (`internal/handlers/image.go:161-168`, `internal/imgcache/service.go:270-280,338-351`) do not carry the request ID, so they cannot be correlated with the access log line for the same request. **3. `/v1/e/` is missing response features `/v1/image/` has.** `internal/handlers/imageenc.go:69-85` sets Content-Type, Content-Length, Cache-Control, and X-Pixa-Cache — but no `ETag`, no `If-None-Match`/304 handling, and `internal/server/routes.go:60` registers only `Get`, no `Head`. `/v1/image/` has all three (`image.go:129-147`, `routes.go:55-56`). Encrypted URLs are the ones most likely to be embedded in pages and re-requested, so they benefit from revalidation at least as much. ## Definition of done 1. `X-Request-ID` is set on every response. 2. The request ID is forwarded on upstream fetches, and included in the per-image log lines so they correlate with the access log. 3. `/v1/e/` sets `ETag`, handles `If-None-Match` with a 304, and the route is registered for `HEAD` as well as `GET` — reusing `/v1/image/`'s implementation rather than duplicating it. 4. `Vary` is emitted wherever a response varies on a request header; if nothing varies yet, state that explicitly in the PR and leave it to the `format=auto` work. 5. Failing tests first for each: response carries the ID; `/v1/e/` returns 304 on a matching `If-None-Match`; `HEAD /v1/e/...` returns headers with no body. 6. `make check` green. ## Note Split into separate commits (one per item) — they are independent, and item 3 is the only one touching handler logic.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:49:48 +02:00

clawbot referenced this issue

2026-08-09 16:10:39 +02:00

Tracking: 1.0.0 sequencing and critical path #103

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/pixa#84