Skip to content

Commit 380984a

Browse files
enisdenjogithub-actions[bot]ardatan
authored
Parsing form data with unexpected Content-Length (#2305)
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Arda TANRIKULU <[email protected]>
1 parent 1ef44e8 commit 380984a

15 files changed

+298
-70
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@whatwg-node/node-fetch": patch
3+
---
4+
dependencies updates:
5+
- Added dependency [`@fastify/busboy@^3.1.1` ↗︎](https://www.npmjs.com/package/@fastify/busboy/v/3.1.1) (to `dependencies`)
6+
- Removed dependency [`busboy@^1.6.0` ↗︎](https://www.npmjs.com/package/busboy/v/1.6.0) (from `dependencies`)

.changeset/chatty-years-say.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@whatwg-node/server': patch
3+
---
4+
5+
Handle request abort signals with streamed body on uWS adapter

.changeset/curvy-geese-beg.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@whatwg-node/node-fetch': patch
3+
---
4+
5+
Abort parsing form data if the request is aborted

.changeset/short-loops-invite.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@whatwg-node/node-fetch': patch
3+
---
4+
5+
Handle parsing form data when supplied Content-Length header value is smaller than the actual data

packages/node-fetch/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,12 @@
3434
},
3535
"typings": "dist/typings/index.d.ts",
3636
"dependencies": {
37+
"@fastify/busboy": "^3.1.1",
3738
"@whatwg-node/disposablestack": "^0.0.6",
3839
"@whatwg-node/promise-helpers": "^1.2.5",
39-
"busboy": "^1.6.0",
4040
"tslib": "^2.6.3"
4141
},
4242
"devDependencies": {
43-
"@types/busboy": "1.5.4",
4443
"@types/pem": "^1.14.0",
4544
"pem": "^1.14.8"
4645
},

packages/node-fetch/src/Body.ts

Lines changed: 84 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/* eslint-disable @typescript-eslint/ban-ts-comment */
22
import { Buffer } from 'node:buffer';
33
import { IncomingMessage } from 'node:http';
4-
import { Readable } from 'node:stream';
5-
import busboy from 'busboy';
4+
import { addAbortSignal, Readable } from 'node:stream';
5+
import { Busboy, BusboyFileStream } from '@fastify/busboy';
66
import { handleMaybePromise, MaybePromise } from '@whatwg-node/promise-helpers';
77
import { hasArrayBufferMethod, hasBufferMethod, hasBytesMethod, PonyfillBlob } from './Blob.js';
88
import { PonyfillFile } from './File.js';
@@ -244,59 +244,104 @@ export class PonyfillBody<TJSON = any> implements Body {
244244
...opts?.formDataLimits,
245245
};
246246
return new Promise((resolve, reject) => {
247-
const bb = busboy({
247+
const stream = this.body?.readable;
248+
if (!stream) {
249+
return reject(new Error('No stream available'));
250+
}
251+
252+
// form data file that is currently being processed, it's
253+
// important to keep track of it in case the stream ends early
254+
let currFile: BusboyFileStream | null = null;
255+
256+
const bb = new Busboy({
248257
headers: {
258+
'content-length':
259+
typeof this.contentLength === 'number'
260+
? this.contentLength.toString()
261+
: this.contentLength || '',
249262
'content-type': this.contentType || '',
250263
},
251264
limits: formDataLimits,
252-
defParamCharset: 'utf-8',
265+
defCharset: 'utf-8',
253266
});
254-
bb.on('field', (name, value, { nameTruncated, valueTruncated }) => {
255-
if (nameTruncated) {
256-
reject(new Error(`Field name size exceeded: ${formDataLimits?.fieldNameSize} bytes`));
267+
268+
if (this.signal) {
269+
addAbortSignal(this.signal, bb);
270+
}
271+
272+
let completed = false;
273+
const complete = (err: unknown) => {
274+
if (completed) return;
275+
completed = true;
276+
stream!.unpipe(bb);
277+
bb.destroy();
278+
if (currFile) {
279+
currFile.destroy();
280+
currFile = null;
281+
}
282+
if (err) {
283+
reject(err);
284+
} else {
285+
// no error occured, this is a successful end/complete/finish
286+
resolve(this._formData!);
287+
}
288+
};
289+
290+
// we dont need to listen to the stream close event because bb will close or error when necessary
291+
// stream.on('close', complete);
292+
293+
// stream can be aborted, for example
294+
stream.on('error', complete);
295+
296+
bb.on('field', (name, value, fieldnameTruncated, valueTruncated) => {
297+
if (fieldnameTruncated) {
298+
return complete(
299+
new Error(`Field name size exceeded: ${formDataLimits?.fieldNameSize} bytes`),
300+
);
257301
}
258302
if (valueTruncated) {
259-
reject(new Error(`Field value size exceeded: ${formDataLimits?.fieldSize} bytes`));
303+
return complete(
304+
new Error(`Field value size exceeded: ${formDataLimits?.fieldSize} bytes`),
305+
);
260306
}
261307
this._formData!.set(name, value);
262308
});
309+
310+
bb.on('file', (name, fileStream, filename, _transferEncoding, mimeType) => {
311+
currFile = fileStream;
312+
const chunks: BlobPart[] = [];
313+
fileStream.on('data', chunk => {
314+
chunks.push(chunk);
315+
});
316+
fileStream.on('error', complete);
317+
fileStream.on('limit', () => {
318+
complete(new Error(`File size limit exceeded: ${formDataLimits?.fileSize} bytes`));
319+
});
320+
fileStream.on('close', () => {
321+
if (fileStream.truncated) {
322+
complete(new Error(`File size limit exceeded: ${formDataLimits?.fileSize} bytes`));
323+
}
324+
currFile = null;
325+
const file = new PonyfillFile(chunks, filename, { type: mimeType });
326+
this._formData!.set(name, file);
327+
});
328+
});
329+
263330
bb.on('fieldsLimit', () => {
264-
reject(new Error(`Fields limit exceeded: ${formDataLimits?.fields}`));
331+
complete(new Error(`Fields limit exceeded: ${formDataLimits?.fields}`));
265332
});
266-
bb.on(
267-
'file',
268-
(name, fileStream: Readable & { truncated: boolean }, { filename, mimeType }) => {
269-
const chunks: BlobPart[] = [];
270-
fileStream.on('limit', () => {
271-
reject(new Error(`File size limit exceeded: ${formDataLimits?.fileSize} bytes`));
272-
});
273-
fileStream.on('data', chunk => {
274-
chunks.push(chunk);
275-
});
276-
fileStream.on('close', () => {
277-
if (fileStream.truncated) {
278-
reject(new Error(`File size limit exceeded: ${formDataLimits?.fileSize} bytes`));
279-
}
280-
const file = new PonyfillFile(chunks, filename, { type: mimeType });
281-
this._formData!.set(name, file);
282-
});
283-
},
284-
);
285333
bb.on('filesLimit', () => {
286-
reject(new Error(`Files limit exceeded: ${formDataLimits?.files}`));
334+
complete(new Error(`Files limit exceeded: ${formDataLimits?.files}`));
287335
});
288336
bb.on('partsLimit', () => {
289-
reject(new Error(`Parts limit exceeded: ${formDataLimits?.parts}`));
337+
complete(new Error(`Parts limit exceeded: ${formDataLimits?.parts}`));
290338
});
291-
bb.on('close', () => {
292-
resolve(this._formData!);
293-
});
294-
bb.on('error', (err: any = 'An error occurred while parsing the form data') => {
295-
const errMessage = err.message || err.toString();
296-
// @ts-ignore - `cause` is in `TypeError`in node
297-
reject(new TypeError(errMessage, err.cause));
298-
});
299-
_body?.readable.pipe(bb);
339+
bb.on('end', complete);
340+
bb.on('finish', complete);
341+
bb.on('close', complete);
342+
bb.on('error', complete);
343+
344+
stream.pipe(bb);
300345
});
301346
}
302347

packages/node-fetch/tests/Body.spec.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ describe('Body', () => {
6666
expect(result).toBe('hello world');
6767
});
6868

69-
it('throws a TypeError if the body is unable to parse as FormData', async () => {
69+
it('throws an error if the body is unable to parse as FormData', async () => {
7070
const formStr =
7171
'--Boundary_with_capital_letters\r\n' +
7272
'Content-Type: application/json\r\n' +
@@ -80,11 +80,13 @@ describe('Body', () => {
8080
type: 'multipart/form-data; boundary=Boundary_with_capital_letters',
8181
}),
8282
);
83+
84+
let err;
8385
try {
8486
await body.formData();
85-
expect(true).toBe(false);
8687
} catch (e) {
87-
expect(e).toBeInstanceOf(TypeError);
88+
err = e;
8889
}
90+
expect(String(err)).toContain('Unexpected end of multipart data');
8991
});
9092
});

packages/node-fetch/tests/FormData.spec.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,9 @@ describe('Form Data', () => {
134134
fileSize: 1,
135135
},
136136
});
137-
try {
138-
await requestWillParse.formData();
139-
expect(true).toBe(false);
140-
} catch (error: any) {
141-
expect(error.message).toBe('File size limit exceeded: 1 bytes');
142-
}
137+
const err = await requestWillParse.formData().catch(e => e);
138+
expect(err).toBeInstanceOf(Error);
139+
expect(err).toHaveProperty('message', 'File size limit exceeded: 1 bytes');
143140
});
144141
it('support native Blob', async () => {
145142
const formData = new PonyfillFormData();

packages/node-fetch/tests/non-http-fetch.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ describe('File protocol', () => {
2020
expect(response.status).toBe(404);
2121
});
2222
it('returns 403 if file is not accessible', async () => {
23+
// this can yield a false negative if the test is run with sufficient privileges
24+
// TODO: consistent behavior across platforms
2325
const response = await fetchPonyfill(pathToFileURL('/root/private_data.txt'));
2426
expect(response.status).toBe(403);
2527
});

packages/server/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"@types/node": "22.14.1",
4848
"express": "5.1.0",
4949
"fastify": "5.3.0",
50+
"form-data": "^4.0.2",
5051
"koa": "^2.15.3",
5152
"react": "19.1.0",
5253
"react-dom": "19.1.0"

0 commit comments

Comments
 (0)