fix(middleware): respect Accept-Encoding q-values in Gzip - #3021
fix(middleware): respect Accept-Encoding q-values in Gzip#3021DucMinhNe wants to merge 1 commit into
Conversation
The Gzip middleware decided whether to compress using a naive
strings.Contains check for "gzip" on the Accept-Encoding header.
Per RFC 9110 §12.5.3 this is incorrect in two ways:
- "Accept-Encoding: gzip;q=0" explicitly means the client does NOT
accept gzip, yet Contains still enabled compression.
- codings such as "x-gzip" or "supergzip" substring-matched "gzip"
even though they are distinct tokens.
Replace the substring check with a small Accept-Encoding parser that
matches the gzip (or "*") coding token and honours its q-value, skipping
compression on an explicit q=0 rejection. Add focused tests covering
q=0, q>0, identity, wildcard and substring false-positives.
There was a problem hiding this comment.
Pull request overview
This PR fixes Echo’s Gzip middleware decision logic by replacing a naive strings.Contains check on Accept-Encoding with token-based parsing that aims to follow RFC 9110 §12.5.3 (exact token matching and honoring q values).
Changes:
- Introduces an
acceptsGziphelper to parseAccept-Encodingcodings andqparameters instead of substring matching. - Updates the Gzip middleware to use
acceptsGzipwhen deciding whether to enable gzip compression. - Adds tests covering
q=0, wildcard*, multiple codings, and substring false-positives likesupergzip/x-gzip.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| middleware/compress.go | Adds acceptsGzip/parseCoding and switches middleware gating from substring match to parsed logic. |
| middleware/compress_test.go | Adds regression tests for Accept-Encoding parsing and gzip enablement behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func acceptsGzip(acceptEncoding string) bool { | ||
| if acceptEncoding == "" { | ||
| return false | ||
| } | ||
| for _, part := range strings.Split(acceptEncoding, ",") { | ||
| coding, qvalue := parseCoding(part) | ||
| if coding != gzipScheme && coding != "*" { | ||
| continue | ||
| } | ||
| if qvalue == "0" || qvalue == "0.0" || qvalue == "0.00" || qvalue == "0.000" { | ||
| // q=0 means "not acceptable"; an explicit gzip rejection wins. | ||
| if coding == gzipScheme { | ||
| return false | ||
| } | ||
| continue | ||
| } | ||
| return true | ||
| } | ||
| return false | ||
| } |
| func TestAcceptsGzip(t *testing.T) { | ||
| assert.True(t, acceptsGzip("gzip")) | ||
| assert.True(t, acceptsGzip(" GZIP ")) | ||
| assert.True(t, acceptsGzip("br, gzip;q=0.9")) | ||
| assert.True(t, acceptsGzip("*")) | ||
| assert.True(t, acceptsGzip("*;q=0.1")) | ||
| assert.False(t, acceptsGzip("")) | ||
| assert.False(t, acceptsGzip("gzip;q=0")) | ||
| assert.False(t, acceptsGzip("gzip; q=0.000")) | ||
| assert.False(t, acceptsGzip("identity")) | ||
| assert.False(t, acceptsGzip("supergzip")) | ||
| assert.False(t, acceptsGzip("x-gzip")) | ||
| assert.False(t, acceptsGzip("*;q=0")) | ||
| } |
|
Are there examples how other frameworks, maybe in other programming languages are dealing with this? |
|
Hey @aldas, sorry for the delayed reply! Here is a quick look at how other frameworks and standard libraries handle this:
Most frameworks do lightweight token parsing + |
The Gzip middleware decides whether to compress a response with a naive substring check:
Per RFC 9110 §12.5.3 this is incorrect in two ways:
Accept-Encoding: gzip;q=0explicitly means the client does not accept gzip ("A qvalue of 0 means not acceptable"), yetstrings.Containsstill enables compression.x-gziporsupergzipsubstring-matches"gzip"and wrongly triggers compression.Repro (before the fix)
A request with
Accept-Encoding: gzip;q=0returns a gzip-encoded body (Content-Encoding: gzip), even though the client asked not to be gzipped.Fix
Replace the substring check with a small
acceptsGzipparser that:gziptoken (or*) exactly, andq-value, skipping compression on an explicitq=0rejection.Tests
Added
TestGzip_AcceptEncodingQValueandTestAcceptsGzipcoveringgzip;q=0,q>0, multi-coding headers,identity, the*wildcard, and thesupergzip/x-gzipsubstring false-positives.go test ./middleware/...,gofmt -landgo vet ./middleware/are all clean. No public API change; behaviour only changes for headers that were previously handled incorrectly.