fix(compress): respect Accept-Encoding when encoding option is set#4951
Merged
yusukebe merged 3 commits intoMay 22, 2026
Merged
Conversation
…gured encoding When the encoding option is set, the middleware previously compressed responses unconditionally, regardless of the Accept-Encoding header. This could send a response in an encoding the client cannot decode (for example, returning gzip when the client only advertised deflate, or when Accept-Encoding is absent). Honor the client's Accept-Encoding header even when an encoding is explicitly configured, falling back to no compression when the configured encoding is not accepted.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4951 +/- ##
==========================================
+ Coverage 92.97% 92.98% +0.01%
==========================================
Files 177 177
Lines 12226 12247 +21
Branches 3711 3723 +12
==========================================
+ Hits 11367 11388 +21
Misses 859 859 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
yusukebe
reviewed
May 21, 2026
| ? accepted?.includes(options.encoding) | ||
| ? options.encoding | ||
| : undefined | ||
| : ENCODING_TYPES.find((encoding) => accepted?.includes(encoding)) |
Member
There was a problem hiding this comment.
includes can not cover all cases.
I think the following patch should be applied:
diff --git a/src/middleware/compress/index.test.ts b/src/middleware/compress/index.test.ts
index 827c74ef..e62b5ac3 100644
--- a/src/middleware/compress/index.test.ts
+++ b/src/middleware/compress/index.test.ts
@@ -301,6 +301,86 @@ describe('Compress Middleware', () => {
expect(res.headers.get('Content-Encoding')).toBeNull()
})
})
+
+ describe('Accept-Encoding parsing', () => {
+ const buildAppDefault = () => {
+ const app = new Hono()
+ app.use('*', compress())
+ app.get('/large', (c) => {
+ c.header('Content-Type', 'text/plain')
+ c.header('Content-Length', '1024')
+ return c.text('a'.repeat(1024))
+ })
+ return app
+ }
+
+ const buildAppWithEncoding = (encoding: 'gzip' | 'deflate') => {
+ const app = new Hono()
+ app.use('*', compress({ encoding }))
+ app.get('/large', (c) => {
+ c.header('Content-Type', 'text/plain')
+ c.header('Content-Length', '1024')
+ return c.text('a'.repeat(1024))
+ })
+ return app
+ }
+
+ it('should not compress when configured encoding has q=0', async () => {
+ const app = buildAppWithEncoding('gzip')
+ const res = await app.request('/large', {
+ headers: { 'Accept-Encoding': 'gzip;q=0' },
+ })
+ expect(res.headers.get('Content-Encoding')).toBeNull()
+ })
+
+ it('should fall back to another encoding when the preferred one has q=0', async () => {
+ const app = buildAppDefault()
+ const res = await app.request('/large', {
+ headers: { 'Accept-Encoding': 'gzip;q=0, deflate' },
+ })
+ expect(res.headers.get('Content-Encoding')).toBe('deflate')
+ })
+
+ it('should compress when Accept-Encoding is the wildcard *', async () => {
+ const app = buildAppWithEncoding('gzip')
+ const res = await app.request('/large', {
+ headers: { 'Accept-Encoding': '*' },
+ })
+ expect(res.headers.get('Content-Encoding')).toBe('gzip')
+ })
+
+ it('should compress when Accept-Encoding token differs only in case', async () => {
+ const app = buildAppWithEncoding('gzip')
+ const res = await app.request('/large', {
+ headers: { 'Accept-Encoding': 'GZIP' },
+ })
+ expect(res.headers.get('Content-Encoding')).toBe('gzip')
+ })
+
+ it('should not treat x-gzip as gzip', async () => {
+ const app = buildAppWithEncoding('gzip')
+ const res = await app.request('/large', {
+ headers: { 'Accept-Encoding': 'x-gzip' },
+ })
+ expect(res.headers.get('Content-Encoding')).toBeNull()
+ })
+
+ it('should pick the encoding with the highest q value', async () => {
+ const app = buildAppDefault()
+ const res = await app.request('/large', {
+ headers: { 'Accept-Encoding': 'gzip;q=0.5, deflate;q=0.9' },
+ })
+ expect(res.headers.get('Content-Encoding')).toBe('deflate')
+ })
+
+ it('should prefer gzip over deflate when q values are equal', async () => {
+ const app = buildAppDefault()
+ const res = await app.request('/large', {
+ headers: { 'Accept-Encoding': 'gzip;q=0.5, deflate;q=0.5' },
+ })
+ expect(res.headers.get('Content-Encoding')).toBe('gzip')
+ })
+ })
})
async function decompressResponse(res: Response): Promise<string> {
diff --git a/src/middleware/compress/index.ts b/src/middleware/compress/index.ts
index e400eaf0..5e08e572 100644
--- a/src/middleware/compress/index.ts
+++ b/src/middleware/compress/index.ts
@@ -4,16 +4,38 @@
*/
import type { MiddlewareHandler } from '../../types'
+import { parseAccept } from '../../utils/accept'
import { COMPRESSIBLE_CONTENT_TYPE_REGEX } from '../../utils/compress'
const ENCODING_TYPES = ['gzip', 'deflate'] as const
+type Encoding = (typeof ENCODING_TYPES)[number]
const cacheControlNoTransformRegExp = /(?:^|,)\s*?no-transform\s*?(?:,|$)/i
interface CompressionOptions {
- encoding?: (typeof ENCODING_TYPES)[number]
+ encoding?: Encoding
threshold?: number
}
+const selectEncoding = (
+ header: string | undefined,
+ candidates: readonly Encoding[]
+): Encoding | undefined => {
+ if (header === undefined) {
+ return undefined
+ }
+ const accepts = parseAccept(header)
+ const wildcardQ = accepts.find((a) => a.type === '*')?.q
+ let best: { encoding: Encoding; q: number } | undefined
+ for (const enc of candidates) {
+ const explicit = accepts.find((a) => a.type.toLowerCase() === enc)
+ const q = explicit ? explicit.q : (wildcardQ ?? 0)
+ if (q > 0 && (!best || q > best.q)) {
+ best = { encoding: enc, q }
+ }
+ }
+ return best?.encoding
+}
+
/**
* Compress Middleware for Hono.
*
@@ -33,6 +55,7 @@ interface CompressionOptions {
*/
export const compress = (options?: CompressionOptions): MiddlewareHandler => {
const threshold = options?.threshold ?? 1024
+ const candidates: readonly Encoding[] = options?.encoding ? [options.encoding] : ENCODING_TYPES
return async function compress(ctx, next) {
await next()
@@ -52,11 +75,7 @@ export const compress = (options?: CompressionOptions): MiddlewareHandler => {
}
const accepted = ctx.req.header('Accept-Encoding')
- const encoding = options?.encoding
- ? accepted?.includes(options.encoding)
- ? options.encoding
- : undefined
- : ENCODING_TYPES.find((encoding) => accepted?.includes(encoding))
+ const encoding = selectEncoding(accepted, candidates)
if (!encoding || !ctx.res.body) {
return
}
Hey @usualoma ! What do you think of this? If you have some time, can you take a look?
Address review from @yusukebe. The previous version used String.includes on the raw Accept-Encoding header, which treated x-gzip as gzip, ignored q=0 explicit refusals, and did not understand the wildcard. Switch to parseAccept (already used by accepts middleware) and pick the candidate encoding with the highest non-zero q value, falling back to the wildcard q if the candidate has no explicit entry. Add seven regression tests covering q=0 on the configured encoding, q=0 fallback to another candidate, wildcard \*, case-insensitive tokens, x-gzip non-matching, highest-q wins, and gzip-over-deflate tie-break.
Member
|
Hi @LeSingh1, @yusukebe (Although this is more of an issue related to existing code,) since Accept-Encoding almost never includes a decimal value like diff --git i/src/middleware/compress/index.ts w/src/middleware/compress/index.ts
index 5e08e572..38a02ca4 100644
--- i/src/middleware/compress/index.ts
+++ w/src/middleware/compress/index.ts
@@ -29,7 +29,9 @@ const selectEncoding = (
for (const enc of candidates) {
const explicit = accepts.find((a) => a.type.toLowerCase() === enc)
const q = explicit ? explicit.q : (wildcardQ ?? 0)
- if (q > 0 && (!best || q > best.q)) {
+ if (q === 1) {
+ return enc
+ } else if (q > 0 && (!best || q > best.q)) {
best = { encoding: enc, q }
}
} |
Per @usualoma: q values are almost always 1 in practice, so short-circuit and return immediately for the common path instead of continuing to scan the remaining candidates.
Member
|
@LeSingh1 Thank you! |
Member
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.
When the
encodingoption is provided to the compress middleware, the response is currently compressed regardless of what the client advertised in itsAccept-Encodingheader. This can produce responses the client cannot decode.For example, with
compress({ encoding: 'gzip' }):Accept-Encoding: deflatestill receives a gzip-encoded body.Accept-Encodingheader still receives a gzip-encoded body.The fallback path (when
encodingis not set) already honorsAccept-EncodingviaENCODING_TYPES.find((encoding) => accepted?.includes(encoding)). The configured-encoding path skipped that check.This change checks the
Accept-Encodingheader in both paths. When the configured encoding is not present in the client's accepted encodings, the response is left uncompressed, matching the documented behavior of the default path.Added three regression tests under a new "Encoding Option" describe block covering the accepted, not-accepted, and absent-header cases.