From e3fd788e66d9a29fb265ab944e856bfa1db09eb3 Mon Sep 17 00:00:00 2001
From: DC <dcposch@dcpos.ch>
Date: Tue, 6 Dec 2016 23:28:06 -0800
Subject: [PATCH 1/4] test: update Buffer.lastIndexOf

Test type coercion for non-number offset arguments. Verify that Buffer and String behave the same way.
---
 test/parallel/test-buffer-indexof.js | 31 +++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/test/parallel/test-buffer-indexof.js b/test/parallel/test-buffer-indexof.js
index 208fed08cdf45c..ffec5e0a661300 100644
--- a/test/parallel/test-buffer-indexof.js
+++ b/test/parallel/test-buffer-indexof.js
@@ -413,14 +413,39 @@ assert.strictEqual(b.lastIndexOf(0x61, Infinity), 0);
 assert.strictEqual(b.lastIndexOf(0x0), -1);
 
 // Test weird offset arguments.
-// Behaviour should match String.lastIndexOf:
-assert.strictEqual(b.lastIndexOf('b', 0), -1);
+// The following offsets coerce to NaN, searching the whole Buffer (or String)
 assert.strictEqual(b.lastIndexOf('b', undefined), 1);
-assert.strictEqual(b.lastIndexOf('b', null), -1);
 assert.strictEqual(b.lastIndexOf('b', {}), 1);
+
+// The following offsets coerce to 0
+assert.strictEqual(b.lastIndexOf('b', 0), -1);
+assert.strictEqual(b.lastIndexOf('b', null), -1);
 assert.strictEqual(b.lastIndexOf('b', []), -1);
+
+// The following offset coerces to 2, in other words +[2] === 2
 assert.strictEqual(b.lastIndexOf('b', [2]), 1);
 
+// Behavior should match String.lastIndexOf()
+var s = 'abcdef';
+assert.strictEqual(
+  b.lastIndexOf('b', undefined),
+  s.lastIndexOf('b', undefined));
+assert.strictEqual(
+  b.lastIndexOf('b', {}),
+  s.lastIndexOf('b', {}));
+assert.strictEqual(
+  b.lastIndexOf('b', 0),
+  s.lastIndexOf('b', 0));
+assert.strictEqual(
+  b.lastIndexOf('b', null),
+  s.lastIndexOf('b', null));
+assert.strictEqual(
+  b.lastIndexOf('b', []),
+  s.lastIndexOf('b', []));
+assert.strictEqual(
+  b.lastIndexOf('b', [2]),
+  s.lastIndexOf('b', [2]));
+
 // Test needles longer than the haystack.
 assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 'ucs2'), -1);
 assert.strictEqual(b.lastIndexOf('aaaaaaaaaaaaaaa', 'utf8'), -1);

From 75af3ef4233c7274a77f77c278fd7cffd6e99256 Mon Sep 17 00:00:00 2001
From: DC <dcposch@dcpos.ch>
Date: Wed, 7 Dec 2016 00:36:16 -0800
Subject: [PATCH 2/4] doc: clarify Buffer.indexOf/lastIndexOf edge cases

---
 doc/api/buffer.md | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/doc/api/buffer.md b/doc/api/buffer.md
index f4ef0b5bd15be7..5d39edbe1321a5 100644
--- a/doc/api/buffer.md
+++ b/doc/api/buffer.md
@@ -1164,6 +1164,30 @@ console.log(utf16Buffer.indexOf('\u03a3', 0, 'ucs2'));
 console.log(utf16Buffer.indexOf('\u03a3', -4, 'ucs2'));
 ```
 
+If `value` is not a string, number, or `Buffer`, this method will throw a
+`TypeError`. If `value` is a number, it will be coerced to a valid byte value,
+an integer between 0 and 255.
+
+If `byteOffset` is not a number, it will be coerced to a number. Any arguments
+that coerce to `NaN` or 0, like `{}`, `[]`, `null` or `undefined`, will search
+the whole buffer. This behavior matches [`String#indexOf()`].
+
+```js
+const b = Buffer.from('abcdef');
+
+// Passing a value that's a number, but not a valid byte
+// Prints: 2, equivalent to searching for 99 or 'c'
+console.log(b.indexOf(99.9));
+console.log(b.indexOf(256 + 99));
+
+// Passing a byteOffset that coerces to NaN or 0
+// Prints: 1, searching the whole buffer
+console.log(b.indexOf('b', undefined));
+console.log(b.indexOf('b', {}));
+console.log(b.indexOf('b', null));
+console.log(b.indexOf('b', []));
+```
+
 ### buf.includes(value[, byteOffset][, encoding])
 <!-- YAML
 added: v5.3.0
@@ -1284,6 +1308,33 @@ console.log(utf16Buffer.lastIndexOf('\u03a3', undefined, 'ucs2'));
 console.log(utf16Buffer.lastIndexOf('\u03a3', -5, 'ucs2'));
 ```
 
+If `value` is not a string, number, or `Buffer`, this method will throw a
+`TypeError`. If `value` is a number, it will be coerced to a valid byte value,
+an integer between 0 and 255.
+
+If `byteOffset` is not a number, it will be coerced to a number. Any arguments
+that coerce to `NaN`, like `{}` or `undefined`, will search the whole buffer.
+This behavior matches [`String#lastIndexOf()`].
+
+```js
+const b = Buffer.from('abcdef');
+
+// Passing a value that's a number, but not a valid byte
+// Prints: 2, equivalent to searching for 99 or 'c'
+console.log(b.lastIndexOf(99.9));
+console.log(b.lastIndexOf(256 + 99));
+
+// Passing a byteOffset that coerces to NaN
+// Prints: 1, searching the whole buffer
+console.log(b.lastIndexOf('b', undefined));
+console.log(b.lastIndexOf('b', {}));
+
+// Passing a byteOffset that coerces to 0
+// Prints: -1, equivalent to passing 0
+console.log(b.lastIndexOf('b', null));
+console.log(b.lastIndexOf('b', []));
+```
+
 ### buf.length
 <!-- YAML
 added: v0.1.90
@@ -2463,6 +2514,8 @@ console.log(buf);
 [RFC1345]: https://tools.ietf.org/html/rfc1345
 [RFC4648, Section 5]: https://tools.ietf.org/html/rfc4648#section-5
 [`String.prototype.length`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length
+[`String#indexOf()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/indexOf
+[`String#lastIndexOf()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/lastIndexOf
 [`TypedArray`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray
 [`TypedArray.from()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/from
 [`Uint32Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint32Array

From 47b1f70f48e2ea4dafcf3e3f0e9a10d7f0d4f4d0 Mon Sep 17 00:00:00 2001
From: DC <dcposch@dcpos.ch>
Date: Wed, 7 Dec 2016 00:47:38 -0800
Subject: [PATCH 3/4] buffer: fix comments in bidirectionalIndexOf

---
 lib/buffer.js | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/buffer.js b/lib/buffer.js
index 875dc13ac3089b..70ab6281eae673 100644
--- a/lib/buffer.js
+++ b/lib/buffer.js
@@ -598,9 +598,10 @@ function bidirectionalIndexOf(buffer, val, byteOffset, encoding, dir) {
   } else if (byteOffset < -0x80000000) {
     byteOffset = -0x80000000;
   }
-  byteOffset = +byteOffset;  // Coerce to Number.
-  if (isNaN(byteOffset)) {
-    // If the offset is undefined, null, NaN, "foo", etc, search whole buffer.
+  // Coerce to Number. Values like null and [] become 0.
+  byteOffset = +byteOffset;
+  // If the offset is undefined, "foo", {}, coerces to NaN, search whole buffer.
+  if (Number.isNaN(byteOffset)) {
     byteOffset = dir ? 0 : (buffer.length - 1);
   }
   dir = !!dir;  // Cast to bool.

From d43ad1fa4cdbd7c2504a3323a601d99b92559bcf Mon Sep 17 00:00:00 2001
From: DC <dcposch@dcpos.ch>
Date: Sun, 18 Dec 2016 11:00:12 -0800
Subject: [PATCH 4/4] test: Buffer#indexOf weird offsets

---
 test/parallel/test-buffer-indexof.js | 36 ++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/test/parallel/test-buffer-indexof.js b/test/parallel/test-buffer-indexof.js
index ffec5e0a661300..99f147b3380fd4 100644
--- a/test/parallel/test-buffer-indexof.js
+++ b/test/parallel/test-buffer-indexof.js
@@ -11,6 +11,8 @@ const buf_f = Buffer.from('f');
 const buf_z = Buffer.from('z');
 const buf_empty = Buffer.from('');
 
+const s = 'abcdef';
+
 assert.strictEqual(b.indexOf('a'), 0);
 assert.strictEqual(b.indexOf('a', 1), -1);
 assert.strictEqual(b.indexOf('a', -1), -1);
@@ -359,6 +361,37 @@ assert.throws(() => {
   b.indexOf([]);
 }, argumentExpected);
 
+// Test weird offset arguments.
+// The following offsets coerce to NaN or 0, searching the whole Buffer
+assert.strictEqual(b.indexOf('b', undefined), 1);
+assert.strictEqual(b.indexOf('b', {}), 1);
+assert.strictEqual(b.indexOf('b', 0), 1);
+assert.strictEqual(b.indexOf('b', null), 1);
+assert.strictEqual(b.indexOf('b', []), 1);
+
+// The following offset coerces to 2, in other words +[2] === 2
+assert.strictEqual(b.indexOf('b', [2]), -1);
+
+// Behavior should match String.indexOf()
+assert.strictEqual(
+  b.indexOf('b', undefined),
+  s.indexOf('b', undefined));
+assert.strictEqual(
+  b.indexOf('b', {}),
+  s.indexOf('b', {}));
+assert.strictEqual(
+  b.indexOf('b', 0),
+  s.indexOf('b', 0));
+assert.strictEqual(
+  b.indexOf('b', null),
+  s.indexOf('b', null));
+assert.strictEqual(
+  b.indexOf('b', []),
+  s.indexOf('b', []));
+assert.strictEqual(
+  b.indexOf('b', [2]),
+  s.indexOf('b', [2]));
+
 // All code for handling encodings is shared between Buffer.indexOf and
 // Buffer.lastIndexOf, so only testing the separate lastIndexOf semantics.
 
@@ -413,7 +446,7 @@ assert.strictEqual(b.lastIndexOf(0x61, Infinity), 0);
 assert.strictEqual(b.lastIndexOf(0x0), -1);
 
 // Test weird offset arguments.
-// The following offsets coerce to NaN, searching the whole Buffer (or String)
+// The following offsets coerce to NaN, searching the whole Buffer
 assert.strictEqual(b.lastIndexOf('b', undefined), 1);
 assert.strictEqual(b.lastIndexOf('b', {}), 1);
 
@@ -426,7 +459,6 @@ assert.strictEqual(b.lastIndexOf('b', []), -1);
 assert.strictEqual(b.lastIndexOf('b', [2]), 1);
 
 // Behavior should match String.lastIndexOf()
-var s = 'abcdef';
 assert.strictEqual(
   b.lastIndexOf('b', undefined),
   s.lastIndexOf('b', undefined));