Sitelet https://github.com/nodejs/node/pull/65521
Skip to content

fs: coerce FileHandle.read length like fs.read - #65521

Open
bun-unsafe wants to merge 1 commit into
nodejs:mainfrom
bun-unsafe:fs-coerce-filehandle-read-length
Open

fs: coerce FileHandle.read length like fs.read#65521
bun-unsafe wants to merge 1 commit into
nodejs:mainfrom
bun-unsafe:fs-coerce-filehandle-read-length

Conversation

@bun-unsafe

@bun-unsafe bun-unsafe commented Aug 24, 2026

Copy link
Copy Markdown

Summary

fs.promises.FileHandle.read() forwarded a non-number length (for example "5") into binding.read. node::fs::Read requires args[3]->IsInt32() and aborts the process on a debug/CHECK failure.

fs.read / fs.readSync already coerce with length |= 0 after defaulting. This applies the same step on the promises path so a bad length cannot reach C++.

Test plan

  • ./node test/parallel/test-fs-promises-file-handle-read.js
  • Existing FileHandle.read cases (empty length, options object, null/undefined length) still pass on CI
const fh = await fs.promises.open(file, 'r');
await fh.read(Buffer.alloc(20), 0, '5', 0); // must not abort; bytesRead === 5

Non-number length values reached node::fs::Read, which CHECKs
IsInt32() and aborts the process. Apply length |= 0 after defaulting,
matching fs.read and fs.readSync.

Signed-off-by: Xia Chao <236466140+bun-unsafe@users.noreply.github.com>
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Aug 24, 2026
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 24, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor
Failed to start CI
- Validating Jenkins credentials
✔  Jenkins credentials valid
- Starting PR CI job
✘  Failed to start PR CI: 400 Bad Request
https://github.com/nodejs/node/actions/runs/32773413890

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.15%. Comparing base (f509cf1) to head (dcefcbc).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #65521   +/-   ##
=======================================
  Coverage   90.14%   90.15%           
=======================================
  Files         751      751           
  Lines      253585   253586    +1     
  Branches    47772    47773    +1     
=======================================
+ Hits       228596   228620   +24     
+ Misses      16228    16204   -24     
- Partials     8761     8762    +1     
Files with missing lines Coverage Δ
lib/internal/fs/promises.js 92.65% <100.00%> (+<0.01%) ⬆️

... and 28 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.

@aduh95 aduh95 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Aug 25, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 25, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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 the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants