Skip to content

fix: apply filter, allowEmptyFiles and minFileSize to octet-stream uploads - #1126

Open
spokodev wants to merge 2 commits into
node-formidable:masterfrom
spokodev:fix/octetstream-file-checks
Open

spokodev wants to merge 2 commits into
node-formidable:masterfrom
spokodev:fix/octetstream-file-checks

Conversation

@spokodev

@spokodev spokodev commented Nov 12, 2026 •

Copy link
Copy Markdown

The octet-stream upload path skips three of the checks that multipart uploads get in _handlePart(): filter, allowEmptyFiles and minFileSize. This is the follow-up to #1113, which adds maxFileSize and maxTotalFileSize to the same path.

Steps to reproduce

Measured on master (9ee1247), sending the same body as application/octet-stream and as one multipart file:

Options Body octet-stream multipart
filter: () => false 1000 bytes filter not called, file saved filter called, file skipped
minFileSize: 1000 10 bytes file saved error 1008 (smallerThanMinFileSize)
defaults (allowEmptyFiles: false) empty, Transfer-Encoding: chunked empty file saved error 1010 (noEmptyFiles)

With Content-Length: 0 the octet-stream plugin is never reached, because the request gets the DummyParser, so an empty body returns no file and no error. That path is unchanged.

Fix

In src/plugins/octetstream.js:

  • options.filter is called with { name: "file", originalFilename, mimetype } before the file is created. name is "file" because that is the key the octet-stream file is returned under. When it returns false, the body is read and discarded, and the parse ends with no file and no error, as the README describes for filter.
  • When the body ends, allowEmptyFiles and minFileSize are checked against bytesReceived, since the octet-stream body is the file, with the same error codes, messages and order as _handlePart(). The checks run before this.ended is set, because _error() ignores errors after that. On error the partial file is removed by the shared _error() cleanup.

With default options any non-empty body behaves as before. The only change with defaults is an empty chunked body, which now fails with noEmptyFiles, like an empty multipart file.

Tests

test/integration/octet-stream-file-checks.test.js:

  • filter returning false: no error, no files, and filter called once with the expected argument
  • minFileSize: 1000 with 10 bytes: error 1008 and no files
  • an empty chunked body with default options: error 1010 and no files
  • an empty chunked body with allowEmptyFiles: true, minFileSize: 0: one empty file, as with multipart

The first three fail on master. Removing any of the new checks fails at least one of them.

Suite status: 98 passed / 3 skipped across 16 jest suites, and 11/11 node tests. The new tests use their own ports. This PR and #1113 merge cleanly in either order.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge.

Summary

The PR applies filter, allowEmptyFiles, and minFileSize to octet-stream uploads.

  • Octet-stream uploads now reject empty and undersized files by the configured rules.
  • The octet-stream path now skips files rejected by the filter.

Reviews (2) · Last reviewed commit: "test: remove uploaded files after octet-..." · Reviewed by Greptile

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant