fix: enforce maxFileSize/maxTotalFileSize on octet-stream uploads

[spokodev]

The octet-stream upload path does not enforce the documented maxFileSize / maxTotalFileSize limits, unlike the multipart path.

Steps to reproduce

POST a 256KB body with Content-Type: application/octet-stream to a form configured with maxFileSize: 1024:

const form = formidable({ maxFileSize: 1024, maxTotalFileSize: 2048 });
form.parse(req, (err, fields, files) => {
  console.log(err, Object.keys(files).length);
});
// request.end(Buffer.alloc(256 * 1024, 0x42));

ACTUAL: err is undefined, one file is returned with size 262144, and the over-sized file is committed to disk via file.end().

EXPECTED: err.code === 1016 (biggerThanMaxFileSize), no file returned, and nothing left on disk. This matches how the multipart path already behaves.

Root cause

The octet-stream plugin's _parser.on("data", ...) handler in src/plugins/octetstream.js writes each chunk straight to the file with no size check, and it bypasses _handlePart() in src/Formidable.js where the multipart size caps are enforced. As a result a raw octet-stream body of any size is accepted regardless of the configured limits.

The README documents maxFileSize and maxTotalFileSize as limiting each file and the batch respectively, with defaults, and does not exempt octet-stream.

Fix

Accumulate the per-file and running total sizes before each write and abort via this._error(...) with the existing FormidableError codes biggerThanMaxFileSize / biggerThanTotalMaxFileSize when a cap is exceeded, mirroring _handlePart. In-limit uploads are unaffected. Because the octet-stream file is tracked in openedFiles, the shared _error cleanup calls file.destroy(), which unlinks the partial file, so no bytes remain on disk (the same cleanup the multipart path relies on).

Authority

CWE-770 (allocation without limits), plus the library's own documented contract and its multipart implementation, which enforces exactly these caps.

Tests

Added an integration case in test/integration/octet-stream.test.js: a 256KB octet-stream body with maxFileSize: 1024 must be rejected with code 1016 and return no files. Verified it fails on the current source (the over-sized upload is accepted with err null) and passes with the fix, with the tmp directory left empty afterwards.

Suite status: 92 passed / 3 skipped across 14 jest suites, and 11/11 node tests.

Confidence Score: 5/5

The PR appears safe to merge; the remaining findings concern import style and test-error diagnostics.

Findings

1.  Unexpected request errors disappear ▶
2.  The two separate import statements can be merged into a single combined import, which is the pattern used throughout the codebase e.g. import FormidableError, as errors from "./FormidableError.js" in Formidable.js . ▶

Summary

The PR adds per-file and total-size checks to octet-stream uploads and an integration test for the per-file limit. The latest change closes the test’s client request after rejection to avoid leaving the connection open.

_{Reviews (4) · Last reviewed commit: "test(octet-stream): close the rejected u..."}

[greptile-apps]

The two separate import statements can be merged into a single combined import, which is the pattern used throughout the codebase (e.g. import FormidableError, * as errors from "./FormidableError.js" in Formidable.js).

Suggested change

	import * as errors from "../FormidableError.js";
	import FormidableError from "../FormidableError.js";
	import FormidableError, * as errors from "../FormidableError.js";

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Unexpected request errors disappear The handler ignores every client request error, not just the expected reset after request.destroy(). If the request fails before reaching the server, the parse callback never runs, so the test times out and leaves the server open instead of reporting the connection error and cleaning up.

[spokodev]

The ci run was failing here, and it was my test's fault rather than the fix's.

The maxFileSize test left a TCP handle open: formidable aborts the parse mid-body, so the connection never completes on its own and server.close() only stops new ones. On CI that showed up as "A worker process has failed to exit gracefully", and the late callback surfaced as a failure inside the next test file, test/integration/store-files-option.test.js — which this PR does not touch.

It reproduces locally when the suite runs in a single process. With the test file in its previous form, jest --runInBand gives the CI result exactly — 1 failed / 14 passed of 15 suites, and 1 failed / 3 skipped / 94 passed of 98 — with the failing assertion's stack originating in octet-stream.test.js. With 1a43a61, which destroys the client request in the parse callback, the same in-band run is 15/15 and 95 passed.

--detectOpenHandles on that file goes from one TCPWRAP at its server.listen to none. Two pre-existing TCPWRAPs remain elsewhere in the suite, in keep-alive-error.test.js and connection-aborted.test.js; this PR touches neither. node --test is 11/11.

Shrinking the body from 256 KB to 4 KB did not fix it, so the leak is the unfinished connection rather than the unsent bytes.

Both workflow runs on this push concluded action_required with no jobs dispatched, so CI needs a maintainer to approve the run before it can confirm any of this here.