buffer: fix end parameter bugs in indexOf/lastIndexOf by ronag · Pull Request #62711 · nodejs/node

ronag · 2026-04-12T20:18:11Z

Fix FastIndexOfNumber parameter order mismatch (end_i64 and is_forward were swapped vs the JS call site and slow path)
Clamp negative end values to 0 to prevent size_t overflow in IndexOfString, IndexOfBuffer, and IndexOfNumberImpl
Clamp empty needle result to search_end

codecov · 2026-04-12T21:32:14Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.62%. Comparing base (4563cb3) to head (1d34d6b).
⚠️ Report is 148 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62711      +/-   ##
==========================================
+ Coverage   88.93%   89.62%   +0.68%     
==========================================
  Files         699      706       +7     
  Lines      216379   219221    +2842     
  Branches    41281    42003     +722     
==========================================
+ Hits       192434   196471    +4037     
+ Misses      15976    14674    -1302     
- Partials     7969     8076     +107

Files with missing lines	Coverage Δ
src/node_buffer.cc	`68.66% <100.00%> (+0.63%)`	⬆️

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

- Fix FastIndexOfNumber parameter order mismatch (end_i64 and is_forward were swapped vs the JS call site and slow path) - Clamp negative end values to 0 to prevent size_t overflow in IndexOfString, IndexOfBuffer, and IndexOfNumberImpl - Clamp empty needle result to search_end Signed-off-by: Robert Nagy <ronagy@icloud.com> Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

ronag · 2026-04-24T14:18:26Z

This is an important bug fix which is unable to get reviews. @nodejs/tsc

panva · 2026-04-24T14:21:38Z

I don't think this has a place on the TSC agenda. I'm happy to review but at this point this can't land anyway on the account of having formatting issues.

Co-authored-by: Robert Nagy <ronagy@icloud.com>

ronag · 2026-04-24T14:26:25Z

I don't think this has a place on the TSC agenda. I'm happy to review but at this point this can't land anyway on the account of having formatting issues.

I guess it's gray zone. Being unable to land an important (security related) fix falls IMHO under the TSC.

Formatting issues resolve. PTAL

panva · 2026-04-24T14:38:51Z

I guess it's gray zone. Being unable to land an important (security related) fix falls IMHO under the TSC.

But there's no ~~rush~~ immediate urgency since it's not due to land on any release given the labels on #62390 yet, or am I reading this wrong. Quite possible.

panva

LGTM

github-actions · 2026-04-24T15:27:43Z

Failed to start CI

[SyntaxError: Unexpected token '<', "https://github.com/nodejs/node/actions/runs/24897465125

nodejs-github-bot · 2026-04-24T15:31:44Z

CI: https://ci.nodejs.org/job/node-test-pull-request/72901/

nodejs-github-bot · 2026-04-24T17:16:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/72904/ ✅

nodejs-github-bot · 2026-04-24T18:54:27Z

Commit Queue failed

- Loading data for nodejs/node/pull/62711 ✔ Done loading data for nodejs/node/pull/62711 ----------------------------------- PR info ------------------------------------ Title buffer: fix end parameter bugs in indexOf/lastIndexOf (#62711) Author Robert Nagy <ronagy@icloud.com> (@ronag) Branch ronag:buffer-index-of-end2 -> nodejs:main Labels buffer, c++, author ready, needs-ci Commits 2 - buffer: fix end parameter bugs in indexOf/lastIndexOf - Apply suggestions from code review Committers 2 - Robert Nagy <ronagy@icloud.com> - GitHub <noreply@github.com> PR-URL: https://github.com/nodejs/node/pull/62711 Fixes: https://github.com/nodejs/node/issues/62873 Reviewed-By: Filip Skokan <panva.ip@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/62711 Fixes: https://github.com/nodejs/node/issues/62873 Reviewed-By: Filip Skokan <panva.ip@gmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 12 Apr 2026 20:18:11 GMT ✔ Approvals: 1 ✔ - Filip Skokan (@panva) (TSC): https://github.com/nodejs/node/pull/62711#pullrequestreview-4171398766 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2026-04-24T17:16:45Z: https://ci.nodejs.org/job/node-test-pull-request/72904/ - Querying data for job/node-test-pull-request/72904/ ✔ Build data downloaded ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 62711 From https://github.com/nodejs/node * branch refs/pull/62711/merge -> FETCH_HEAD ✔ Fetched commits as 9c4ca0a12d1c..1d34d6bdb97c -------------------------------------------------------------------------------- [main cd8d9d00c1] buffer: fix end parameter bugs in indexOf/lastIndexOf Author: Robert Nagy <ronagy@icloud.com> Date: Sun Apr 12 22:16:34 2026 +0200 2 files changed, 52 insertions(+), 11 deletions(-) [main 1d3743e568] Apply suggestions from code review Author: Robert Nagy <ronagy@icloud.com> Date: Fri Apr 24 16:25:08 2026 +0200 1 file changed, 2 insertions(+), 4 deletions(-) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. (node:339) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated. (Use `node --trace-deprecation ...` to show where the warning was created) Rebasing (2/4) Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- buffer: fix end parameter bugs in indexOf/lastIndexOf

Fix FastIndexOfNumber parameter order mismatch (end_i64 and
is_forward were swapped vs the JS call site and slow path)
Clamp negative end values to 0 to prevent size_t overflow in
IndexOfString, IndexOfBuffer, and IndexOfNumberImpl
Clamp empty needle result to search_end

Signed-off-by: Robert Nagy <ronagy@icloud.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR-URL: #62711
Fixes: #62873
Reviewed-By: Filip Skokan <panva.ip@gmail.com>

[detached HEAD 524e56e6a2] buffer: fix end parameter bugs in indexOf/lastIndexOf
Author: Robert Nagy <ronagy@icloud.com>
Date: Sun Apr 12 22:16:34 2026 +0200
2 files changed, 52 insertions(+), 11 deletions(-)
Rebasing (3/4)
Rebasing (4/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
Apply suggestions from code review

Co-authored-by: Robert Nagy <ronagy@icloud.com>
PR-URL: #62711
Fixes: #62873
Reviewed-By: Filip Skokan <panva.ip@gmail.com>

[detached HEAD c84de63c63] Apply suggestions from code review
Author: Robert Nagy <ronagy@icloud.com>
Date: Fri Apr 24 16:25:08 2026 +0200
1 file changed, 2 insertions(+), 4 deletions(-)
Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/24906439861

nodejs-github-bot · 2026-04-24T19:41:25Z

Landed in 8176c2c

- Fix FastIndexOfNumber parameter order mismatch (end_i64 and is_forward were swapped vs the JS call site and slow path) - Clamp negative end values to 0 to prevent size_t overflow in IndexOfString, IndexOfBuffer, and IndexOfNumberImpl - Clamp empty needle result to search_end Signed-off-by: Robert Nagy <ronagy@icloud.com> Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> PR-URL: #62711 Fixes: #62873 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

- Fix FastIndexOfNumber parameter order mismatch (end_i64 and is_forward were swapped vs the JS call site and slow path) - Clamp negative end values to 0 to prevent size_t overflow in IndexOfString, IndexOfBuffer, and IndexOfNumberImpl - Clamp empty needle result to search_end Signed-off-by: Robert Nagy <ronagy@icloud.com> Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> PR-URL: nodejs#62711 Fixes: nodejs#62873 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Signed-off-by: Stewart X Addison <sxa@ibm.com>

- Fix FastIndexOfNumber parameter order mismatch (end_i64 and is_forward were swapped vs the JS call site and slow path) - Clamp negative end values to 0 to prevent size_t overflow in IndexOfString, IndexOfBuffer, and IndexOfNumberImpl - Clamp empty needle result to search_end Signed-off-by: Robert Nagy <ronagy@icloud.com> Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> PR-URL: #62711 Fixes: #62873 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

The frame parser called indexOf(value, 'hex', startingIndex). Node up to 24.18.1 treated the string byteOffset as the encoding and ignored the third argument, so it worked by accident. Node 24.20.0 (nodejs/node#62711) now honours the argument positions, and the suffix search returned -1 for every frame: the parser waited forever for more data, no inbound packet was dispatched, and every ping timed out while the device answered normally.

ronag requested review from jasnell and mcollina April 12, 2026 20:18

ronag added the buffer Issues and PRs related to the buffer subsystem. label Apr 12, 2026

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 Apr 12, 2026

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 12, 2026

ronag force-pushed the buffer-index-of-end2 branch 2 times, most recently from 81afd27 to 927c25a Compare April 12, 2026 20:28

ronag mentioned this pull request Apr 21, 2026

buffer: add end parameter #62390

Merged

ronag requested a review from ChALkeR April 21, 2026 15:24

ronag force-pushed the buffer-index-of-end2 branch from 927c25a to 7d46b35 Compare April 21, 2026 23:47

ronag force-pushed the buffer-index-of-end2 branch from 7d46b35 to e4bbd84 Compare April 22, 2026 11:05

ronag requested review from ShogunPanda, anonrig, bnoordhuis and tniessen April 23, 2026 05:07

ronag added the tsc-agenda Issues and PRs to discuss during Technical Steering Committee meetings. label Apr 24, 2026

panva removed the tsc-agenda Issues and PRs to discuss during Technical Steering Committee meetings. label Apr 24, 2026

ronag commented Apr 24, 2026

View reviewed changes

Comment thread src/node_buffer.cc Outdated

ronag commented Apr 24, 2026

View reviewed changes

Comment thread src/node_buffer.cc Outdated

Apply suggestions from code review

1d34d6b

Co-authored-by: Robert Nagy <ronagy@icloud.com>

panva approved these changes Apr 24, 2026

View reviewed changes

panva added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Apr 24, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 24, 2026

github-actions Bot added the request-ci-failed Starting CI with the request-ci label failed and requires manual intervention. label Apr 24, 2026

panva removed the request-ci-failed Starting CI with the request-ci label failed and requires manual intervention. label Apr 24, 2026

ronag added the commit-queue PRs queued for automated landing through the Commit Queue. label Apr 24, 2026

nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Apr 24, 2026

anonrig approved these changes Apr 24, 2026

View reviewed changes

panva added commit-queue-squash PRs the Commit Queue should land as one squashed commit. commit-queue PRs queued for automated landing through the Commit Queue. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Apr 24, 2026

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Apr 24, 2026

nodejs-github-bot merged commit 8176c2c into nodejs:main Apr 24, 2026
79 checks passed

aduh95 mentioned this pull request May 5, 2026

2026-05-07, Version 26.1.0 #63137

Merged

github-actions Bot mentioned this pull request Aug 21, 2026

2026-08-26, Version 24.20.0 'Krypton' (LTS) #65461

Merged

cleishm mentioned this pull request Sep 3, 2026

fix: pass Buffer.indexOf arguments in the right order (all devices time out on Node 24.20+) ayushsharma82/homebridge-tuya-local#28

Open

Uh oh!

Uh oh!

Conversation

ronag commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ronag commented Apr 24, 2026

Uh oh!

panva commented Apr 24, 2026

Uh oh!

Uh oh!

Uh oh!

ronag commented Apr 24, 2026

Uh oh!

panva commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

panva left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

nodejs-github-bot commented Apr 24, 2026

Uh oh!

nodejs-github-bot commented Apr 24, 2026 • edited by ronag Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 24, 2026

Signed-off-by: Robert Nagy <ronagy@icloud.com> Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> PR-URL: #62711 Fixes: #62873 Reviewed-By: Filip Skokan <panva.ip@gmail.com>

Co-authored-by: Robert Nagy <ronagy@icloud.com> PR-URL: #62711 Fixes: #62873 Reviewed-By: Filip Skokan <panva.ip@gmail.com>

[detached HEAD c84de63c63] Apply suggestions from code review Author: Robert Nagy <ronagy@icloud.com> Date: Fri Apr 24 16:25:08 2026 +0200 1 file changed, 2 insertions(+), 4 deletions(-) Successfully rebased and updated refs/heads/main.

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ronag commented Apr 12, 2026 •

edited

Loading

codecov Bot commented Apr 12, 2026 •

edited

Loading

panva commented Apr 24, 2026 •

edited

Loading

nodejs-github-bot commented Apr 24, 2026 •

edited by ronag

Loading

Signed-off-by: Robert Nagy <ronagy@icloud.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR-URL: #62711
Fixes: #62873
Reviewed-By: Filip Skokan <panva.ip@gmail.com>

Co-authored-by: Robert Nagy <ronagy@icloud.com>
PR-URL: #62711
Fixes: #62873
Reviewed-By: Filip Skokan <panva.ip@gmail.com>

[detached HEAD c84de63c63] Apply suggestions from code review
Author: Robert Nagy <ronagy@icloud.com>
Date: Fri Apr 24 16:25:08 2026 +0200
1 file changed, 2 insertions(+), 4 deletions(-)
Successfully rebased and updated refs/heads/main.