Skip to content

Commit ecf6798

Browse files
waleedlatif1waleedlatif1
committed
fix(next): keep route-handler framing for bodies Next did not decode
1 parent 12ca9d6 commit ecf6798

3 files changed

Lines changed: 234 additions & 1 deletion

File tree

‎apps/docs/scripts/test-header-merge-e2e.ts‎

Lines changed: 140 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
import { mkdir, writeFile } from 'node:fs/promises'
2-
import { createServer, type IncomingMessage, type ServerResponse } from 'node:http'
2+
import { createServer, get, type IncomingMessage, type ServerResponse } from 'node:http'
33
import { createRequire } from 'node:module'
44
import { dirname } from 'node:path'
55
import { pathToFileURL } from 'node:url'
66
import { createLogger } from '@sim/logger'
77
import { getErrorMessage } from '@sim/utils/errors'
88
import { toRecord } from '@sim/utils/object'
9+
import { NodeNextRequest, NodeNextResponse } from 'next/dist/server/base-http/node'
910
import { patchSetHeaderWithCookieSupport } from 'next/dist/server/lib/patch-set-header'
1011
import { addRequestMeta } from 'next/dist/server/request-meta'
12+
import { sendResponse } from 'next/dist/server/send-response'
1113

1214
const logger = createLogger('DocsHeaderMergeE2E')
1315
const reportPath = process.env.DOCS_HEADER_REPORT_PATH
@@ -80,11 +82,66 @@ const cases: HeaderCase[] = [
8082
},
8183
]
8284

85+
interface FramingCase {
86+
name: string
87+
body: string
88+
headers: Record<string, string>
89+
expected: { 'content-length'?: string; 'content-encoding'?: string }
90+
forbidden?: string[]
91+
}
92+
93+
const framingBody = 'route handler body'
94+
const framingCases: FramingCase[] = [
95+
{
96+
name: 'an unencoded body keeps its declared length',
97+
body: framingBody,
98+
headers: { 'Content-Length': String(framingBody.length) },
99+
expected: { 'content-length': String(framingBody.length) },
100+
},
101+
{
102+
name: 'an identity coding keeps its length and coding',
103+
body: framingBody,
104+
headers: { 'Content-Encoding': 'identity', 'Content-Length': String(framingBody.length) },
105+
expected: { 'content-length': String(framingBody.length), 'content-encoding': 'identity' },
106+
},
107+
{
108+
name: 'a streaming response keeps its no-compression marker',
109+
body: framingBody,
110+
headers: { 'Content-Encoding': 'none' },
111+
expected: { 'content-encoding': 'none' },
112+
},
113+
{
114+
name: 'a transparently decoded body drops its stale length and coding',
115+
body: framingBody,
116+
headers: { 'Content-Encoding': 'gzip', 'Content-Length': '7' },
117+
expected: {},
118+
forbidden: ['content-length', 'content-encoding'],
119+
},
120+
{
121+
name: 'a declared transfer coding is never forwarded',
122+
body: framingBody,
123+
headers: { 'Transfer-Encoding': 'gzip, chunked' },
124+
expected: {},
125+
},
126+
]
127+
83128
const require = createRequire(import.meta.url)
84129
const esmPath = require.resolve('next/dist/esm/server/lib/patch-set-header.js')
85130
const imported: unknown = await import(pathToFileURL(esmPath).href)
86131
const esmPatch = toRecord(imported).patchSetHeaderWithCookieSupport
87132
if (typeof esmPatch !== 'function') throw new Error('The ESM header merger is unavailable')
133+
const esmSendPath = require.resolve('next/dist/esm/server/send-response.js')
134+
const esmSendResponse = toRecord(await import(pathToFileURL(esmSendPath).href)).sendResponse
135+
if (typeof esmSendResponse !== 'function') throw new Error('The ESM response sender is unavailable')
136+
const senders: Array<{ name: string; send: typeof sendResponse }> = [
137+
{ name: 'CommonJS', send: sendResponse },
138+
{
139+
name: 'ESM',
140+
send: async (req, res, response) => {
141+
await esmSendResponse(req, res, response)
142+
},
143+
},
144+
]
88145

89146
const variants: Array<{
90147
name: string
@@ -187,6 +244,88 @@ try {
187244
)
188245
}
189246
}
247+
for (const sender of senders) {
248+
const server = createServer((req, res) => {
249+
const entry = framingCases[Number(req.url?.slice(1))]
250+
if (!entry) {
251+
res.writeHead(404).end()
252+
return
253+
}
254+
sender
255+
.send(
256+
new NodeNextRequest(req),
257+
new NodeNextResponse(res),
258+
new Response(entry.body, { headers: entry.headers })
259+
)
260+
.catch((error: unknown) => {
261+
res.statusCode = 500
262+
res.end(getErrorMessage(error))
263+
})
264+
})
265+
await new Promise<void>((resolve, reject) => {
266+
server.once('error', reject)
267+
server.listen(0, '127.0.0.1', resolve)
268+
})
269+
try {
270+
const address = server.address()
271+
if (!address || typeof address === 'string')
272+
throw new Error('No loopback listener is available')
273+
for (const [index, entry] of framingCases.entries()) {
274+
const started = performance.now()
275+
let error: string | undefined
276+
try {
277+
const { status, headers, body } = await new Promise<{
278+
status: number
279+
headers: IncomingMessage['headers']
280+
body: string
281+
}>((resolve, reject) => {
282+
const request = get(
283+
{ host: '127.0.0.1', port: address.port, path: `/${index}`, timeout: 5000 },
284+
(response) => {
285+
const chunks: Buffer[] = []
286+
response.on('data', (chunk: Buffer) => chunks.push(chunk))
287+
response.on('end', () =>
288+
resolve({
289+
status: response.statusCode ?? 0,
290+
headers: response.headers,
291+
body: Buffer.concat(chunks).toString('utf8'),
292+
})
293+
)
294+
response.on('error', reject)
295+
}
296+
)
297+
request.on('timeout', () => request.destroy(new Error('Request timed out')))
298+
request.on('error', reject)
299+
})
300+
if (status !== 200 || body !== entry.body)
301+
throw new Error(`Unexpected response: ${status} ${JSON.stringify(body)}`)
302+
for (const [name, value] of Object.entries(entry.expected))
303+
if (headers[name] !== value)
304+
throw new Error(`Expected ${name} ${value}, got ${String(headers[name])}`)
305+
for (const name of entry.forbidden ?? [])
306+
if (headers[name] !== undefined)
307+
throw new Error(`Stale ${name} was forwarded: ${String(headers[name])}`)
308+
if (
309+
entry.headers['Transfer-Encoding'] &&
310+
headers['transfer-encoding'] === 'gzip, chunked'
311+
)
312+
throw new Error('The declared transfer coding was forwarded')
313+
} catch (caught) {
314+
error = getErrorMessage(caught)
315+
}
316+
checks.push({
317+
name: `${sender.name}: ${entry.name}`,
318+
status: error ? 'failed' : 'passed',
319+
durationMs: Math.round(performance.now() - started),
320+
...(error ? { error } : {}),
321+
})
322+
}
323+
} finally {
324+
await new Promise<void>((resolve, reject) =>
325+
server.close((error) => (error ? reject(error) : resolve()))
326+
)
327+
}
328+
}
190329
} catch (error) {
191330
checks.push({
192331
name: 'Header suite lifecycle',

‎patches/README.md‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,34 @@ bun run --cwd apps/docs test:negotiation:e2e
140140
Remove the patch when upgrading to a release with equivalent Vary preservation,
141141
and retain the HTTP regression check.
142142

143+
# Next.js route-handler response framing
144+
145+
`next@16.4.5` (GHSA-pc58-hwjq-p385) drops `Content-Length`, `Content-Encoding`,
146+
and `Transfer-Encoding` from every Node route-handler response. The concern is a
147+
fetched upstream response returned unchanged: fetch decodes its body but keeps
148+
the original coding and length, so forwarding them frames a body that was never
149+
sent. Dropping the headers from every response also strips the declared length
150+
from file downloads, which then arrive chunked with no size, and the
151+
`Content-Encoding: none` marker that keeps streaming responses uncompressed.
152+
153+
The same file's patch narrows that rule in `sendResponse`, in both the CommonJS
154+
and ESM builds. A response whose `Content-Encoding` names a real coding keeps
155+
upstream's behavior: its length and coding are dropped. A response with no coding,
156+
or with `identity`/`none`, was not decoded, so its `Content-Length` and coding are
157+
forwarded. `Transfer-Encoding` is never forwarded; Node frames the response
158+
itself. Route handlers, App Page rendering, and the edge runtime are otherwise
159+
unchanged.
160+
161+
`test:headers:e2e` sends real responses through both builds of `sendResponse`
162+
over loopback HTTP. The unencoded, `identity`, and `none` checks fail against
163+
unpatched 16.4.5. The decoded-body and transfer-coding checks fail when their
164+
guards are removed; forwarding a decoded body's stale length leaves the client
165+
unable to parse the connection. The project-files E2E's large-download check
166+
asserts the declared length of a 100 MB file served through the running app.
167+
168+
Remove this part of the patch when upstream forwards framing for undecoded
169+
route-handler bodies, and keep both regression checks.
170+
143171
# Zod collection size bounds
144172

145173
`zod@4.3.6` runs an array's `.max()`/`.length()` (and a set's or map's `.max()`/`.size()`)

‎patches/next@16.4.5.patch‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,39 @@ index f2236f7ae222afcb8554267ed295ada59dceee6a..3be146fc8fa48196254d93a8a21c2cea
3434
if (name.toLowerCase() === 'set-cookie') {
3535
const middlewareValue = getRequestMeta(req, 'middlewareCookie');
3636
if (!middlewareValue || !Array.isArray(value) || !value.every((item, idx)=>item === middlewareValue[idx])) {
37+
diff --git a/dist/esm/server/send-response.js b/dist/esm/server/send-response.js
38+
index 080d60cb20a57b7e81411c12ed16e41e1fdbf621..4ee71ca70339473a26366d7ecee0ba6c0b64be92 100644
39+
--- a/dist/esm/server/send-response.js
40+
+++ b/dist/esm/server/send-response.js
41+
@@ -10,6 +10,12 @@ const FORBIDDEN_HEADERS = [
42+
'content-encoding',
43+
'transfer-encoding'
44+
];
45+
+// A body that declares no content coding was not transparently decoded, so its declared length
46+
+// still frames it; only a decoded body's length and coding are stale.
47+
+const IDENTITY_ENCODINGS = [
48+
+ 'identity',
49+
+ 'none'
50+
+];
51+
/**
52+
* Sends the response on the underlying next response object.
53+
*
54+
@@ -37,12 +43,14 @@ const FORBIDDEN_HEADERS = [
55+
'proxy-authenticate',
56+
'vary'
57+
];
58+
+ const encoding = (_response_headers = response.headers) == null ? void 0 : _response_headers.get('content-encoding');
59+
+ const bodyEncoded = typeof encoding === 'string' && !IDENTITY_ENCODINGS.includes(encoding.trim().toLowerCase());
60+
(_response_headers = response.headers) == null ? void 0 : _response_headers.forEach((value, name)=>{
61+
// `x-middleware-set-cookie` is an internal header not needed for the response
62+
if (name.toLowerCase() === 'x-middleware-set-cookie') {
63+
return;
64+
}
65+
- if (FORBIDDEN_HEADERS.includes(name)) {
66+
+ if (name === 'transfer-encoding' || bodyEncoded && FORBIDDEN_HEADERS.includes(name)) {
67+
return;
68+
}
69+
// The append handling is special cased for `set-cookie`.
3770
diff --git a/dist/server/lib/patch-set-header.js b/dist/server/lib/patch-set-header.js
3871
index 16cdc4703f3e9957a16c68c962cfaf5ee5d04561..a1c3c7061bc917284e01ed9947cd914818cfd708 100644
3972
--- a/dist/server/lib/patch-set-header.js
@@ -73,3 +106,36 @@ index 16cdc4703f3e9957a16c68c962cfaf5ee5d04561..a1c3c7061bc917284e01ed9947cd9148
73106
if (name.toLowerCase() === 'set-cookie') {
74107
const middlewareValue = (0, _requestmeta.getRequestMeta)(req, 'middlewareCookie');
75108
if (!middlewareValue || !Array.isArray(value) || !value.every((item, idx)=>item === middlewareValue[idx])) {
109+
diff --git a/dist/server/send-response.js b/dist/server/send-response.js
110+
index 6232e7a729e33f6550e83f8ff7ffe0581c891683..57dc52ecfc58b95e5b433932badd67c2b158450c 100644
111+
--- a/dist/server/send-response.js
112+
+++ b/dist/server/send-response.js
113+
@@ -20,6 +20,12 @@ const FORBIDDEN_HEADERS = [
114+
'content-encoding',
115+
'transfer-encoding'
116+
];
117+
+// A body that declares no content coding was not transparently decoded, so its declared length
118+
+// still frames it; only a decoded body's length and coding are stale.
119+
+const IDENTITY_ENCODINGS = [
120+
+ 'identity',
121+
+ 'none'
122+
+];
123+
async function sendResponse(req, res, response, waitUntil) {
124+
if (// The type check here ensures that `req` is correctly typed, and the
125+
// environment variable check provides dead code elimination.
126+
@@ -41,12 +47,14 @@ async function sendResponse(req, res, response, waitUntil) {
127+
'proxy-authenticate',
128+
'vary'
129+
];
130+
+ const encoding = (_response_headers = response.headers) == null ? void 0 : _response_headers.get('content-encoding');
131+
+ const bodyEncoded = typeof encoding === 'string' && !IDENTITY_ENCODINGS.includes(encoding.trim().toLowerCase());
132+
(_response_headers = response.headers) == null ? void 0 : _response_headers.forEach((value, name)=>{
133+
// `x-middleware-set-cookie` is an internal header not needed for the response
134+
if (name.toLowerCase() === 'x-middleware-set-cookie') {
135+
return;
136+
}
137+
- if (FORBIDDEN_HEADERS.includes(name)) {
138+
+ if (name === 'transfer-encoding' || bodyEncoded && FORBIDDEN_HEADERS.includes(name)) {
139+
return;
140+
}
141+
// The append handling is special cased for `set-cookie`.

0 commit comments

Comments
 (0)