Skip to content

Commit

Permalink
buffer: fix lastIndexOf index underflow issue
Browse files Browse the repository at this point in the history
Fix `buffer.lastIndexOf()` for the case that the first character
of the needle is contained in the haystack, but in a location that
makes it impossible to be part of a full match.

For example, when searching for 'abc' in 'abcdef', only the 'cdef'
part needs to be searched for 'c', because earlier locations can
be excluded by index calculations alone.

Previously, such a search would result in an assertion failure.

This applies only to Node.js v6, as `lastIndexOf` was added in it.

PR-URL: #6511
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
addaleax authored and evanlucas committed May 17, 2016
1 parent 6aa92d5 commit 4401575
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 5 deletions.
12 changes: 7 additions & 5 deletions src/string_search.h
Original file line number Diff line number Diff line change
Expand Up @@ -261,19 +261,19 @@ inline size_t FindFirstCharacter(Vector<const Char> pattern,
const uint8_t search_byte = GetHighestValueByte(pattern_first_char);
size_t pos = index;
do {
size_t bytes_to_search;
const size_t bytes_to_search = (max_n - pos) * sizeof(Char);
const void* void_pos;
if (subject.forward()) {
// Assert that bytes_to_search won't overflow
CHECK_LE(pos, max_n);
CHECK_LE(max_n - pos, SIZE_MAX / sizeof(Char));
bytes_to_search = (max_n - pos) * sizeof(Char);
void_pos = memchr(subject.start() + pos, search_byte, bytes_to_search);
} else {
CHECK_LE(pos, subject.length());
CHECK_LE(subject.length() - pos, SIZE_MAX / sizeof(Char));
bytes_to_search = (subject.length() - pos) * sizeof(Char);
void_pos = MemrchrFill(subject.start(), search_byte, bytes_to_search);
void_pos = MemrchrFill(subject.start() + pattern.length() - 1,
search_byte,
bytes_to_search);
}
const Char* char_pos = static_cast<const Char*>(void_pos);
if (char_pos == nullptr)
Expand Down Expand Up @@ -308,7 +308,9 @@ inline size_t FindFirstCharacter(Vector<const uint8_t> pattern,
if (subject.forward()) {
pos = memchr(subject.start() + index, pattern_first_char, max_n - index);
} else {
pos = MemrchrFill(subject.start(), pattern_first_char, subj_len - index);
pos = MemrchrFill(subject.start() + pattern.length() - 1,
pattern_first_char,
max_n - index);
}
const uint8_t* char_pos = static_cast<const uint8_t*>(pos);
if (char_pos == nullptr) {
Expand Down
17 changes: 17 additions & 0 deletions test/parallel/test-buffer-indexof.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,23 @@ assert.equal(13, bufferString.lastIndexOf('a ', -1));
assert.equal(0, bufferString.lastIndexOf('a ', -27));
assert.equal(-1, bufferString.lastIndexOf('a ', -28));

// Test lastIndexOf for the case that the first character can be found,
// but in a part of the buffer that does not make search to search
// due do length constraints.
const abInUCS2 = Buffer.from('ab', 'ucs2');
assert.strictEqual(-1, Buffer.from('µaaaa¶bbbb', 'binary').lastIndexOf('µ'));
assert.strictEqual(-1, Buffer.from('bc').lastIndexOf('ab'));
assert.strictEqual(-1, Buffer.from('abc').lastIndexOf('qa'));
assert.strictEqual(-1, Buffer.from('abcdef').lastIndexOf('qabc'));
assert.strictEqual(-1, Buffer.from('bc').lastIndexOf(Buffer.from('ab')));
assert.strictEqual(-1, Buffer.from('bc', 'ucs2').lastIndexOf('ab', 'ucs2'));
assert.strictEqual(-1, Buffer.from('bc', 'ucs2').lastIndexOf(abInUCS2));

assert.strictEqual(0, Buffer.from('abc').lastIndexOf('ab'));
assert.strictEqual(0, Buffer.from('abc').lastIndexOf('ab', 1));
assert.strictEqual(0, Buffer.from('abc').lastIndexOf('ab', 2));
assert.strictEqual(0, Buffer.from('abc').lastIndexOf('ab', 3));

// The above tests test the LINEAR and SINGLE-CHAR strategies.
// Now, we test the BOYER-MOORE-HORSPOOL strategy.
// Test lastIndexOf on a long buffer w multiple matches:
Expand Down

0 comments on commit 4401575

Please sign in to comment.