Skip to content

net: validate onread buffers before each read - #67407

Open
fatihguzeldev wants to merge 1 commit into
nodejs:mainfrom
fatihguzeldev:net-onread-buffer-lifetime
Open

fatihguzeldev wants to merge 1 commit into
nodejs:mainfrom
fatihguzeldev:net-onread-buffer-lifetime

Conversation

@fatihguzeldev

Copy link
Copy Markdown
Contributor

socket reads can modify an immutable arraybuffer created by transferring
a registered onread buffer, because native code keeps the old address.

fail the read with a socket error when the view is no longer writable.
buffer factories can still supply a replacement and continue reading.

fixes #67397

an onread buffer can be detached after registration while native code
still holds its address. validate the current view before each read
and report an error instead of writing into the former backing store.

Fixes: nodejs#67397
Signed-off-by: Fatih Güzel <fatihguzeldev@gmail.com>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Nov 11, 2026
@codecov

codecov Bot commented Nov 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.57143% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.69%. Comparing base (77e6379) to head (663b343).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/stream_base.cc 88.57% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67407      +/-   ##
==========================================
- Coverage   90.72%   90.69%   -0.04%     
==========================================
  Files         798      801       +3     
  Lines      280102   281589    +1487     
  Branches    53896    54254     +358     
==========================================
+ Hits       254135   255385    +1250     
- Misses      16252    16351      +99     
- Partials     9715     9853     +138     
Files with missing lines Coverage Δ
src/stream_base.h 60.71% <ø> (-1.36%) ⬇️
src/stream_base.cc 76.66% <88.57%> (+0.53%) ⬆️

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

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

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

net: onread can modify buffers made immutable after registration

2 participants