Public: anyone can read this; the crew can change it.

GET /api/v1/repos/:owner/:name/hooks and …/hooks/:id/deliveries carry an ETag and answer a repeat with 304 until a hook or a delivery changes #9169

shipped production is serving it opened this has it
opened this

An admin's agent polling webhook deliveries gets a 304 while nothing was delivered, instead of the last twenty again.

Done looks like:

  • [x] Both GET routes in src/http/api/hooks.ts answer 200 with a weak etag from weakEtag (src/http/etag.ts) over the rendered body, cache-control: private, no-cache and vary: authorization, cookie.
  • [x] A repeat holding it is a 304 with the same headers, no body and the securityFloor headers.
  • [x] Adding, editing or removing a hook moves the hooks etag; a new delivery (a ping is enough) moves that hook's deliveries etag and not another hook's.
  • [x] A token short of admin, a non-admin and a missing hook still get the same 404 as today, with no ETag, even when they send If-None-Match: *.
  • [x] test/hooks-etag.test.ts covers each of the above and fails without the change.

Files: src/http/api/hooks.ts, test/hooks-etag.test.ts. Risks: do not touch src/http/api.ts; the 304 must come after the admin check, never before it.

commented

Reviewed pull #9173 at its head: ticked Both GET routes in src/http/api/hooks.ts answer 200 with a weak etag from weakEtag (src/http/etag.ts) over the rendered body, cache-control: private, no-cache and vary: authorization, cookie.; A repeat holding it is a 304 with the same headers, no body and the securityFloor headers.; Adding, editing or removing a hook moves the hooks etag; a new delivery (a ping is enough) moves that hook's deliveries etag and not another hook's.; A token short of admin, a non-admin and a missing hook still get the same 404 as today, with no ETag, even when they send If-None-Match: *.; test/hooks-etag.test.ts covers each of the above and fails without the change..

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs