Skip to content

buffer: fix negative index for large buffers - #66325

Open
Dansatch wants to merge 2 commits into
nodejs:mainfrom
Dansatch:fix-buffer-indexof-2gib
Open

Dansatch wants to merge 2 commits into
nodejs:mainfrom
Dansatch:fix-buffer-indexof-2gib

Conversation

@Dansatch

Copy link
Copy Markdown

Fixes #66294.

Buffer.prototype.indexOf() returned a negative position when a match occurred beyond 2 GiB because the result was converted to a signed 32-bit integer.

This change returns the position as a JavaScript number and adds regression tests for number, Buffer, and string searches.

@nodejs-github-bot nodejs-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 Sep 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

@Dansatch
Dansatch force-pushed the fix-buffer-indexof-2gib branch from d1a0f65 to 04a1fd7 Compare September 26, 2026 18:00
@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.36%. Comparing base (c0681e5) to head (8861a47).
⚠️ Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
src/node_buffer.cc 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66325      +/-   ##
==========================================
- Coverage   90.37%   90.36%   -0.02%     
==========================================
  Files         790      792       +2     
  Lines      274514   275324     +810     
  Branches    52572    52768     +196     
==========================================
+ Hits       248102   248785     +683     
- Misses      16889    16964      +75     
- Partials     9523     9575      +52     
Files with missing lines Coverage Δ
src/node_buffer.cc 70.41% <85.71%> (ø)

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

@panva panva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use int64_t instead of double for the casts and return types. The result is an integer offset or -1, and int64_t fixes the overflow while still exposing a JavaScript Number.

double also disables this Fast API signature on PPC64 and s390x. int64_t keeps it eligible.

Comment thread test/pummel/test-buffer-indexof-large.js
Comment on lines +22 to +24
assert.strictEqual(buffer.indexOf(0x0a, match), match);
assert.strictEqual(buffer.indexOf(Buffer.from([0x0a]), match), match);
assert.strictEqual(buffer.indexOf('\n', match), match);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
assert.strictEqual(buffer.indexOf(0x0a, match), match);
assert.strictEqual(buffer.indexOf(Buffer.from([0x0a]), match), match);
assert.strictEqual(buffer.indexOf('\n', match), match);
assert.strictEqual(buffer.indexOf(0x0a, -1), match);
assert.strictEqual(buffer.indexOf(Buffer.from([0x0a]), -1), match);
assert.strictEqual(buffer.indexOf('\n', -1), match);
assert.strictEqual(buffer.lastIndexOf(0x0a, -1), match);
assert.strictEqual(buffer.lastIndexOf(Buffer.from([0x0a]), -1), match);
assert.strictEqual(buffer.lastIndexOf('\n', -1), match);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I appreciate your review.
I’ve switched the return types and casts to int64_t, moved the test to test/pummel and added the suggested indexOf() and lastIndexOf() assertions.

Fix incorrect negative index results for large buffers.
Add tests for number, Buffer and string searches beyond 2 GiB.

Fixes: nodejs#66294

Signed-off-by: dansatch <dansatch98@gmail.com>
@Dansatch
Dansatch force-pushed the fix-buffer-indexof-2gib branch from 04a1fd7 to 7580065 Compare September 27, 2026 03:58
Signed-off-by: dansatch <dansatch98@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buffer.prototype.indexOf returns a negative (int32-wrapped) position for matches beyond 2 GiB

3 participants