fix(middleware): respect Accept-Encoding q-values in Gzip#3021
Open
DucMinhNe wants to merge 1 commit into
Open
fix(middleware): respect Accept-Encoding q-values in Gzip#3021DucMinhNe wants to merge 1 commit into
DucMinhNe 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.
Comment on lines
+35
to
+54
| 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 | ||
| } |
Comment on lines
+444
to
+457
| 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")) | ||
| } |
Contributor
|
Are there examples how other frameworks, maybe in other programming languages are dealing with this? |
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
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.
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.