Skip to content

fs: fix crash on negative zero file descriptor - #65888

Open
christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:fs-negative-zero-fd
Open

fs: fix crash on negative zero file descriptor#65888
christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:fs-negative-zero-fd

Conversation

@christianaurichzm

Copy link
Copy Markdown
Contributor

fs.writeFileSync(-0, '') and fs.appendFileSync(-0, '') currently abort in the utf8 fast path. I also found the same issue with fs.readFileSync(-0, 'utf8').

The JS side treats -0 as an int32 file descriptor, but V8 does not report -0 as an Int32 in the binding. The binding then takes it for a path and hits the null check.

This normalizes -0 to 0 before entering the binding, matching the existing fs behavior for negative-zero file descriptors.

Tests were added to test/parallel/test-fs-negative-zero.js.

Fixes: #65886

`isInt32()` accepts -0 because `-0 === (-0 | 0)`, but V8 does not
represent -0 as an Int32 value, so `Value::IsInt32()` rejects it. The
utf8 fast paths of `readFileSync()` and `writeFileSync()` hand the value
straight to the binding, which then took it for a path and aborted on
the null check.

Coerce -0 to 0 before the call, matching `getValidatedFd()` and the rest
of fs, where -0 is a valid way to name file descriptor 0.

Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run. labels Sep 7, 2026
@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.18%. Comparing base (b113d09) to head (c4cc7a9).
⚠️ Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65888      +/-   ##
==========================================
+ Coverage   90.16%   90.18%   +0.02%     
==========================================
  Files         771      771              
  Lines      265094   265102       +8     
  Branches    50355    50364       +9     
==========================================
+ Hits       239027   239091      +64     
+ Misses      17011    16963      -48     
+ Partials     9056     9048       -8     
Files with missing lines Coverage Δ
lib/fs.js 97.31% <100.00%> (+<0.01%) ⬆️

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@meixg meixg added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 8, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 8, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@hamidrezaghavami hamidrezaghavami left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

By using process.platform !== 'win32', we guarantee it won't throw a "not defined" error, and the tests will continue running perfectly on all the Linux and Mac machines where they already passed!

Comment thread test/parallel/test-fs-negative-zero.js
Comment thread test/parallel/test-fs-negative-zero.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node:fs: calling writeFileSync/appendFileSync with negative 0 causes an assertion failure

4 participants