src: change Fill() to use ParseArrayIndex()

Changed Fill() to use ParseArrayIndex() when getting start and end of buffers instead of Uint32Value, supporting buffers of greater than 2**32 Fixes: #31514 Co-Authored-By: Rich Trott <[email protected]> PR-URL: #31591 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Mar 17, 2020 · da9e71b · da9e71b
1 parent 1caf98f
commit da9e71b
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 12 deletions.
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -570,10 +570,11 @@ void Fill(const FunctionCallbackInfo<Value>& args) {
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
   SPREAD_BUFFER_ARG(args[0], ts_obj);
 
-  uint32_t start;
-  if (!args[2]->Uint32Value(ctx).To(&start)) return;
-  uint32_t end;
-  if (!args[3]->Uint32Value(ctx).To(&end)) return;
+  size_t start;
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &start));
+  size_t end;
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[3], 0, &end));
+
   size_t fill_length = end - start;
   Local<String> str_obj;
   size_t str_length;

diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js
@@ -325,10 +325,12 @@ Buffer.alloc(8, '');
   assert.strictEqual(buf.toString(), 'էէէէէ');
 }
 
-// Testing process.binding. Make sure "start" is properly checked for -1 wrap
-// around.
-assert.strictEqual(
-  internalBinding('buffer').fill(Buffer.alloc(1), 1, -1, 0, 1), -2);
+// Testing process.binding. Make sure "start" is properly checked for range
+// errors.
+assert.throws(
+  () => { internalBinding('buffer').fill(Buffer.alloc(1), 1, -1, 0, 1); },
+  { code: 'ERR_OUT_OF_RANGE' }
+);
 
 // Make sure "end" is properly checked, even if it's magically mangled using
 // Symbol.toPrimitive.
@@ -347,10 +349,12 @@ assert.strictEqual(
   });
 }
 
-// Testing process.binding. Make sure "end" is properly checked for -1 wrap
-// around.
-assert.strictEqual(
-  internalBinding('buffer').fill(Buffer.alloc(1), 1, 1, -2, 1), -2);
+// Testing process.binding. Make sure "end" is properly checked for range
+// errors.
+assert.throws(
+  () => { internalBinding('buffer').fill(Buffer.alloc(1), 1, 1, -2, 1); },
+  { code: 'ERR_OUT_OF_RANGE' }
+);
 
 // Test that bypassing 'length' won't cause an abort.
 common.expectsError(() => {