From df61339d4934c59a876357659103714734e59a2a Mon Sep 17 00:00:00 2001 From: Lars Laskowski Date: Sun, 23 Aug 2026 11:55:59 +0000 Subject: [PATCH] Parse Accept-Encoding q-values in withGzip withGzip decided whether to compress with strings.Contains(header, "gzip"), but Accept-Encoding is a q-value list (RFC 9110 12.5.3), not an opaque string. A client sending "gzip;q=0" is explicitly refusing gzip, yet the substring match compressed anyway and handed it a body it said it could not decode. The deprecated "x-gzip" token matched for the same reason, as would any future coding whose name contains "gzip". Parse the header instead: split on commas, compare the coding token exactly (case-insensitively), and skip entries carrying q=0. A bare "*" now counts as accepting gzip, which is what the header means and what most servers do; previously "Accept-Encoding: *" got an identity response. An explicit gzip entry outranks a wildcard, so "gzip;q=0, *" stays a refusal. A qvalue that fails to parse is ignored rather than read as a refusal, keeping the prior behaviour for malformed headers. The JSON response shape is unchanged, so this is not an /api/v1 breaking change. --- docs/API.md | 5 ++ internal/httpapi/middleware.go | 41 +++++++++++++- internal/httpapi/middleware_test.go | 84 +++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 1 deletion(-) diff --git a/docs/API.md b/docs/API.md index 570c16a..149025e 100644 --- a/docs/API.md +++ b/docs/API.md @@ -45,6 +45,11 @@ and `Vary: Accept-Encoding`); the JSON body is unchanged, only its wire encoding differs. Requests without that header receive the identity (uncompressed) response, so existing clients keep working unmodified. +The header is parsed as the q-value list it is (RFC 9110 §12.5.3): an +explicit refusal (`Accept-Encoding: gzip;q=0`) is honoured and yields an +identity response, `Accept-Encoding: *` counts as accepting gzip, and the +deprecated `x-gzip` token is not treated as `gzip`. + ## Caching `/api/v1/...` responses carry `Cache-Control: no-store` and a `Vary` naming diff --git a/internal/httpapi/middleware.go b/internal/httpapi/middleware.go index a300340..a7feac9 100644 --- a/internal/httpapi/middleware.go +++ b/internal/httpapi/middleware.go @@ -6,6 +6,7 @@ import ( "crypto/subtle" "io" "net/http" + "strconv" "strings" "sync" "time" @@ -136,6 +137,44 @@ func (w *gzipResponseWriter) Write(b []byte) (int, error) { return w.gw.Write(b) } +// acceptsGzip reports whether the client accepts a gzip-encoded response. +// Accept-Encoding is a q-value list (RFC 9110 §12.5.3), not an opaque string: +// "gzip;q=0" is an explicit refusal and "x-gzip" is a distinct (deprecated) +// token, so substring-matching "gzip" would compress for clients that asked +// us not to. An explicit gzip entry outranks a wildcard, per the same +// section, so "gzip;q=0, *" is still a refusal. +func acceptsGzip(header string) bool { + wildcard := false + for _, part := range strings.Split(header, ",") { + coding, params, _ := strings.Cut(strings.TrimSpace(part), ";") + accepted := !qualityIsZero(params) + switch strings.ToLower(strings.TrimSpace(coding)) { + case "gzip": + return accepted + case "*": + wildcard = accepted + } + } + return wildcard +} + +// qualityIsZero reports whether an Accept-Encoding parameter list carries a +// qvalue of zero, in any of the forms RFC 9110 permits ("0", "0.", "0.0", +// "0.00", "0.000"). A qvalue that doesn't parse is ignored rather than read +// as a refusal, so a malformed header keeps the pre-existing behaviour of +// compressing instead of silently losing compression. +func qualityIsZero(params string) bool { + for _, p := range strings.Split(params, ";") { + k, v, ok := strings.Cut(p, "=") + if !ok || !strings.EqualFold(strings.TrimSpace(k), "q") { + continue + } + q, err := strconv.ParseFloat(strings.TrimSpace(v), 64) + return err == nil && q == 0 + } + return false +} + // withGzip transparently gzip-compresses responses for clients that // advertise support via Accept-Encoding. Responses to history/metrics // payloads are highly repetitive JSON and compress roughly 10x, which @@ -144,7 +183,7 @@ func (w *gzipResponseWriter) Write(b []byte) (int, error) { // responses unchanged, so this is backward compatible. func (s *Server) withGzip(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") { + if !acceptsGzip(r.Header.Get("Accept-Encoding")) { next.ServeHTTP(w, r) return } diff --git a/internal/httpapi/middleware_test.go b/internal/httpapi/middleware_test.go index c8a124d..4b56b0a 100644 --- a/internal/httpapi/middleware_test.go +++ b/internal/httpapi/middleware_test.go @@ -331,3 +331,87 @@ func TestHealthz_BypassesMaxInFlight(t *testing.T) { t.Fatalf("api status while in-flight limit is full = %d, want %d (sanity check that the fill above actually exercised the limiter)", apiRec.Code, http.StatusServiceUnavailable) } } + +// TestWithGzip_AcceptEncodingNegotiation exercises Accept-Encoding parsing +// through the full handler chain. The header is a q-value list per RFC 9110 +// §12.5.3, so an explicit refusal ("gzip;q=0") must yield an identity +// response and a lookalike token ("x-gzip") must not be treated as gzip. +func TestWithGzip_AcceptEncodingNegotiation(t *testing.T) { + tests := []struct { + name string + acceptEncoding string + wantCompressed bool + }{ + {"plain gzip", "gzip", true}, + {"gzip among several codings", "gzip, deflate, br", true}, + {"gzip with explicit q=1.0", "deflate, gzip;q=1.0, *;q=0.5", true}, + {"wildcard", "*", true}, + {"uppercase coding", "GZIP", true}, + {"gzip refused", "gzip;q=0", false}, + {"gzip refused alongside identity", "identity, gzip;q=0", false}, + {"gzip refused with trailing zeros", "gzip;q=0.000", false}, + {"wildcard refused", "*;q=0", false}, + {"explicit refusal outranks wildcard", "gzip;q=0, *", false}, + {"explicit acceptance alongside refused wildcard", "gzip, *;q=0", true}, + {"x-gzip lookalike", "x-gzip", false}, + {"other coding only", "deflate", false}, + {"empty header", "", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s, _ := newTestServer(Config{}) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/metrics", nil) + if tt.acceptEncoding != "" { + req.Header.Set("Accept-Encoding", tt.acceptEncoding) + } + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, req) + + got := rec.Header().Get("Content-Encoding") + if tt.wantCompressed && got != "gzip" { + t.Fatalf("Accept-Encoding %q: Content-Encoding = %q, want %q", tt.acceptEncoding, got, "gzip") + } + if !tt.wantCompressed && got != "" { + t.Fatalf("Accept-Encoding %q: Content-Encoding = %q, want identity (empty)", tt.acceptEncoding, got) + } + }) + } +} + +// TestAcceptsGzip_HeaderForms covers the Accept-Encoding grammar directly, +// including whitespace and parameter forms that are tedious to drive through +// a full request. +func TestAcceptsGzip_HeaderForms(t *testing.T) { + tests := []struct { + header string + want bool + }{ + {"gzip", true}, + {"gzip, deflate, br", true}, + {"deflate, gzip;q=1.0, *;q=0.5", true}, + {"*", true}, + {" gzip ; q = 0.5 ", true}, + {"GZip;Q=1", true}, + {"gzip;q=0.001", true}, + {"deflate, gzip", true}, + {"gzip;q=0", false}, + {"identity, gzip;q=0", false}, + {"gzip;q=0.000", false}, + {"gzip;q=0.", false}, + {"*;q=0", false}, + {"x-gzip", false}, + {"gzipped", false}, + {"deflate", false}, + {"identity;q=0", false}, + {"", false}, + {",", false}, + } + + for _, tt := range tests { + if got := acceptsGzip(tt.header); got != tt.want { + t.Errorf("acceptsGzip(%q) = %v, want %v", tt.header, got, tt.want) + } + } +}