Skip to content

fix(compress): respect Accept-Encoding when encoding option is set#4951

Merged
yusukebe merged 3 commits into
honojs:mainfrom
LeSingh1:fix/compress-respect-accept-encoding
May 22, 2026
Merged

fix(compress): respect Accept-Encoding when encoding option is set#4951
yusukebe merged 3 commits into
honojs:mainfrom
LeSingh1:fix/compress-respect-accept-encoding

Conversation

@LeSingh1

Copy link
Copy Markdown
Contributor

When the encoding option is provided to the compress middleware, the response is currently compressed regardless of what the client advertised in its Accept-Encoding header. This can produce responses the client cannot decode.

For example, with compress({ encoding: 'gzip' }):

  • A request with Accept-Encoding: deflate still receives a gzip-encoded body.
  • A request with no Accept-Encoding header still receives a gzip-encoded body.

The fallback path (when encoding is not set) already honors Accept-Encoding via ENCODING_TYPES.find((encoding) => accepted?.includes(encoding)). The configured-encoding path skipped that check.

This change checks the Accept-Encoding header 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.

…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

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.98%. Comparing base (a83ddb8) to head (fd161e0).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/middleware/compress/index.ts Outdated
? accepted?.includes(options.encoding)
? options.encoding
: undefined
: ENCODING_TYPES.find((encoding) => accepted?.includes(encoding))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@usualoma

Copy link
Copy Markdown
Member

Hi @LeSingh1, @yusukebe
Thank you for your consideration!

(Although this is more of an issue related to existing code,) since Accept-Encoding almost never includes a decimal value like q=0.5, I think it’s best to return immediately if q is not specified (i.e., 1).

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.
@usualoma

Copy link
Copy Markdown
Member

@LeSingh1 Thank you!

@yusukebe yusukebe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yusukebe

Copy link
Copy Markdown
Member

@LeSingh1 @usualoma Thank you! This is a good fix.

@yusukebe yusukebe merged commit f0ed246 into honojs:main May 22, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants