[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; both numbered findings are fixed and no new blocking issue was found.

Summary

The PR adds maxFileSize and maxTotalFileSize checks before octet-stream chunks are written.

• Octet-stream uploads now stop when a file-size cap is exceeded.

_{Reviews (5) · Last reviewed commit: "test: fail on a request error raised bef..." · Reviewed by Greptile}

[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.