Skip to content

Commit 28ddac2

Browse files
addaleaxaddaleax
committed
buffer: fix indexOf for empty searches
Make searches for empty subsequences do exactly what `String.prototype.indexOf()` does. Fixes: nodejs#13023 PR-URL: nodejs#13024 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
1 parent 5a948f6 commit 28ddac2

5 files changed

Lines changed: 62 additions & 34 deletions

File tree

‎doc/api/buffer.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,6 +1340,10 @@ console.log(b.indexOf('b', null));
13401340
console.log(b.indexOf('b', []));
13411341
```
13421342

1343+
If `value` is an empty string or empty `Buffer` and `byteOffset` is less
1344+
than `buf.length`, `byteOffset` will be returned. If `value` is empty and
1345+
`byteOffset` is at least `buf.length`, `buf.length` will be returned.
1346+
13431347
### buf.keys()
13441348
<!-- YAML
13451349
added: v1.1.0
@@ -1450,6 +1454,8 @@ console.log(b.lastIndexOf('b', null));
14501454
console.log(b.lastIndexOf('b', []));
14511455
```
14521456

1457+
If `value` is an empty string or empty `Buffer`, `byteOffset` will be returned.
1458+
14531459
### buf.length
14541460
<!-- YAML
14551461
added: v0.1.90

‎lib/buffer.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ function bidirectionalIndexOf(buffer, val, byteOffset, encoding, dir) {
686686
// If the offset is undefined, "foo", {}, coerces to NaN, search whole buffer.
687687
// `x !== x`-style conditionals are a faster form of `isNaN(x)`
688688
if (byteOffset !== byteOffset) {
689-
byteOffset = dir ? 0 : (buffer.length - 1);
689+
byteOffset = dir ? 0 : buffer.length;
690690
}
691691
dir = !!dir; // Cast to bool.
692692

‎src/node_buffer.cc‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -919,27 +919,29 @@ void Compare(const FunctionCallbackInfo<Value> &args) {
919919
// Computes the offset for starting an indexOf or lastIndexOf search.
920920
// Returns either a valid offset in [0...<length - 1>], ie inside the Buffer,
921921
// or -1 to signal that there is no possible match.
922-
int64_t IndexOfOffset(size_t length, int64_t offset_i64, bool is_forward) {
922+
int64_t IndexOfOffset(size_t length,
923+
int64_t offset_i64,
924+
int64_t needle_length,
925+
bool is_forward) {
923926
int64_t length_i64 = static_cast<int64_t>(length);
924-
if (length_i64 == 0) {
925-
// Empty buffer, no match.
926-
return -1;
927-
}
928927
if (offset_i64 < 0) {
929928
if (offset_i64 + length_i64 >= 0) {
930929
// Negative offsets count backwards from the end of the buffer.
931930
return length_i64 + offset_i64;
932-
} else if (is_forward) {
931+
} else if (is_forward || needle_length == 0) {
933932
// indexOf from before the start of the buffer: search the whole buffer.
934933
return 0;
935934
} else {
936935
// lastIndexOf from before the start of the buffer: no match.
937936
return -1;
938937
}
939938
} else {
940-
if (offset_i64 < length_i64) {
939+
if (offset_i64 + needle_length <= length_i64) {
941940
// Valid positive offset.
942941
return offset_i64;
942+
} else if (needle_length == 0) {
943+
// Out of buffer bounds, but empty needle: point to end of buffer.
944+
return length_i64;
943945
} else if (is_forward) {
944946
// indexOf from past the end of the buffer: no match.
945947
return -1;
@@ -974,11 +976,21 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
974976
const size_t needle_length =
975977
StringBytes::Size(args.GetIsolate(), needle, enc);
976978

977-
if (needle_length == 0 || haystack_length == 0) {
979+
int64_t opt_offset = IndexOfOffset(haystack_length,
980+
offset_i64,
981+
needle_length,
982+
is_forward);
983+
984+
if (needle_length == 0) {
985+
// Match String#indexOf() and String#lastIndexOf() behaviour.
986+
args.GetReturnValue().Set(static_cast<double>(opt_offset));
987+
return;
988+
}
989+
990+
if (haystack_length == 0) {
978991
return args.GetReturnValue().Set(-1);
979992
}
980993

981-
int64_t opt_offset = IndexOfOffset(haystack_length, offset_i64, is_forward);
982994
if (opt_offset <= -1) {
983995
return args.GetReturnValue().Set(-1);
984996
}
@@ -1077,11 +1089,21 @@ void IndexOfBuffer(const FunctionCallbackInfo<Value>& args) {
10771089
const char* needle = buf_data;
10781090
const size_t needle_length = buf_length;
10791091

1080-
if (needle_length == 0 || haystack_length == 0) {
1092+
int64_t opt_offset = IndexOfOffset(haystack_length,
1093+
offset_i64,
1094+
needle_length,
1095+
is_forward);
1096+
1097+
if (needle_length == 0) {
1098+
// Match String#indexOf() and String#lastIndexOf() behaviour.
1099+
args.GetReturnValue().Set(static_cast<double>(opt_offset));
1100+
return;
1101+
}
1102+
1103+
if (haystack_length == 0) {
10811104
return args.GetReturnValue().Set(-1);
10821105
}
10831106

1084-
int64_t opt_offset = IndexOfOffset(haystack_length, offset_i64, is_forward);
10851107
if (opt_offset <= -1) {
10861108
return args.GetReturnValue().Set(-1);
10871109
}
@@ -1132,8 +1154,8 @@ void IndexOfNumber(const FunctionCallbackInfo<Value>& args) {
11321154
int64_t offset_i64 = args[2]->IntegerValue();
11331155
bool is_forward = args[3]->IsTrue();
11341156

1135-
int64_t opt_offset = IndexOfOffset(ts_obj_length, offset_i64, is_forward);
1136-
if (opt_offset <= -1) {
1157+
int64_t opt_offset = IndexOfOffset(ts_obj_length, offset_i64, 1, is_forward);
1158+
if (opt_offset <= -1 || ts_obj_length == 0) {
11371159
return args.GetReturnValue().Set(-1);
11381160
}
11391161
size_t offset = static_cast<size_t>(opt_offset);

‎test/parallel/test-buffer-includes.js‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ assert(b.includes('bc', -Infinity));
2929
assert(!b.includes('bc', Infinity));
3030
assert(b.includes('f'), b.length - 1);
3131
assert(!b.includes('z'));
32-
assert(!b.includes(''));
33-
assert(!b.includes('', 1));
34-
assert(!b.includes('', b.length + 1));
35-
assert(!b.includes('', Infinity));
32+
assert(b.includes(''));
33+
assert(b.includes('', 1));
34+
assert(b.includes('', b.length + 1));
35+
assert(b.includes('', Infinity));
3636
assert(b.includes(buf_a));
3737
assert(!b.includes(buf_a, 1));
3838
assert(!b.includes(buf_a, -1));
@@ -51,10 +51,10 @@ assert(b.includes(buf_bc, -Infinity));
5151
assert(!b.includes(buf_bc, Infinity));
5252
assert(b.includes(buf_f), b.length - 1);
5353
assert(!b.includes(buf_z));
54-
assert(!b.includes(buf_empty));
55-
assert(!b.includes(buf_empty, 1));
56-
assert(!b.includes(buf_empty, b.length + 1));
57-
assert(!b.includes(buf_empty, Infinity));
54+
assert(b.includes(buf_empty));
55+
assert(b.includes(buf_empty, 1));
56+
assert(b.includes(buf_empty, b.length + 1));
57+
assert(b.includes(buf_empty, Infinity));
5858
assert(b.includes(0x61));
5959
assert(!b.includes(0x61, 1));
6060
assert(!b.includes(0x61, -1));

‎test/parallel/test-buffer-indexof.js‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ assert.strictEqual(b.indexOf('bc', -Infinity), 1);
3131
assert.strictEqual(b.indexOf('bc', Infinity), -1);
3232
assert.strictEqual(b.indexOf('f'), b.length - 1);
3333
assert.strictEqual(b.indexOf('z'), -1);
34-
assert.strictEqual(b.indexOf(''), -1);
35-
assert.strictEqual(b.indexOf('', 1), -1);
36-
assert.strictEqual(b.indexOf('', b.length + 1), -1);
37-
assert.strictEqual(b.indexOf('', Infinity), -1);
34+
assert.strictEqual(b.indexOf(''), 0);
35+
assert.strictEqual(b.indexOf('', 1), 1);
36+
assert.strictEqual(b.indexOf('', b.length + 1), b.length);
37+
assert.strictEqual(b.indexOf('', Infinity), b.length);
3838
assert.strictEqual(b.indexOf(buf_a), 0);
3939
assert.strictEqual(b.indexOf(buf_a, 1), -1);
4040
assert.strictEqual(b.indexOf(buf_a, -1), -1);
@@ -53,10 +53,10 @@ assert.strictEqual(b.indexOf(buf_bc, -Infinity), 1);
5353
assert.strictEqual(b.indexOf(buf_bc, Infinity), -1);
5454
assert.strictEqual(b.indexOf(buf_f), b.length - 1);
5555
assert.strictEqual(b.indexOf(buf_z), -1);
56-
assert.strictEqual(b.indexOf(buf_empty), -1);
57-
assert.strictEqual(b.indexOf(buf_empty, 1), -1);
58-
assert.strictEqual(b.indexOf(buf_empty, b.length + 1), -1);
59-
assert.strictEqual(b.indexOf(buf_empty, Infinity), -1);
56+
assert.strictEqual(b.indexOf(buf_empty), 0);
57+
assert.strictEqual(b.indexOf(buf_empty, 1), 1);
58+
assert.strictEqual(b.indexOf(buf_empty, b.length + 1), b.length);
59+
assert.strictEqual(b.indexOf(buf_empty, Infinity), b.length);
6060
assert.strictEqual(b.indexOf(0x61), 0);
6161
assert.strictEqual(b.indexOf(0x61, 1), -1);
6262
assert.strictEqual(b.indexOf(0x61, -1), -1);
@@ -429,10 +429,10 @@ assert.strictEqual(b.lastIndexOf(buf_bc, -Infinity), -1);
429429
assert.strictEqual(b.lastIndexOf(buf_bc, Infinity), 1);
430430
assert.strictEqual(b.lastIndexOf(buf_f), b.length - 1);
431431
assert.strictEqual(b.lastIndexOf(buf_z), -1);
432-
assert.strictEqual(b.lastIndexOf(buf_empty), -1);
433-
assert.strictEqual(b.lastIndexOf(buf_empty, 1), -1);
434-
assert.strictEqual(b.lastIndexOf(buf_empty, b.length + 1), -1);
435-
assert.strictEqual(b.lastIndexOf(buf_empty, Infinity), -1);
432+
assert.strictEqual(b.lastIndexOf(buf_empty), b.length);
433+
assert.strictEqual(b.lastIndexOf(buf_empty, 1), 1);
434+
assert.strictEqual(b.lastIndexOf(buf_empty, b.length + 1), b.length);
435+
assert.strictEqual(b.lastIndexOf(buf_empty, Infinity), b.length);
436436
// lastIndexOf number:
437437
assert.strictEqual(b.lastIndexOf(0x61), 0);
438438
assert.strictEqual(b.lastIndexOf(0x61, 1), 0);

0 commit comments

Comments
 (0)