From c4c39e25fd4c51b8d33d42a75863cce5f83b18bd Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 01:14:18 +0000 Subject: [PATCH 1/2] fix(node): reject overlapping body reads `toStandardBody` only guarded on `req.readable`, which turns false once the stream ends. A second read that overlapped the first, or followed a lazy (octet-stream / event-stream) read that had not been drained, attached a second async iterator to the request and silently got an empty body (`undefined` JSON, a 0-byte File, an empty stream or iterator) instead of an error. Track requests a parser has been attached to in a module-level WeakSet and mark them synchronously, before any parser touches the stream, so a second read now rejects with the same TypeError as the fetch and peer adapters. `hint: 'none'` and the upstream-parsed `req.body` path are unchanged, and the Fastify fallback parser gets the fix for free. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Se7vZ6pehYANTk37V7MB2V --- packages/fastify/src/request.test.ts | 34 ++++++++++ packages/node/src/body.test.ts | 94 ++++++++++++++++++++++++++++ packages/node/src/body.ts | 13 +++- 3 files changed, 140 insertions(+), 1 deletion(-) diff --git a/packages/fastify/src/request.test.ts b/packages/fastify/src/request.test.ts index 2e689eaf..f3119199 100644 --- a/packages/fastify/src/request.test.ts +++ b/packages/fastify/src/request.test.ts @@ -113,6 +113,40 @@ describe('toStandardLazyRequest', () => { expect(standardBody).toEqual(expectedBody) }) + it('fallback to standard body parser rejects overlapping body reads', async ({ + onTestFinished, + }) => { + let results: PromiseSettledResult[] = [] + + const fastify = Fastify() + onTestFinished(() => fastify.close()) + + // allow any content type, and leave the body unparsed + fastify.addContentTypeParser('*', (req, payload, done) => { + done(null, undefined) + }) + + fastify.all('/', async (req, reply) => { + const standardRequest = toStandardLazyRequest(req, reply) + results = await Promise.allSettled([ + standardRequest.resolveBody('json'), + standardRequest.resolveBody('json'), + ]) + }) + + await fastify.ready() + await request(fastify.server) + .post('/') + .set('content-type', 'application/octet-stream') + .send('{"amount":100}') + + const [first, second] = results as [PromiseFulfilledResult, PromiseRejectedResult] + expect(first).toEqual({ status: 'fulfilled', value: { amount: 100 } }) + expect(second.status).toBe('rejected') + expect(second.reason).toBeInstanceOf(TypeError) + expect(second.reason.message).toContain('Failed to read body: body stream already read') + }) + it('forwards the body hint to the standard body parser', async ({ onTestFinished }) => { let fastifyReq: any let standardBody: any diff --git a/packages/node/src/body.test.ts b/packages/node/src/body.test.ts index 4cf99bc8..33038888 100644 --- a/packages/node/src/body.test.ts +++ b/packages/node/src/body.test.ts @@ -457,6 +457,100 @@ describe('toStandardBody', () => { expect(await toStandardBody(req!, { hint: 'none' })).toBe(undefined) }) + it.each([ + ['json', '{"amount":100}', { amount: 100 }], + ['file', '{"amount":100}', '{"amount":100}'], + ['url-search-params', 'amount=100', new URLSearchParams('amount=100')], + ] as const)('throw on overlapping body reads (hint=%s)', async (hint, body, expected) => { + let results: PromiseSettledResult[] = [] + + await request(async (req: IncomingMessage, res: ServerResponse) => { + results = await Promise.allSettled([ + toStandardBody(req, { hint }), + toStandardBody(req, { hint }), + ]) + res.end() + }) + .post('/') + .send(body) + + const [first, second] = results as [PromiseFulfilledResult, PromiseRejectedResult] + expect(first.status).toBe('fulfilled') + expect(first.value instanceof File ? await first.value.text() : first.value).toEqual(expected) + expect(second.status).toBe('rejected') + expect(second.reason).toBeInstanceOf(TypeError) + expect(second.reason.message).toContain('Failed to read body: body stream already read') + }) + + it('throw on read body again before the octet-stream is drained', async () => { + let first: unknown + let secondError: unknown + let streamed: string | undefined + + await request(async (req: IncomingMessage, res: ServerResponse) => { + first = await toStandardBody(req, { hint: 'octet-stream' }) + secondError = await toStandardBody(req, { hint: 'octet-stream' }).catch((e) => e) + streamed = await new Response(first as ReadableStream).text() + res.end() + }) + .post('/') + .send('hello') + + expect(first).toBeInstanceOf(ReadableStream) + expect(streamed).toBe('hello') + expect(secondError).toBeInstanceOf(TypeError) + expect(secondError).toHaveProperty( + 'message', + expect.stringContaining('Failed to read body: body stream already read'), + ) + }) + + it('throw on read body again before the event-stream is drained', async () => { + let events: IteratorResult[] = [] + let secondError: unknown + + await request(async (req: IncomingMessage, res: ServerResponse) => { + const iterator = (await toStandardBody(req)) as AsyncIterator + secondError = await toStandardBody(req).catch((e) => e) + events = [await iterator.next(), await iterator.next(), await iterator.next()] + res.end() + }) + .post('/') + .type('text/event-stream') + .send('event: message\ndata: 1\n\nevent: message\ndata: 2\n\nevent: close\ndata: 3\n\n') + + expect(events).toEqual([ + { done: false, value: 1 }, + { done: false, value: 2 }, + { done: true, value: 3 }, + ]) + expect(secondError).toBeInstanceOf(TypeError) + expect(secondError).toHaveProperty( + 'message', + expect.stringContaining('Failed to read body: body stream already read'), + ) + }) + + it('hint=none neither reads nor marks the body', async () => { + let before: StandardBody = {} as any + let standardBody: StandardBody + let during: StandardBody = {} as any + + await request(async (req: IncomingMessage, res: ServerResponse) => { + before = await toStandardBody(req, { hint: 'none' }) + const reading = toStandardBody(req) + during = await toStandardBody(req, { hint: 'none' }) + standardBody = await reading + res.end() + }) + .post('/') + .send({ foo: 'bar' }) + + expect(before).toBe(undefined) + expect(during).toBe(undefined) + expect(standardBody).toEqual({ foo: 'bar' }) + }) + it('prefers user defined body hint over standard-server header', async () => { let standardBody: any diff --git a/packages/node/src/body.ts b/packages/node/src/body.ts index ca3a24fd..e60c41c7 100644 --- a/packages/node/src/body.ts +++ b/packages/node/src/body.ts @@ -20,6 +20,11 @@ export interface ToStandardBodyOptions { hint?: StandardBodyHint | undefined } +/** + * Requests whose body stream a parser has already been attached to. + */ +const consumedRequests = new WeakSet() + /** * Parses the body of a node http request. */ @@ -45,11 +50,17 @@ export async function toStandardBody( return undefined } - if (!req.readable) { + // `readable` only turns false once the stream ends, so it misses a read that overlaps + // another or follows a lazy (stream/iterator) one, where a second consumer would + // silently get an empty body. Like the fetch and peer adapters, reject reading it + // more than once. Marking happens synchronously, before any parser touches the stream. + if (!req.readable || consumedRequests.has(req)) { // native fetch error use TypeError throw new TypeError('Failed to read body: body stream already read or destroyed') } + consumedRequests.add(req) + if (hint === 'json') { const text = await _streamToString(req) return parseEmptyableJSON(text) From 1857b707c9a5745b21aa7f2c531012c1495d1e95 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 01:57:21 +0000 Subject: [PATCH 2/2] test(node): simplify overlapping body read assertions Assert the rejection with a single `toEqual(new TypeError(...))`, which checks both the error class and the exact message, instead of three different two-step forms. Read the drained stream with the already imported `text` helper, drop the redundant mid-read `hint: 'none'` call, and shorten the guard comment in `toStandardBody`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Se7vZ6pehYANTk37V7MB2V --- packages/fastify/src/request.test.ts | 12 ++++--- packages/node/src/body.test.ts | 49 +++++++++++----------------- packages/node/src/body.ts | 6 ++-- 3 files changed, 28 insertions(+), 39 deletions(-) diff --git a/packages/fastify/src/request.test.ts b/packages/fastify/src/request.test.ts index f3119199..c7048dbb 100644 --- a/packages/fastify/src/request.test.ts +++ b/packages/fastify/src/request.test.ts @@ -140,11 +140,13 @@ describe('toStandardLazyRequest', () => { .set('content-type', 'application/octet-stream') .send('{"amount":100}') - const [first, second] = results as [PromiseFulfilledResult, PromiseRejectedResult] - expect(first).toEqual({ status: 'fulfilled', value: { amount: 100 } }) - expect(second.status).toBe('rejected') - expect(second.reason).toBeInstanceOf(TypeError) - expect(second.reason.message).toContain('Failed to read body: body stream already read') + expect(results).toEqual([ + { status: 'fulfilled', value: { amount: 100 } }, + { + status: 'rejected', + reason: new TypeError('Failed to read body: body stream already read or destroyed'), + }, + ]) }) it('forwards the body hint to the standard body parser', async ({ onTestFinished }) => { diff --git a/packages/node/src/body.test.ts b/packages/node/src/body.test.ts index 33038888..bcd653f0 100644 --- a/packages/node/src/body.test.ts +++ b/packages/node/src/body.test.ts @@ -457,16 +457,21 @@ describe('toStandardBody', () => { expect(await toStandardBody(req!, { hint: 'none' })).toBe(undefined) }) + const alreadyReadError = new TypeError( + 'Failed to read body: body stream already read or destroyed', + ) + it.each([ ['json', '{"amount":100}', { amount: 100 }], ['file', '{"amount":100}', '{"amount":100}'], ['url-search-params', 'amount=100', new URLSearchParams('amount=100')], ] as const)('throw on overlapping body reads (hint=%s)', async (hint, body, expected) => { - let results: PromiseSettledResult[] = [] + let results: PromiseSettledResult[] = [] await request(async (req: IncomingMessage, res: ServerResponse) => { results = await Promise.allSettled([ - toStandardBody(req, { hint }), + // a File only compares by its metadata, so compare its text instead + toStandardBody(req, { hint }).then((b) => (b instanceof File ? b.text() : b)), toStandardBody(req, { hint }), ]) res.end() @@ -474,35 +479,27 @@ describe('toStandardBody', () => { .post('/') .send(body) - const [first, second] = results as [PromiseFulfilledResult, PromiseRejectedResult] - expect(first.status).toBe('fulfilled') - expect(first.value instanceof File ? await first.value.text() : first.value).toEqual(expected) - expect(second.status).toBe('rejected') - expect(second.reason).toBeInstanceOf(TypeError) - expect(second.reason.message).toContain('Failed to read body: body stream already read') + expect(results).toEqual([ + { status: 'fulfilled', value: expected }, + { status: 'rejected', reason: alreadyReadError }, + ]) }) it('throw on read body again before the octet-stream is drained', async () => { - let first: unknown let secondError: unknown let streamed: string | undefined await request(async (req: IncomingMessage, res: ServerResponse) => { - first = await toStandardBody(req, { hint: 'octet-stream' }) + const stream = (await toStandardBody(req, { hint: 'octet-stream' })) as ReadableStream secondError = await toStandardBody(req, { hint: 'octet-stream' }).catch((e) => e) - streamed = await new Response(first as ReadableStream).text() + streamed = await text(stream) res.end() }) .post('/') .send('hello') - expect(first).toBeInstanceOf(ReadableStream) expect(streamed).toBe('hello') - expect(secondError).toBeInstanceOf(TypeError) - expect(secondError).toHaveProperty( - 'message', - expect.stringContaining('Failed to read body: body stream already read'), - ) + expect(secondError).toEqual(alreadyReadError) }) it('throw on read body again before the event-stream is drained', async () => { @@ -524,30 +521,22 @@ describe('toStandardBody', () => { { done: false, value: 2 }, { done: true, value: 3 }, ]) - expect(secondError).toBeInstanceOf(TypeError) - expect(secondError).toHaveProperty( - 'message', - expect.stringContaining('Failed to read body: body stream already read'), - ) + expect(secondError).toEqual(alreadyReadError) }) it('hint=none neither reads nor marks the body', async () => { - let before: StandardBody = {} as any + let none: StandardBody = {} as any let standardBody: StandardBody - let during: StandardBody = {} as any await request(async (req: IncomingMessage, res: ServerResponse) => { - before = await toStandardBody(req, { hint: 'none' }) - const reading = toStandardBody(req) - during = await toStandardBody(req, { hint: 'none' }) - standardBody = await reading + none = await toStandardBody(req, { hint: 'none' }) + standardBody = await toStandardBody(req) res.end() }) .post('/') .send({ foo: 'bar' }) - expect(before).toBe(undefined) - expect(during).toBe(undefined) + expect(none).toBe(undefined) expect(standardBody).toEqual({ foo: 'bar' }) }) diff --git a/packages/node/src/body.ts b/packages/node/src/body.ts index e60c41c7..e16cd0ff 100644 --- a/packages/node/src/body.ts +++ b/packages/node/src/body.ts @@ -50,10 +50,8 @@ export async function toStandardBody( return undefined } - // `readable` only turns false once the stream ends, so it misses a read that overlaps - // another or follows a lazy (stream/iterator) one, where a second consumer would - // silently get an empty body. Like the fetch and peer adapters, reject reading it - // more than once. Marking happens synchronously, before any parser touches the stream. + // `readable` stays true until the stream ends, so it misses overlapping or lazy + // (stream/iterator) reads; mark the request before the first await to catch those. if (!req.readable || consumedRequests.has(req)) { // native fetch error use TypeError throw new TypeError('Failed to read body: body stream already read or destroyed')