~ knotless Search/
↑↓ move↵ open⇧↵ all results

grain.social/grain

photography-focused photo sharing platform

grain.social
1 collaborator
0
2

Every query answer says it varies by who asks, as knotless's do

Closed
Closed chadtmiller.com asked to merge into feature/rust from grain.social/grain:infra/headers

What changes

Every query answer says it varies by who asks. grain-serve's queries (GET /xrpc/*) answer Vary: authorization, cookie, always. One that found someone asking, by a service token or the web's session cookie, is Cache-Control: private, no-cache; one that read what is the asker's alone, private, no-store: an account's preferences (social.grain.actor.getPreferences) and everything an admin reads (listReports, getReport, getSubjectStatus, listLabelDefinitions, getReviewSubject). An answer asked by no one, or by a token that does not hold, has no Cache-Control. Procedures and the routes off /xrpc/ are as they were.

viewer::answers is one layer over the whole router, so a query added later is covered without naming it. viewer::viewer keeps whom it found in a task-local the layer reads, and viewer::unshared() marks an answer for no cache, as knotless's private::touched does.

Why

The rewrite copies knotless wherever knotless already does the thing. knotless's queries answer with these headers (docs/reference/api.md, "Who asks"), and grain's need them for the same reason: the same URL answers differently for each asker, so a shared cache in front, or the browser's, must not hand one asker's answer to another.

Mirrors knotless's crates/knotless-serve/src/viewer.rs (viewer, answered, private, vary) and crates/knotless-serve/src/private.rs (touched, unshared). It differs from knotless in two grain-specific ways. grain finds the asker inside each handler (viewer::viewer) rather than in a layer over a queries router, so the layer learns of the asker from what the handler found. And it covers every GET /xrpc/* route, not one sub-router, so that the parallel pulls adding queries need not touch it. Where knotless marks private repositories unshared, grain marks preferences and admin reads; the private galleries pull should call viewer::unshared() where it reads a space.

How it was checked

Tests through tests/common (a new get_answered returns the headers too):

  • read.rs: getProfile with a good token is private, no-cache, and with none or a bad one has no Cache-Control. All three vary.
  • login.rs: getUnreadCount with the session cookie is private, no-cache.
  • notifications.rs: getPreferences is private, no-store.
  • moderation.rs: an admin's listLabelDefinitions is private, no-store.
  • health.rs: _health varies with no Cache-Control, and /oauth/session has neither.

The fmt, clippy, harness and coverage runs are left to CI.

Shortcomings

  • /_status does not exist here yet. When it lands (the image, health and rollout pull) it should answer no-store as knotless's does.
  • No CORS on the queries, as knotless has. grain-serve has no tower-http, and that change is separate.

🤖 Generated with Claude Code

Approved an earlier version

This adds one layer over the router. It marks every GET /xrpc/* answer Vary: authorization, cookie. It adds private, no-cache when the handler found an asker, and private, no-store when the handler called viewer::unshared(). It calls unshared() for preferences and for any successful admin check. I read the diff only and did not run the tests or look at CI.

The design holds up. The layer is outermost, and a handler runs inside the same future as next.run, so the task-local is visible to it. Procedures and non-/xrpc/ routes pass through untouched. The tests cover the token, cookie, bad-token, no-store and non-query cases.

One point to keep in mind, not a blocker: the layer relies on a handler not spawning a separate task before it calls viewer or unshared. If it did, try_with would silently skip the marking. Today's handlers call them inline.

Approved

Round 2 adds one commit on top of what I approved. It rewrites cached in viewer.rs to compute the Cache-Control value first and then set all headers in one pass. The test in notifications.rs now passes its bearer token as &str. The behaviour is unchanged: no asker and not unshared gives no Cache-Control, an asker gives private, no-cache, and unshared gives private, no-store. Vary is still always appended. Nothing from the earlier review needed fixing. I read the diff only and did not run the tests.

Landed on feature/rust at fc5f686 through the batch pull Seven reviewed pulls land on feature/rust together, checked once, checked there as a whole. This pull's head is in feature/rust's history.