Skip to content

Fix #55422: Buffer.concat and Buffer.copy silently produce invalid r - #61914

Closed
danielalanbates wants to merge 1 commit into
nodejs:mainfrom
danielalanbates:fix/issue-55422
Closed

Fix #55422: Buffer.concat and Buffer.copy silently produce invalid r#61914
danielalanbates wants to merge 1 commit into
nodejs:mainfrom
danielalanbates:fix/issue-55422

Conversation

@danielalanbates

@danielalanbatesdanielalanbates commented Feb 21, 2026

Copy link
Copy Markdown

Fixes: #55422

Summary

This PR fixes: Buffer.concat and Buffer.copy silently produce invalid results when the operation involves indices equal or greater than 2^32

Changes

src/node_buffer.cc | 24 ++++++++++++------
test/parallel/test-buffer-copy-large.js | 44 +++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 8 deletions(-)

Testing

Please review the changes carefully. The fix was verified against the existing test suite.


This PR was created with the assistance of Claude Sonnet 4.6 by Anthropic | effort: high. Happy to make any adjustments!

@nodejs-github-botnodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 21, 2026
@codecov

codecovBot commented Feb 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.65%. Comparing base (f632f3f) to head (6ac2ab5).
⚠️ Report is 188 commits behind head on main.

Files with missing linesPatch %Lines
src/node_buffer.cc66.66%2 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #61914 +/- ##
==========================================
- Coverage 91.70% 89.65% -2.06% 
==========================================
Files 337 676 +339 Lines 140104 206549 +66445 Branches 21758 39544 +17786 ==========================================
+ Hits 128484 185172 +56688 - Misses 11398 13480 +2082 - Partials 222 7897 +7675 
Files with missing linesCoverage Δ
src/node_buffer.cc67.90% <66.66%> (ø)

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

Comment threadsrc/node_buffer.cc
@danielalanbates

Copy link
Copy Markdown
Author

Thanks for the feedback, @Renegade334! I've pushed an update addressing your review comments. Please take another look when you get a chance.

@Renegade334

Copy link
Copy Markdown
Member

Mostly good, modulo:

@Renegade334

Copy link
Copy Markdown
Member

@danielalanbates are you able to address the above comments?

Buffer.copy silently produced incorrect results when indices
involved were >= 2^32 due to 32-bit integer overflow in the
native SlowCopy and FastCopy functions in node_buffer.cc.
Change the parameter types and return type of SlowCopy and
FastCopy from uint32_t to uint64_t so that offsets beyond
4 GiB are handled correctly.
Fixes: nodejs#55422
@Renegade334Renegade334 added request-ci Add this label to start a Jenkins CI on a PR. dont-land-on-v20.x lts-watch-v22.x PRs that may need to be released in v22.x lts-watch-v24.x PRs that may need to be released in v24.x labels Mar 9, 2026
@Renegade334
Renegade334 requested a review from ronagMarch 9, 2026 03:50
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 9, 2026
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@Renegade334Renegade334 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 9, 2026
@danielalanbates

Copy link
Copy Markdown
Author

Thank you for your time reviewing this. We're withdrawing this PR. Apologies for any inconvenience.

@aduh95aduh95 removed the lts-watch-v24.x PRs that may need to be released in v24.x label Apr 10, 2026
@marco-ippolitomarco-ippolito removed the lts-watch-v22.x PRs that may need to be released in v22.x label May 11, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.bufferIssues and PRs related to the buffer subsystem.c++Issues and PRs that require attention from people who are familiar with C++.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buffer.concat and Buffer.copy silently produce invalid results when the operation involves indices equal or greater than 2^32

5 participants

@danielalanbates@Renegade334@nodejs-github-bot@aduh95@marco-ippolito