fix(rest): set Vary: Accept-Encoding on GzipMiddleware responses#229
Merged
Conversation
GzipMiddleware negotiates compression on Accept-Encoding but never told a shared cache the body varied by it. A cache keyed on the URL alone could store one variant and hand it to a client that negotiated the other: a gzipped body to a client that cannot decode it, or the reverse. Set Vary: Accept-Encoding unconditionally at the top of the middleware, so both branches (gzip and identity) and both surfaces it fronts (/api and the docs handler) carry it, including the bodyless 204/304 finals. Add, not Set, so a Vary set upstream survives. The golden corpus stays unchanged and Vary stays off the contractHeaders allowlist: the goldens replay against the old stack, which sent no Vary, so recording it would pin its absence. The header is pinned by rest/vary_test.go instead and documented in openapi.yaml next to the Cache-Control entries. > *This was generated by AI*
…sponses The GzipMiddleware Vary comment described the wrong direction. Its Add runs at the top of the middleware, before next.ServeHTTP, so it can only preserve a Vary set by an outer layer, not protect a downstream route handler that later calls Set. Reword it to match the companion vary_test.go. Also fix a word collision where "Set unconditionally" used Set colloquially right next to the literal "Add, not Set," and scope "every response inherits it" to responses that reach this layer, since the outer auth 401 short-circuits first. GzipMiddleware adds Vary: Accept-Encoding to every response that reaches the handlers, including the JSON errors (400/403/404/500) every operation surfaces through the shared Error response. The spec declared Vary only on the 200 blocks, so it under-documented the header. Add the same Vary definition to the Error response component. Unauthorized (401) stays untouched: auth short-circuits before the gzip layer, so 401s carry no Vary. > *This was generated by AI*
This was referenced Jun 30, 2026
SyniRon
added a commit
that referenced
this pull request
Jun 30, 2026
… ships (#231) GzipMiddleware adds Vary: Accept-Encoding to every response a handler produces, and #229 documented it in the spec. Nothing kept the two in step the way TestSpec_Every200DeclaresCacheControl does for Cache-Control, so a future endpoint could document Cache-Control (enforced) but forget Vary and drift the spec from the wire again. TestSpec_VaryAccompaniesEveryGzippedResponse walks the spec and asserts Vary is declared on every response that reaches the gzip layer (all 2xx and the shared Error tier) and absent on the pre-gzip auth short-circuit tier, which has two wire statuses: the 401 and the auth-path 503 (the DB-outage branch in rest/auth.go). It checks the same schema rigor as the Cache-Control test: present, optional, string, const Accept-Encoding. Test-only change. The spec was already correct, so the guard passes today; its job is to fail a future PR that forgets the header. Proven non-vacuous against temporary spec mutations, including an explicit 503 to confirm it expects no Vary there. Closes #230. > *This was generated by AI*
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #228.
GzipMiddleware picks the response encoding from Accept-Encoding but never told caches the body varied by it. A shared cache keyed on the URL alone could hand a gzipped body to a client that can't decode it, or the reverse. This sets
Vary: Accept-Encodingonce at the top of the middleware, so every response that reaches the layer inherits it: the gzip and identity paths, and both surfaces the middleware fronts (/apiand the docs handler). It's appended withAdd, notSet, so aVaryvalue already set by an outer layer survives (Setwould drop it).Per the maintainer brief on the issue, the header stays out of the golden corpus and the
contractHeadersallowlist (the goldens replay against the old stack, which never sent it). It's pinned instead by a newrest/test and documented in the openapi response header blocks.Changes:
rest/gzip.go: one unconditionalHeader().Add("Vary", "Accept-Encoding").rest/vary_test.go: pins the header for gzip and non-gzip requests on/apiand the docs surface, plus the append-not-overwrite and keep-on-204 cases.openapi/openapi.yaml: documents the header on the success responses and the shared error response.contract/spec_test.go: keeps the mutation-canary fixture in step.Out of scope (unchanged): nginx/CDN cache config, golden re-recording, the gzip negotiation logic, and the hijack work in #181.