From 44015754a35107b81c642871271e380203b10739 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 3 May 2016 03:22:47 +0200 Subject: [PATCH] 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 Reviewed-By: Trevor Norris --- src/string_search.h | 12 +++++++----- test/parallel/test-buffer-indexof.js | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/string_search.h b/src/string_search.h index bf246702d7e75e..e7b13559416cb3 100644 --- a/src/string_search.h +++ b/src/string_search.h @@ -261,19 +261,19 @@ inline size_t FindFirstCharacter(Vector 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(void_pos); if (char_pos == nullptr) @@ -308,7 +308,9 @@ inline size_t FindFirstCharacter(Vector 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(pos); if (char_pos == nullptr) { diff --git a/test/parallel/test-buffer-indexof.js b/test/parallel/test-buffer-indexof.js index cd3fa1e148c8b8..477ac2c470c064 100644 --- a/test/parallel/test-buffer-indexof.js +++ b/test/parallel/test-buffer-indexof.js @@ -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: