fs: fix crash on negative zero file descriptor - #65888
Merged
nodejs-github-bot merged 1 commit intoSep 26, 2026
Merged
Conversation
`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
added
fs
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
meixg
approved these changes
Sep 8, 2026
meixg
added
the
request-ci
github-actions
Bot
removed
the
request-ci
This comment has been minimized.
This comment has been minimized.
hamidrezaghavami
left a comment
hamidrezaghavami
left a comment
Contributor
There was a problem hiding this comment.
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!
Contributor
Author
|
@meixg, could you please rerun CI here when you have a chance? The previous failures appear to be unrelated flakes. Thanks! |
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
Looks like the rerun also ran into an unrelated CI issue. This run is listed in the reliability report as Would someone mind giving CI another try? Thanks! |
panva
added
the
author ready
Collaborator
panva
added
the
resume-ci
github-actions
Bot
removed
the
resume-ci
Collaborator
aduh95
added
the
resume-ci
panva
removed
the
resume-ci
Collaborator
Collaborator
Collaborator
Collaborator
Collaborator
panva
added
the
commit-queue
Collaborator
|
Landed in 1e17275 |
nodejs-github-bot
removed
the
commit-queue
aduh95
pushed a commit
that referenced
this pull request
Sep 27, 2026
`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> PR-URL: #65888 Fixes: #65886 Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
HoonDongKang
pushed a commit
to HoonDongKang/node
that referenced
this pull request
Sep 28, 2026
`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> PR-URL: nodejs#65888 Fixes: nodejs#65886 Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
Sep 28, 2026
`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> PR-URL: #65888 Fixes: #65886 Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fs.writeFileSync(-0, '')andfs.appendFileSync(-0, '')currently abort in the utf8 fast path. I also found the same issue withfs.readFileSync(-0, 'utf8').The JS side treats
-0as an int32 file descriptor, but V8 does not report-0as anInt32in the binding. The binding then takes it for a path and hits the null check.This normalizes
-0to0before 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