feat(context): make fetch Response headers mutable (#3318)

* make headers mutable

* add test

* immutability check

* revert change

* fix(context): clone response object if it's immutable

* test(runtime-node): specify ip address as numbers

* test(runtime-node): use address from server info instead of `127.0.0.1`

* test(runtime-node): use large date to test compress

---------

Co-authored-by: Taku Amano <taku@taaas.jp>
Co-authored-by: Yusuke Wada <yusuke@kamawada.com>

runtime_tests/node/index.test.ts

@@ -1,12 +1,14 @@
-import { createAdaptorServer } from '@hono/node-server'
+import type { Server } from 'node:http'
+import { createAdaptorServer, serve } from '@hono/node-server'
 import request from 'supertest'
 import { Hono } from '../../src'
 import { Context } from '../../src/context'
 import { env, getRuntimeKey } from '../../src/helper/adapter'
 import { basicAuth } from '../../src/middleware/basic-auth'
 import { jwt } from '../../src/middleware/jwt'
-import { HonoRequest } from '../../src/request'
+import { compress } from '../../src/middleware/compress'
 import { stream, streamSSE } from '../../src/helper/streaming'
+import type { AddressInfo } from 'node:net'
 
 // Test only minimal patterns.
 // See <https://github.com/honojs/node-server> for more tests and information.
@@ -38,7 +40,7 @@ describe('Basic', () => {
 
 describe('Environment Variables', () => {
   it('Should return the environment variable', async () => {
-    const c = new Context(new HonoRequest(new Request('http://localhost/')))
+    const c = new Context(new Request('http://localhost/'))
     const { NAME } = env<{ NAME: string }>(c)
     expect(NAME).toBe('Node')
   })
@@ -203,3 +205,44 @@ describe('streamSSE', () => {
     expect(aborted).toBe(false)
   })
 })
+
+describe('compress', async () => {
+  const cssContent = Array.from({ length: 60 }, () => 'body { color: red; }').join('\n')
+  const [externalServer, serverInfo] = await new Promise<[Server, AddressInfo]>((resolve) => {
+    const externalApp = new Hono()
+    externalApp.get('/style.css', (c) =>
+      c.text(cssContent, {
+        headers: {
+          'Content-Type': 'text/css',
+        },
+      })
+    )
+    const server = serve(
+      {
+        fetch: externalApp.fetch,
+        port: 0,
+      },
+      (serverInfo) => {
+        resolve([server as Server, serverInfo])
+      }
+    )
+  })
+
+  const app = new Hono()
+  app.use(compress())
+  app.get('/fetch/:file', (c) => {
+    return fetch(`http://${serverInfo.address}:${serverInfo.port}/${c.req.param('file')}`)
+  })
+  const server = createAdaptorServer(app)
+
+  afterAll(() => {
+    externalServer.close()
+  })
+
+  it('Should be compressed a fetch response', async () => {
+    const res = await request(server).get('/fetch/style.css')
+    expect(res.status).toBe(200)
+    expect(res.headers['content-encoding']).toBe('gzip')
+    expect(res.text).toBe(cssContent)
+  })
+})

src/context.test.ts

@@ -1,6 +1,29 @@
 import { Context } from './context'
 import { setCookie } from './helper/cookie'
 
+const makeResponseHeaderImmutable = (res: Response) => {
+  Object.defineProperty(res, 'headers', {
+    value: new Proxy(res.headers, {
+      set(target, prop, value) {
+        if (prop === 'set') {
+          throw new TypeError('Cannot modify headers: Headers are immutable')
+        }
+        return Reflect.set(target, prop, value)
+      },
+      get(target, prop) {
+        if (prop === 'set') {
+          return function () {
+            throw new TypeError('Cannot modify headers: Headers are immutable')
+          }
+        }
+        return Reflect.get(target, prop)
+      },
+    }),
+    writable: false,
+  })
+  return res
+}
+
 describe('Context', () => {
   const req = new Request('http://localhost/')
 
@@ -360,6 +383,28 @@ describe('Context header', () => {
     const res = c.text('Hi')
     expect(res.headers.get('set-cookie')).toBe('a, b, c')
   })
+
+  it('Should be able to overwrite a fetch response with a new response.', async () => {
+    c.res = makeResponseHeaderImmutable(new Response('bar'))
+    c.res = new Response('foo', {
+      headers: {
+        'X-Custom': 'Message',
+      },
+    })
+    expect(c.res.text()).resolves.toBe('foo')
+    expect(c.res.headers.get('X-Custom')).toBe('Message')
+  })
+
+  it('Should be able to overwrite a response with a fetch response.', async () => {
+    c.res = new Response('foo', {
+      headers: {
+        'X-Custom': 'Message',
+      },
+    })
+    c.res = makeResponseHeaderImmutable(new Response('bar'))
+    expect(c.res.text()).resolves.toBe('bar')
+    expect(c.res.headers.get('X-Custom')).toBe('Message')
+  })
 })
 
 describe('Pass a ResponseInit to respond methods', () => {

src/context.ts

@@ -465,16 +465,31 @@ export class Context<
   set res(_res: Response | undefined) {
     this.#isFresh = false
     if (this.#res && _res) {
-      this.#res.headers.delete('content-type')
-      for (const [k, v] of this.#res.headers.entries()) {
-        if (k === 'set-cookie') {
-          const cookies = this.#res.headers.getSetCookie()
-          _res.headers.delete('set-cookie')
-          for (const cookie of cookies) {
-            _res.headers.append('set-cookie', cookie)
+      try {
+        for (const [k, v] of this.#res.headers.entries()) {
+          if (k === 'content-type') {
+            continue
           }
+          if (k === 'set-cookie') {
+            const cookies = this.#res.headers.getSetCookie()
+            _res.headers.delete('set-cookie')
+            for (const cookie of cookies) {
+              _res.headers.append('set-cookie', cookie)
+            }
+          } else {
+            _res.headers.set(k, v)
+          }
+        }
+      } catch (e) {
+        if (e instanceof TypeError && e.message.includes('immutable')) {
+          // `_res` is immutable (probably a response from a fetch API), so retry with a new response.
+          this.res = new Response(_res.body, {
+            headers: _res.headers,
+            status: _res.status,
+          })
+          return
         } else {
-          _res.headers.set(k, v)
+          throw e
         }
       }
     }