fix: preserve headers mutated after raw Response construction#357
Merged
Conversation
When a handler returns `new Response(body, init)` and appends headers
after construction (e.g. `res.headers.append('Set-Cookie', ...)`), the
headers were silently dropped once middleware cloned the response via
`new Response(c.res.body, c.res)` (the pattern used by `cors` and
`compress`).
There were two paths that lost the mutations:
- The constructor read `init.#init.headers` — the *original* init
passed at construction time — instead of `init.headers` (the live
getter backed by `cacheKey[2]`).
- `getResponseCache` built the underlying `GlobalResponse` from
`this.#init` without consulting the live `cacheKey[2]` headers, so
any access to a delegated property (`.body`, `.text()`, ...) before
the clone would freeze the stale init headers.
Both spots now consult the live `Headers` instance on `cacheKey[2]`
when present, so post-construction header mutations survive cloning.
Fixes #304
Member
|
Hi @abdulmunimjemal, |
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.
Fixes #304.
Problem
When a Hono handler returns
new Response(body, init)and appends headers after construction — e.g.res.headers.append('Set-Cookie', ...)— the appended headers are silently dropped once middleware such ascorsorcompressclones the response. The bug only manifests on@hono/node-server; the same code works correctly on Bun, Cloudflare Workers, and AWS Lambda.Minimal repro from the issue:
The
Set-Cookieheader never reaches the client.Root cause
src/response.tsdefines a lightweightResponse2that stores headers in two places:cacheKey[2]— the liveHeadersobject exposed by theheadersgetter, reflecting every.append()/.set()/.delete()mutation.this.#init.headers— the originalResponseInit.headersargument, never updated.Two code paths read from
#init.headersinstead of the live headers:getResponseCachebuilt the underlyingGlobalResponsefromthis.#init. Any access to a delegated property (.body,.text(), …) — which middleware does when streaming a raw body — froze the stale init headers and lost subsequent mutations.new Response(body, init)whereinitis anotherResponse2without a materializedresponseCache, readinit.#init.headersinstead ofinit.headers(the getter that returns the livecacheKey[2]).Fix
Both spots now consult the live
Headersinstance oncacheKey[2]when present, so post-construction header mutations survive cloning. The fix is local tosrc/response.ts; no public API changes.[getResponseCache](): globalThis.Response { + const cache = (this as LightResponse)[cacheKey] + const liveHeaders = cache && cache[2] instanceof Headers ? cache[2] : undefined delete (this as LightResponse)[cacheKey] - return ((this as LightResponse)[responseCache] ||= new GlobalResponse(this.#body, this.#init)) + return ((this as LightResponse)[responseCache] ||= new GlobalResponse( + this.#body, + liveHeaders ? { ...this.#init, headers: liveHeaders } : this.#init + )) }} else { this.#init = init.#init - headers = new Headers((init.#init as ResponseInit).headers) + headers = new Headers(init.headers) }new Headers(...)still produces an independent copy, so parent and child do not share the same object — the existing "Should not lose header data" test continues to pass.Tests
Two regression tests added:
test/response.test.ts— unit test covering both clone patterns:new Response('body', parent)beforegetResponseCacheis triggered (exercises the constructor fix).new Response(parent.body, parent)after.bodymaterializes the GlobalResponse (exercises thegetResponseCachefix).test/server.test.ts— end-to-end test that mirrors the issue: a middleware re-wrapsc.resvianew Response(c.res.body, c.res), and the handler returns a rawResponsewith an appendedSet-Cookie. Asserts that theSet-Cookiesurvives.Both tests fail on
mainand pass with the fix applied. The full suite stays green: 343 passed / 343 (12 files).