From 5a0d56ac1d8e79dd47fc34187ba9fe17d1cb30f1 Mon Sep 17 00:00:00 2001 From: Pedro Cunha Date: Tue, 7 Jul 2026 16:54:47 +0100 Subject: [PATCH 1/2] fix: reject cross-realm FormData request bodies request() accepts FormData bodies through the duck-typed util.isFormDataLike() check, but only undici's own FormData class can be encoded: the multipart extraction is gated on the webidl brand check, so a FormData from another realm (Node.js' builtin globalThis.FormData, another undici copy, a polyfill) fell through and was dispatched as a body that never produced any bytes, hanging the request until an external timeout with zero bytes on the wire. Per review, reject such bodies instead of encoding them: the Request constructor now throws InvalidArgumentError when a FormData-like body is not an instance of undici's FormData, in the same place other invalid body types are rejected, so both the HTTP/1 and HTTP/2 clients reject cleanly at dispatch time. Note this is a behavior change relative to v6, which encoded cross-realm FormData successfully; v7.0.0 through 7.19.x failed these bodies with an unrelated TypeError and 7.20.0+ hung. Fixes: https://github.com/nodejs/undici/issues/5520 Signed-off-by: Pedro Cunha --- lib/core/request.js | 18 +++++- test/issue-5520.js | 145 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 test/issue-5520.js diff --git a/lib/core/request.js b/lib/core/request.js index 8e2072a46c7..5b9fef273ca 100644 --- a/lib/core/request.js +++ b/lib/core/request.js @@ -47,6 +47,11 @@ const kHandler = Symbol('handler') const kController = Symbol('controller') const kResume = Symbol('resume') +// Lazily loaded brand check for undici's own FormData class; only needed +// when a FormData-like body is passed, so the fetch machinery is not +// loaded upfront. +let isUndiciFormData + class RequestController { #paused = false #reason = null @@ -206,7 +211,18 @@ class Request { this.body = body.byteLength ? Buffer.from(body) : null } else if (typeof body === 'string') { this.body = body.length ? Buffer.from(body) : null - } else if (isFormDataLike(body) || isIterable(body) || isBlobLike(body)) { + } else if (isFormDataLike(body)) { + // Only undici's own FormData can be encoded. Instances from other + // realms (Node.js' builtin, another undici copy, a polyfill) would + // otherwise fall through to the iterable handling and produce a + // broken body, historically hanging the request. + // https://github.com/nodejs/undici/issues/5520 + isUndiciFormData ??= require('../web/webidl').webidl.is.FormData + if (!isUndiciFormData(body)) { + throw new InvalidArgumentError('FormData body must be an instance of undici\'s FormData class. FormData objects from other realms (e.g. Node.js\' global FormData) are not supported.') + } + this.body = body + } else if (isIterable(body) || isBlobLike(body)) { this.body = body } else { throw new InvalidArgumentError('body must be a string, a Buffer, a Readable stream, an iterable, or an async iterable') diff --git a/test/issue-5520.js b/test/issue-5520.js new file mode 100644 index 00000000000..e33bfaec4ce --- /dev/null +++ b/test/issue-5520.js @@ -0,0 +1,145 @@ +'use strict' + +const { test } = require('node:test') +const { createServer } = require('node:http') +const { createServer: createH2Server } = require('node:http2') +const { once } = require('node:events') +const { request, FormData: UndiciFormData, H2CClient, errors } = require('..') + +// https://github.com/nodejs/undici/issues/5520 +// request() accepts FormData bodies via a duck-typed check +// (util.isFormDataLike), but only undici's own FormData can be encoded. +// Instances from other realms (e.g. Node.js' builtin globalThis.FormData) +// used to fall through the brand-checked extraction and were dispatched as +// a body that never produced any bytes, hanging the request until an +// external timeout. They are now rejected upfront with InvalidArgumentError. + +function createEchoServer (t) { + const server = createServer((req, res) => { + const chunks = [] + req.on('data', (chunk) => chunks.push(chunk)) + req.on('end', () => { + res.setHeader('content-type', 'application/json') + res.end(JSON.stringify({ + contentType: req.headers['content-type'] ?? null, + contentLength: req.headers['content-length'] ?? null, + body: Buffer.concat(chunks).toString('utf8') + })) + }) + }) + t.after(() => { + server.closeAllConnections?.() + server.close() + }) + return server +} + +test('request() rejects a cross-realm (Node builtin) FormData body', { timeout: 60000 }, async (t) => { + // The premise of the test: the global FormData is not undici's own class. + t.assert.notStrictEqual(globalThis.FormData, UndiciFormData) + + const server = createEchoServer(t) + server.listen(0) + await once(server, 'listening') + + const form = new globalThis.FormData() + form.append('field', 'value') + form.append('file', new Blob(['file contents'], { type: 'text/plain' }), 'hello.txt') + + await t.assert.rejects( + request(`http://localhost:${server.address().port}/`, { method: 'POST', body: form }), + errors.InvalidArgumentError + ) +}) + +test('request() rejects a string-only cross-realm FormData body', { timeout: 60000 }, async (t) => { + const server = createEchoServer(t) + server.listen(0) + await once(server, 'listening') + + const form = new globalThis.FormData() + form.append('a', '1') + + await t.assert.rejects( + request(`http://localhost:${server.address().port}/`, { method: 'POST', body: form }), + errors.InvalidArgumentError + ) +}) + +test('HTTP/2 client rejects a cross-realm (Node builtin) FormData body', { timeout: 60000 }, async (t) => { + const server = createH2Server((req, res) => { + req.on('data', () => {}) + req.on('end', () => res.end('{}')) + }) + t.after(() => server.close()) + + server.listen(0) + await once(server, 'listening') + + const client = new H2CClient(`http://localhost:${server.address().port}`) + t.after(() => client.close()) + + const form = new globalThis.FormData() + form.append('field', 'value') + + await t.assert.rejects( + client.request({ path: '/', method: 'POST', body: form }), + errors.InvalidArgumentError + ) +}) + +test('request() still encodes undici\'s own FormData body', { timeout: 60000 }, async (t) => { + const server = createEchoServer(t) + server.listen(0) + await once(server, 'listening') + + const form = new UndiciFormData() + form.append('field', 'value with spaces') + form.append('file', new Blob(['file contents'], { type: 'text/plain' }), 'hello.txt') + + const res = await request(`http://localhost:${server.address().port}/`, { + method: 'POST', + body: form + }) + const echo = await res.body.json() + + t.assert.strictEqual(res.statusCode, 200) + const boundary = /^multipart\/form-data; boundary=(\S+)$/.exec(echo.contentType)?.[1] + t.assert.ok(boundary, `expected multipart content-type, got ${echo.contentType}`) + t.assert.strictEqual(echo.contentLength, String(Buffer.byteLength(echo.body))) + t.assert.ok(echo.body.includes('Content-Disposition: form-data; name="field"\r\n\r\nvalue with spaces\r\n')) + t.assert.ok(echo.body.includes('Content-Disposition: form-data; name="file"; filename="hello.txt"')) + t.assert.ok(echo.body.includes('file contents')) + t.assert.ok(echo.body.endsWith(`--${boundary}--\r\n`)) +}) + +test('HTTP/2 client still encodes undici\'s own FormData body', { timeout: 60000 }, async (t) => { + const server = createH2Server((req, res) => { + const chunks = [] + req.on('data', (chunk) => chunks.push(chunk)) + req.on('end', () => { + res.setHeader('content-type', 'application/json') + res.end(JSON.stringify({ + contentType: req.headers['content-type'] ?? null, + body: Buffer.concat(chunks).toString('utf8') + })) + }) + }) + t.after(() => server.close()) + + server.listen(0) + await once(server, 'listening') + + const client = new H2CClient(`http://localhost:${server.address().port}`) + t.after(() => client.close()) + + const form = new UndiciFormData() + form.append('file', new Blob(['file contents'], { type: 'text/plain' }), 'hello.txt') + + const res = await client.request({ path: '/', method: 'POST', body: form }) + const echo = await res.body.json() + + t.assert.strictEqual(res.statusCode, 200) + t.assert.ok(echo.body.includes('Content-Disposition: form-data; name="file"; filename="hello.txt"')) + t.assert.ok(echo.body.includes('file contents')) +}) From 6db392378a533e4d4144adab6e4df493ad0baaf5 Mon Sep 17 00:00:00 2001 From: Pedro Cunha Date: Tue, 7 Jul 2026 21:19:19 +0100 Subject: [PATCH 2/2] fixup: trim comments and redundant tests per review Drop the lazy-loading and rationale comments (the error message and issue link carry the context), the tautological precondition assert, and the tests already covered elsewhere: own-FormData encoding is tested by test/client-request.js and test/http2-body.js, and the string-only case is identical to the file case under rejection. Signed-off-by: Pedro Cunha --- lib/core/request.js | 8 ---- test/issue-5520.js | 99 ++------------------------------------------- 2 files changed, 4 insertions(+), 103 deletions(-) diff --git a/lib/core/request.js b/lib/core/request.js index 5b9fef273ca..d79526c57ef 100644 --- a/lib/core/request.js +++ b/lib/core/request.js @@ -47,9 +47,6 @@ const kHandler = Symbol('handler') const kController = Symbol('controller') const kResume = Symbol('resume') -// Lazily loaded brand check for undici's own FormData class; only needed -// when a FormData-like body is passed, so the fetch machinery is not -// loaded upfront. let isUndiciFormData class RequestController { @@ -212,11 +209,6 @@ class Request { } else if (typeof body === 'string') { this.body = body.length ? Buffer.from(body) : null } else if (isFormDataLike(body)) { - // Only undici's own FormData can be encoded. Instances from other - // realms (Node.js' builtin, another undici copy, a polyfill) would - // otherwise fall through to the iterable handling and produce a - // broken body, historically hanging the request. - // https://github.com/nodejs/undici/issues/5520 isUndiciFormData ??= require('../web/webidl').webidl.is.FormData if (!isUndiciFormData(body)) { throw new InvalidArgumentError('FormData body must be an instance of undici\'s FormData class. FormData objects from other realms (e.g. Node.js\' global FormData) are not supported.') diff --git a/test/issue-5520.js b/test/issue-5520.js index e33bfaec4ce..b343fac7f3f 100644 --- a/test/issue-5520.js +++ b/test/issue-5520.js @@ -4,41 +4,20 @@ const { test } = require('node:test') const { createServer } = require('node:http') const { createServer: createH2Server } = require('node:http2') const { once } = require('node:events') -const { request, FormData: UndiciFormData, H2CClient, errors } = require('..') +const { request, H2CClient, errors } = require('..') // https://github.com/nodejs/undici/issues/5520 -// request() accepts FormData bodies via a duck-typed check -// (util.isFormDataLike), but only undici's own FormData can be encoded. -// Instances from other realms (e.g. Node.js' builtin globalThis.FormData) -// used to fall through the brand-checked extraction and were dispatched as -// a body that never produced any bytes, hanging the request until an -// external timeout. They are now rejected upfront with InvalidArgumentError. -function createEchoServer (t) { +test('request() rejects a cross-realm (Node builtin) FormData body', { timeout: 60000 }, async (t) => { const server = createServer((req, res) => { - const chunks = [] - req.on('data', (chunk) => chunks.push(chunk)) - req.on('end', () => { - res.setHeader('content-type', 'application/json') - res.end(JSON.stringify({ - contentType: req.headers['content-type'] ?? null, - contentLength: req.headers['content-length'] ?? null, - body: Buffer.concat(chunks).toString('utf8') - })) - }) + req.on('data', () => {}) + req.on('end', () => res.end('{}')) }) t.after(() => { server.closeAllConnections?.() server.close() }) - return server -} - -test('request() rejects a cross-realm (Node builtin) FormData body', { timeout: 60000 }, async (t) => { - // The premise of the test: the global FormData is not undici's own class. - t.assert.notStrictEqual(globalThis.FormData, UndiciFormData) - const server = createEchoServer(t) server.listen(0) await once(server, 'listening') @@ -52,20 +31,6 @@ test('request() rejects a cross-realm (Node builtin) FormData body', { timeout: ) }) -test('request() rejects a string-only cross-realm FormData body', { timeout: 60000 }, async (t) => { - const server = createEchoServer(t) - server.listen(0) - await once(server, 'listening') - - const form = new globalThis.FormData() - form.append('a', '1') - - await t.assert.rejects( - request(`http://localhost:${server.address().port}/`, { method: 'POST', body: form }), - errors.InvalidArgumentError - ) -}) - test('HTTP/2 client rejects a cross-realm (Node builtin) FormData body', { timeout: 60000 }, async (t) => { const server = createH2Server((req, res) => { req.on('data', () => {}) @@ -87,59 +52,3 @@ test('HTTP/2 client rejects a cross-realm (Node builtin) FormData body', { timeo errors.InvalidArgumentError ) }) - -test('request() still encodes undici\'s own FormData body', { timeout: 60000 }, async (t) => { - const server = createEchoServer(t) - server.listen(0) - await once(server, 'listening') - - const form = new UndiciFormData() - form.append('field', 'value with spaces') - form.append('file', new Blob(['file contents'], { type: 'text/plain' }), 'hello.txt') - - const res = await request(`http://localhost:${server.address().port}/`, { - method: 'POST', - body: form - }) - const echo = await res.body.json() - - t.assert.strictEqual(res.statusCode, 200) - const boundary = /^multipart\/form-data; boundary=(\S+)$/.exec(echo.contentType)?.[1] - t.assert.ok(boundary, `expected multipart content-type, got ${echo.contentType}`) - t.assert.strictEqual(echo.contentLength, String(Buffer.byteLength(echo.body))) - t.assert.ok(echo.body.includes('Content-Disposition: form-data; name="field"\r\n\r\nvalue with spaces\r\n')) - t.assert.ok(echo.body.includes('Content-Disposition: form-data; name="file"; filename="hello.txt"')) - t.assert.ok(echo.body.includes('file contents')) - t.assert.ok(echo.body.endsWith(`--${boundary}--\r\n`)) -}) - -test('HTTP/2 client still encodes undici\'s own FormData body', { timeout: 60000 }, async (t) => { - const server = createH2Server((req, res) => { - const chunks = [] - req.on('data', (chunk) => chunks.push(chunk)) - req.on('end', () => { - res.setHeader('content-type', 'application/json') - res.end(JSON.stringify({ - contentType: req.headers['content-type'] ?? null, - body: Buffer.concat(chunks).toString('utf8') - })) - }) - }) - t.after(() => server.close()) - - server.listen(0) - await once(server, 'listening') - - const client = new H2CClient(`http://localhost:${server.address().port}`) - t.after(() => client.close()) - - const form = new UndiciFormData() - form.append('file', new Blob(['file contents'], { type: 'text/plain' }), 'hello.txt') - - const res = await client.request({ path: '/', method: 'POST', body: form }) - const echo = await res.body.json() - - t.assert.strictEqual(res.statusCode, 200) - t.assert.ok(echo.body.includes('Content-Disposition: form-data; name="file"; filename="hello.txt"')) - t.assert.ok(echo.body.includes('file contents')) -})