buffer: fix lastIndexOf index underflow issue

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: https://github.com/nodejs/node/pull/6511
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

2 files changed, 24 insertions, 5 deletions

diff --git a/src/string_search.h b/src/string_search.h
index a9345eb802..0426fd840c 100644
--- a/src/string_search.h
+++ b/src/string_search.h
@@ -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)
@@ -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) {
diff --git a/test/parallel/test-buffer-indexof.js b/test/parallel/test-buffer-indexof.js
index 205a8dfa6a..7d54ea0d47 100644
--- a/test/parallel/test-buffer-indexof.js
+++ b/test/parallel/test-buffer-indexof.js
@@ -391,6 +391,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: