buffer: fix crash for invalid index types

2555cb4a4049dc4c41d8a2f4ce50909cc0a12a4a introduced a crash
when a non-number value was passed to `ParseArrayIndex()`.

We do not always have JS typechecking for that in place, though.
This returns back to the previous behavior of coercing values
to integers, which is certainly questionable.

Refs: https://github.com/nodejs/node/pull/22129
Fixes: https://github.com/nodejs/node/issues/23668

PR-URL: https://github.com/nodejs/node/pull/25154
Reviewed-By: James M Snell <jasnell@gmail.com>

1 files changed, 26 insertions, 0 deletions

diff --git a/test/parallel/test-buffer-copy.js b/test/parallel/test-buffer-copy.js
index 86b1bbebcb..82b71a1dd8 100644
--- a/test/parallel/test-buffer-copy.js
+++ b/test/parallel/test-buffer-copy.js
@@ -26,6 +26,17 @@ let cntr = 0;
 }
 
 {
+  // Current behavior is to coerce values to integers.
+  b.fill(++cntr);
+  c.fill(++cntr);
+  const copied = b.copy(c, '0', '0', '512');
+  assert.strictEqual(copied, 512);
+  for (let i = 0; i < c.length; i++) {
+    assert.strictEqual(c[i], b[i]);
+  }
+}
+
+{
   // copy c into b, without specifying sourceEnd
   b.fill(++cntr);
   c.fill(++cntr);
@@ -152,3 +163,18 @@ assert.strictEqual(b.copy(c, 512, 0, 10), 0);
     assert.strictEqual(c[i], e[i]);
   }
 }
+
+// https://github.com/nodejs/node/issues/23668: Do not crash for invalid input.
+c.fill('c');
+b.copy(c, 'not a valid offset');
+// Make sure this acted like a regular copy with `0` offset.
+assert.deepStrictEqual(c, b.slice(0, c.length));
+
+{
+  c.fill('C');
+  assert.throws(() => {
+    b.copy(c, { [Symbol.toPrimitive]() { throw new Error('foo'); } });
+  }, /foo/);
+  // No copying took place:
+  assert.deepStrictEqual(c.toString(), 'C'.repeat(c.length));
+}