From c2b0de42a0fbe54c9862ed505c4b9ee46ed435ac Mon Sep 17 00:00:00 2001 From: nitedani <67418286+nitedani@users.noreply.github.com> Date: Sun, 8 Sep 2024 08:59:42 +0200 Subject: [PATCH] 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 Co-authored-by: Yusuke Wada --- runtime_tests/node/index.test.ts | 49 ++++++++++++++++++++++++++++++-- src/context.test.ts | 45 +++++++++++++++++++++++++++++ src/context.ts | 31 ++++++++++++++------ 3 files changed, 114 insertions(+), 11 deletions(-) diff --git a/runtime_tests/node/index.test.ts b/runtime_tests/node/index.test.ts index 92892ef8..b34cf23a 100644 --- a/runtime_tests/node/index.test.ts +++ b/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 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) + }) +}) diff --git a/src/context.test.ts b/src/context.test.ts index 8503220f..ee02296b 100644 --- a/src/context.test.ts +++ b/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', () => { diff --git a/src/context.ts b/src/context.ts index 7e5ce67e..beb13321 100644 --- a/src/context.ts +++ b/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 } } }