From 1c3b01cc2ef6c8d2b7207111ada4442ba3dd86d2 Mon Sep 17 00:00:00 2001 From: superbot-helper Date: Tue, 3 Nov 2026 00:13:54 -0600 Subject: [PATCH 1/2] fix(plugins): enforce maxFieldsSize and maxFields on json and urlencoded maxFieldsSize is compared only in _handlePart's field branch, so the two non-multipart parsers never check it: src/plugins/json.js assigned the parsed object with no size check src/plugins/querystring.js emitted each field with no size check README documents the option as "limit the amount of memory all fields together (except files) can allocate in bytes" with no content-type caveat, so this is a gap rather than a design choice. Because the JSON plugin also never emitted the `field` event, _setUpMaxFields never saw its keys and maxFields was ignored on that path too. Measured over real HTTP with maxFieldsSize: 1024 and maxFields: 2, before -> after: multipart field 64KiB 1006/413 -> 1006/413 (control, unchanged) urlencoded 2MiB accepted whole -> 1006/413 json 2MiB accepted whole -> 1006/413 json 3 keys, maxFields 3 keys accepted -> 1007/413 urlencoded 1025B accepted -> 1006/413 exactly 1024B accepted -> accepted (boundary held) The fix extracts the existing check into _addFieldSize() on Formidable and routes all three parsers through it, rather than duplicating the comparison. --- src/Formidable.js | 31 ++-- src/plugins/json.js | 13 +- src/plugins/querystring.js | 3 + test-node/standalone/field-limits.test.js | 167 ++++++++++++++++++++++ 4 files changed, 204 insertions(+), 10 deletions(-) create mode 100644 test-node/standalone/field-limits.test.js diff --git a/src/Formidable.js b/src/Formidable.js index 733ff673..4a87568e 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -372,15 +372,7 @@ class IncomingForm extends EventEmitter { ); part.on("data", (buffer) => { - this._fieldsSize += buffer.length; - if (this._fieldsSize > this.options.maxFieldsSize) { - this._error( - new FormidableError( - `options.maxFieldsSize (${this.options.maxFieldsSize} bytes) exceeded, received ${this._fieldsSize} bytes of field data`, - errors.maxFieldsSizeExceeded, - 413 // Payload Too Large - ) - ); + if (!this._addFieldSize(buffer.length)) { return; } value += decoder.write(buffer); @@ -684,6 +676,27 @@ class IncomingForm extends EventEmitter { } } + /** + * Account `size` bytes of field data against `options.maxFieldsSize`. + * Shared by the multipart field parser and the buffering JSON/urlencoded + * plugins so the option applies to every built-in parser, not just multipart. + * Returns false (after emitting the error) when the limit is exceeded. + */ + _addFieldSize(size) { + this._fieldsSize += size; + if (this._fieldsSize > this.options.maxFieldsSize) { + this._error( + new FormidableError( + `options.maxFieldsSize (${this.options.maxFieldsSize} bytes) exceeded, received ${this._fieldsSize} bytes of field data`, + errors.maxFieldsSizeExceeded, + 413 // Payload Too Large + ) + ); + return false; + } + return true; + } + _setUpMaxFields() { if (this.options.maxFields !== Infinity) { let fieldsCount = 0; diff --git a/src/plugins/json.js b/src/plugins/json.js index 00ef60da..ba5a4291 100644 --- a/src/plugins/json.js +++ b/src/plugins/json.js @@ -27,7 +27,18 @@ function init(_self, _opts) { const parser = new JSONParser(this.options); parser.on("data", (fields) => { - this.fields = Object.assign(Object.create(null), fields); + // Emit each field through the regular `field` path instead of assigning + // `this.fields` wholesale, so the shared field accounting (maxFieldsSize) + // and the maxFields listener apply to the JSON parser as well. + Object.entries(fields).every(([name, value]) => { + const serialized = + typeof value === "string" ? value : JSON.stringify(value); + if (!this._addFieldSize(Buffer.byteLength(serialized || ""))) { + return false; + } + this.emit("field", name, value); + return !this.error; + }); }); parser.once("end", () => { diff --git a/src/plugins/querystring.js b/src/plugins/querystring.js index bc9df6d2..7320e7fe 100644 --- a/src/plugins/querystring.js +++ b/src/plugins/querystring.js @@ -26,6 +26,9 @@ function init(_self, _opts) { const parser = new QuerystringParser(this.options); parser.on("data", ({ key, value }) => { + if (!this._addFieldSize(Buffer.byteLength(value))) { + return; + } this.emit("field", key, value); }); diff --git a/test-node/standalone/field-limits.test.js b/test-node/standalone/field-limits.test.js new file mode 100644 index 00000000..5634c39d --- /dev/null +++ b/test-node/standalone/field-limits.test.js @@ -0,0 +1,167 @@ +import { deepStrictEqual, strictEqual } from 'node:assert'; +import { createServer } from 'node:http'; +import test from 'node:test'; +import formidable, { errors } from '../../src/index.js'; + +const MAX_FIELDS_SIZE = 1024; +const MAX_FIELDS = 2; +const BIG = 2 * 1024 * 1024; + +// Parse one body with the given options and return the parse callback outcome. +async function parseBody(options, contentType, body) { + let outcome; + const server = createServer((req, res) => { + const form = formidable(options); + form.parse(req, (err, fields) => { + outcome = { err, fields }; + res.writeHead(204); + res.end(); + }); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address(); + try { + const res = await fetch(`http://127.0.0.1:${port}/`, { + method: 'POST', + headers: { + 'content-type': contentType, + 'content-length': String(body.length), + }, + body, + }); + await res.arrayBuffer(); + // The parse callback runs before the response is written; give the + // microtask queue a tick so `outcome` is visible here. + await new Promise((resolve) => setImmediate(resolve)); + } finally { + server.closeAllConnections(); + await new Promise((resolve) => server.close(resolve)); + } + return outcome; +} + +const limits = { maxFieldsSize: MAX_FIELDS_SIZE, maxFields: MAX_FIELDS }; +const json = (obj) => Buffer.from(JSON.stringify(obj)); +const urlenc = (pairs) => + Buffer.from(pairs.map(([k, v]) => `${encodeURIComponent(k)}=${encodeURIComponent(v)}`).join('&')); + +function multipartField(name, data) { + const boundary = `----field-limits-${Math.random().toString(16).slice(2)}`; + const head = Buffer.from( + `--${boundary}\r\nContent-Disposition: form-data; name="${name}"\r\n\r\n`, + ); + const tail = Buffer.from(`\r\n--${boundary}--\r\n`); + return { + type: `multipart/form-data; boundary=${boundary}`, + body: Buffer.concat([head, data, tail]), + }; +} + +function assertRejected(outcome, code) { + strictEqual(outcome.err?.code, code); + strictEqual(outcome.err?.httpCode, 413); +} + +test('maxFieldsSize is enforced for application/json', async () => { + const outcome = await parseBody( + limits, + 'application/json', + json({ blob: 'a'.repeat(BIG) }), + ); + assertRejected(outcome, errors.maxFieldsSizeExceeded); +}); + +test('maxFieldsSize is enforced for application/x-www-form-urlencoded', async () => { + const outcome = await parseBody( + limits, + 'application/x-www-form-urlencoded', + urlenc([['blob', 'a'.repeat(BIG)]]), + ); + assertRejected(outcome, errors.maxFieldsSizeExceeded); +}); + +test('maxFieldsSize is enforced for multipart fields (control)', async () => { + const part = multipartField('blob', Buffer.alloc(BIG, 0x61)); + const outcome = await parseBody(limits, part.type, part.body); + assertRejected(outcome, errors.maxFieldsSizeExceeded); +}); + +test('maxFields is enforced for application/json', async () => { + const outcome = await parseBody(limits, 'application/json', json({ a: 1, b: 2, c: 3 })); + assertRejected(outcome, errors.maxFieldsExceeded); +}); + +test('maxFields is enforced for application/x-www-form-urlencoded (control)', async () => { + const outcome = await parseBody( + limits, + 'application/x-www-form-urlencoded', + urlenc([ + ['a', '1'], + ['b', '2'], + ['c', '3'], + ]), + ); + assertRejected(outcome, errors.maxFieldsExceeded); +}); + +test('maxFieldsSize boundary: a field equal to the limit is accepted', async () => { + const jsonOutcome = await parseBody( + limits, + 'application/json', + json({ blob: 'a'.repeat(MAX_FIELDS_SIZE) }), + ); + strictEqual(jsonOutcome.err, null); + strictEqual(jsonOutcome.fields.blob.length, MAX_FIELDS_SIZE); + + const urlencodedOutcome = await parseBody( + limits, + 'application/x-www-form-urlencoded', + urlenc([['blob', 'a'.repeat(MAX_FIELDS_SIZE)]]), + ); + strictEqual(urlencodedOutcome.err, null); + strictEqual(urlencodedOutcome.fields.blob[0].length, MAX_FIELDS_SIZE); +}); + +test('maxFieldsSize boundary: a field one byte over the limit is rejected', async () => { + const jsonOutcome = await parseBody( + limits, + 'application/json', + json({ blob: 'a'.repeat(MAX_FIELDS_SIZE + 1) }), + ); + assertRejected(jsonOutcome, errors.maxFieldsSizeExceeded); + + const urlencodedOutcome = await parseBody( + limits, + 'application/x-www-form-urlencoded', + urlenc([['blob', 'a'.repeat(MAX_FIELDS_SIZE + 1)]]), + ); + assertRejected(urlencodedOutcome, errors.maxFieldsSizeExceeded); +}); + +test('JSON fields are still parsed and collected when under the limits', async () => { + const outcome = await parseBody( + { ...limits, maxFields: 10 }, + 'application/json', + json({ a: 'b', nested: { k: 'v' }, list: [1, 2] }), + ); + strictEqual(outcome.err, null); + deepStrictEqual({ ...outcome.fields }, { + a: 'b', + nested: { k: 'v' }, + list: [1, 2], + }); + strictEqual(Object.getPrototypeOf(outcome.fields), null); +}); + +test('urlencoded fields are still parsed and collected when under the limits', async () => { + const outcome = await parseBody( + limits, + 'application/x-www-form-urlencoded', + urlenc([ + ['a', 'b'], + ['c', 'd'], + ]), + ); + strictEqual(outcome.err, null); + deepStrictEqual({ ...outcome.fields }, { a: ['b'], c: ['d'] }); +}); From 271ffe896e26ee440c3560521e5b03ff688d9eaf Mon Sep 17 00:00:00 2001 From: Superbot Helper Date: Mon, 9 Nov 2026 16:05:19 -0700 Subject: [PATCH 2/2] fix(json): count field bytes without a recursive stringify JSON.stringify recurses per nesting level: on Node 18/20/22 a 20 KB body with 10,000 nested arrays is accepted by JSON.parse but throws RangeError inside the new maxFieldsSize accounting, so the parse fails below both limits. Measure the value with an explicit stack instead. The walk only covers types JSON.parse can produce, so it stays exact for objects, arrays and strings, and it does not run user code. Adds regressions for the deep value (accepted under the limit, rejected with maxFieldsSizeExceeded over it) and for combined-size accounting: two fields that each fit but together exceed maxFieldsSize, plus the exact combined boundary, for JSON and urlencoded. Verified with node --test test-node/standalone/field-limits.test.js on Node 18/20/24 (13/13 pass; the two deep-JSON tests fail on the pre-fix plugin) and jest test/integration/json + test/unit/custom-plugins. --- src/plugins/json.js | 53 ++++++++++++++++-- test-node/standalone/field-limits.test.js | 65 +++++++++++++++++++++++ 2 files changed, 115 insertions(+), 3 deletions(-) diff --git a/src/plugins/json.js b/src/plugins/json.js index ba5a4291..53631413 100644 --- a/src/plugins/json.js +++ b/src/plugins/json.js @@ -18,6 +18,51 @@ export default function plugin(formidable, options) { return self; } +/** + * Byte length of a JSON value's encoding, counted with an explicit stack. + * `JSON.stringify` recurses per nesting level and overflows the call stack on a + * deeply nested value that `JSON.parse` (and so this parser) accepted; a value + * that reached this handler is plain JSON data, so the walk only has to cover + * the types `JSON.parse` can produce. + */ +function jsonByteLength(value) { + let bytes = 0; + const stack = [value]; + + while (stack.length > 0) { + const current = stack.pop(); + + if (current === null) { + bytes += 4; // "null" + } else if (typeof current === "string") { + bytes += Buffer.byteLength(JSON.stringify(current)); + } else if (typeof current === "number") { + // JSON.stringify encodes a non-finite number as "null" + bytes += Number.isFinite(current) + ? Buffer.byteLength(String(current)) + : 4; + } else if (typeof current === "boolean") { + bytes += current ? 4 : 5; + } else if (Array.isArray(current)) { + bytes += 2 + Math.max(current.length - 1, 0); // brackets and commas + for (let index = 0; index < current.length; index += 1) { + stack.push(current[index]); + } + } else if (typeof current === "object") { + const keys = Object.keys(current); + bytes += 2 + Math.max(keys.length - 1, 0); // braces and commas + for (let index = 0; index < keys.length; index += 1) { + const key = keys[index]; + bytes += Buffer.byteLength(JSON.stringify(key)) + 1; // key and colon + stack.push(current[key]); + } + } + // undefined, functions and symbols cannot come out of JSON.parse + } + + return bytes; +} + // Note that it's a good practice (but it's up to you) to use the `this.options` instead // of the passed `options` (second) param, because when you decide // to test the plugin you can pass custom `this` context to it (and so `this.options`) @@ -31,9 +76,11 @@ function init(_self, _opts) { // `this.fields` wholesale, so the shared field accounting (maxFieldsSize) // and the maxFields listener apply to the JSON parser as well. Object.entries(fields).every(([name, value]) => { - const serialized = - typeof value === "string" ? value : JSON.stringify(value); - if (!this._addFieldSize(Buffer.byteLength(serialized || ""))) { + const size = + typeof value === "string" + ? Buffer.byteLength(value) + : jsonByteLength(value); + if (!this._addFieldSize(size)) { return false; } this.emit("field", name, value); diff --git a/test-node/standalone/field-limits.test.js b/test-node/standalone/field-limits.test.js index 5634c39d..0034c2ef 100644 --- a/test-node/standalone/field-limits.test.js +++ b/test-node/standalone/field-limits.test.js @@ -165,3 +165,68 @@ test('urlencoded fields are still parsed and collected when under the limits', a strictEqual(outcome.err, null); deepStrictEqual({ ...outcome.fields }, { a: ['b'], c: ['d'] }); }); + +test('maxFieldsSize counts JSON fields together, not one at a time', async () => { + const half = MAX_FIELDS_SIZE / 2; + const atLimit = await parseBody( + limits, + 'application/json', + json({ a: 'a'.repeat(half), b: 'b'.repeat(half) }), + ); + strictEqual(atLimit.err, null); + + const overLimit = await parseBody( + limits, + 'application/json', + json({ a: 'a'.repeat(half), b: 'b'.repeat(half + 1) }), + ); + assertRejected(overLimit, errors.maxFieldsSizeExceeded); +}); + +test('maxFieldsSize counts urlencoded fields together, not one at a time', async () => { + const half = MAX_FIELDS_SIZE / 2; + const atLimit = await parseBody( + limits, + 'application/x-www-form-urlencoded', + urlenc([ + ['a', 'a'.repeat(half)], + ['b', 'b'.repeat(half)], + ]), + ); + strictEqual(atLimit.err, null); + + const overLimit = await parseBody( + limits, + 'application/x-www-form-urlencoded', + urlenc([ + ['a', 'a'.repeat(half)], + ['b', 'b'.repeat(half + 1)], + ]), + ); + assertRejected(overLimit, errors.maxFieldsSizeExceeded); +}); + +// 10,000 nested arrays: JSON.parse accepts this depth, but JSON.stringify +// recurses per level and throws RangeError on Node 18/20/22 at this size. +const DEEP = 10000; +const deeplyNested = (depth) => `${'['.repeat(depth)}${']'.repeat(depth)}`; +const deepBody = (depth) => Buffer.from(`{"deep":${deeplyNested(depth)}}`); + +test('a deeply nested JSON value is measured without overflowing the stack', async () => { + const outcome = await parseBody( + { maxFieldsSize: 4 * DEEP, maxFields: 10 }, + 'application/json', + deepBody(DEEP), + ); + strictEqual(outcome.err, null); + strictEqual(Array.isArray(outcome.fields.deep), true); +}); + +test('a deeply nested JSON value over the limit is rejected, not a RangeError', async () => { + const outcome = await parseBody( + { maxFieldsSize: MAX_FIELDS_SIZE, maxFields: 10 }, + 'application/json', + deepBody(DEEP), + ); + assertRejected(outcome, errors.maxFieldsSizeExceeded); +});