Skip to content

fix: prevent file handle leak when maxFiles is exceeded #1068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guoyangzhen wants to merge 3 commits into
base: master
Choose a base branch
from
+6 −0
Open

fix: prevent file handle leak when maxFiles is exceeded #1068

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
178784c
fix: prevent file handle leak when maxFiles is exceeded
guoyangzhen Mar 16, 2026
a23bbfd
Merge branch 'master' into master
tunnckoCore Apr 16, 2026
71780c6
chore: retrigger kodiak for #1068
tunnckoCore Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/Formidable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,12 @@ class IncomingForm extends EventEmitter {
});
this.emit("fileBegin", part.name, file);

// Check for error after fileBegin (e.g., maxFiles exceeded) to avoid leaking file handles
if (this.error) {
this._flushing -= 1;
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the multipart path, but the same maxFiles/fileBegin sequencing exists in the octet-stream plugin (src/plugins/octetstream.js): it emits fileBegin before file.open() and openedFiles.push(file). If maxFiles is exceeded there, _error() will run before the file is tracked and the handle can still leak. Consider applying the same this.error short-circuit (and matching _flushing handling) in the octet-stream code path as well to make the fix comprehensive.

Suggested change
this._flushing -= 1;

Copilot uses AI. Check for mistakes.
return;
}
Comment on lines +415 to +419
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are existing Jest tests asserting maxFiles emits an error, but they don’t cover the new behavior that prevents file.open()/handle creation after fileBegin triggers _error(). Please add a test that reproduces the leak scenario (e.g., stub _newFile() to return a file with an open spy and ensure open is not called for the part that exceeds maxFiles, and/or assert no streams remain open).

Copilot uses AI. Check for mistakes.

file.open();
this.openedFiles.push(file);

Expand Down
Loading