Skip to content

Commit cb18a94

Browse files
ronagclaude
andcommitted
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> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4563cb3 commit cb18a94

File tree

2 files changed

+53
-8
lines changed

2 files changed

+53
-8
lines changed

src/node_buffer.cc

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,8 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
984984

985985
// search_end is the exclusive upper bound of the search range.
986986
size_t search_end = static_cast<size_t>(
987-
std::min(end_i64, static_cast<int64_t>(haystack_length)));
987+
std::min(std::max(end_i64, int64_t{0}),
988+
static_cast<int64_t>(haystack_length)));
988989
if (enc == UCS2) search_end &= ~static_cast<size_t>(1);
989990

990991
int64_t opt_offset = IndexOfOffset(haystack_length,
@@ -993,8 +994,11 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
993994
is_forward);
994995

995996
if (needle_length == 0) {
996-
// Match String#indexOf() and String#lastIndexOf() behavior.
997-
args.GetReturnValue().Set(static_cast<double>(opt_offset));
997+
// Match String#indexOf() and String#lastIndexOf() behavior,
998+
// but clamp to search_end.
999+
int64_t clamped = std::min(opt_offset,
1000+
static_cast<int64_t>(search_end));
1001+
args.GetReturnValue().Set(static_cast<double>(clamped));
9981002
return;
9991003
}
10001004

@@ -1109,7 +1113,8 @@ void IndexOfBuffer(const FunctionCallbackInfo<Value>& args) {
11091113

11101114
// search_end is the exclusive upper bound of the search range.
11111115
size_t search_end = static_cast<size_t>(
1112-
std::min(end_i64, static_cast<int64_t>(haystack_length)));
1116+
std::min(std::max(end_i64, int64_t{0}),
1117+
static_cast<int64_t>(haystack_length)));
11131118
if (enc == UCS2) search_end &= ~static_cast<size_t>(1);
11141119

11151120
int64_t opt_offset = IndexOfOffset(haystack_length,
@@ -1118,8 +1123,11 @@ void IndexOfBuffer(const FunctionCallbackInfo<Value>& args) {
11181123
is_forward);
11191124

11201125
if (needle_length == 0) {
1121-
// Match String#indexOf() and String#lastIndexOf() behavior.
1122-
args.GetReturnValue().Set(static_cast<double>(opt_offset));
1126+
// Match String#indexOf() and String#lastIndexOf() behavior,
1127+
// but clamp to search_end.
1128+
int64_t clamped = std::min(opt_offset,
1129+
static_cast<int64_t>(search_end));
1130+
args.GetReturnValue().Set(static_cast<double>(clamped));
11231131
return;
11241132
}
11251133

@@ -1185,7 +1193,8 @@ int32_t IndexOfNumberImpl(Local<Value> buffer_obj,
11851193
size_t offset = static_cast<size_t>(opt_offset);
11861194
// search_end is the exclusive upper bound of the search range.
11871195
size_t search_end = static_cast<size_t>(
1188-
std::min(end_i64, static_cast<int64_t>(buffer_length)));
1196+
std::min(std::max(end_i64, int64_t{0}),
1197+
static_cast<int64_t>(buffer_length)));
11891198

11901199
const void* ptr;
11911200
if (is_forward) {
@@ -1222,8 +1231,8 @@ int32_t FastIndexOfNumber(Local<Value>,
12221231
Local<Value> buffer_obj,
12231232
uint32_t needle,
12241233
int64_t offset_i64,
1225-
int64_t end_i64,
12261234
bool is_forward,
1235+
int64_t end_i64,
12271236
// NOLINTNEXTLINE(runtime/references)
12281237
FastApiCallbackOptions& options) {
12291238
HandleScope scope(options.isolate);

test/parallel/test-buffer-indexof.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -691,3 +691,39 @@ assert.strictEqual(reallyLong.lastIndexOf(pattern), 0);
691691

692692
assert.strictEqual(buf.includes('c'), true);
693693
}
694+
695+
// Regression tests for end parameter edge cases.
696+
{
697+
const buf = Buffer.from('abcabc');
698+
699+
// Negative end should be treated as 0 (no match possible).
700+
assert.strictEqual(buf.indexOf('a', 0, -1), -1);
701+
assert.strictEqual(buf.indexOf('a', 0, -100), -1);
702+
assert.strictEqual(buf.indexOf(0x61, 0, -1), -1);
703+
assert.strictEqual(buf.lastIndexOf('a', 5, -1), -1);
704+
assert.strictEqual(buf.lastIndexOf(0x61, 5, -1), -1);
705+
assert.strictEqual(buf.includes('a', 0, -1), false);
706+
assert.strictEqual(buf.indexOf(Buffer.from('a'), 0, -1), -1);
707+
assert.strictEqual(buf.lastIndexOf(Buffer.from('a'), 5, -1), -1);
708+
709+
// end = 0 means empty search range.
710+
assert.strictEqual(buf.indexOf('a', 0, 0), -1);
711+
assert.strictEqual(buf.indexOf(0x61, 0, 0), -1);
712+
assert.strictEqual(buf.lastIndexOf('a', 5, 0), -1);
713+
assert.strictEqual(buf.lastIndexOf(0x61, 5, 0), -1);
714+
715+
// end greater than buffer length should be clamped.
716+
assert.strictEqual(buf.indexOf('c', 0, 100), 2);
717+
assert.strictEqual(buf.indexOf(0x63, 0, 100), 2);
718+
assert.strictEqual(buf.lastIndexOf('c', 5, 100), 5);
719+
assert.strictEqual(buf.lastIndexOf(0x63, 5, 100), 5);
720+
assert.strictEqual(buf.indexOf(Buffer.from('c'), 0, 100), 2);
721+
722+
// Empty needle with end parameter should clamp to search_end.
723+
assert.strictEqual(buf.indexOf('', 0, 3), 0);
724+
assert.strictEqual(buf.indexOf('', 5, 3), 3);
725+
assert.strictEqual(buf.indexOf(Buffer.from(''), 5, 3), 3);
726+
assert.strictEqual(buf.indexOf('', 0, 0), 0);
727+
assert.strictEqual(buf.lastIndexOf('', 5, 3), 3);
728+
assert.strictEqual(buf.lastIndexOf(Buffer.from(''), 5, 3), 3);
729+
}

0 commit comments

Comments
 (0)