lib,src: refactor buffer out of range index

PR-URL: #11296 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>
nodejs · May 28, 2017 · 234353a · TimothyGu · May 28, 2017 · Trott
1 parent 1474b7a
commit 234353a
Show file tree

Hide file tree

Showing 9 changed files with 72 additions and 50 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -558,6 +558,14 @@ found [here][online].
   the connected party did not properly respond after a period of time. Usually
   encountered by [`http`][] or [`net`][] -- often a sign that a `socket.end()`
   was not properly called.
+
+  <a id="ERROR_CODES"></a>
+  ### ERROR CODES
+
+  <a id="ERR_INDEX_OUT_OF_RANGE"></a>
+  ### ERR_INDEX_OUT_OF_RANGE
+
+The `'ERR_INDEX_OUT_OF_RANGE'` error code is used when a given index is out of the accepted range.
 
 
 <a id="nodejs-error-codes"></a>

diff --git a/lib/buffer.js b/lib/buffer.js
@@ -28,6 +28,7 @@ const { isAnyArrayBuffer, isUint8Array } = process.binding('util');
 const bindingObj = {};
 const internalUtil = require('internal/util');
 const pendingDeprecation = !!config.pendingDeprecation;
+const errors = require('internal/errors');
 
 class FastBuffer extends Uint8Array {
   constructor(arg1, arg2, arg3) {
@@ -629,28 +630,28 @@ Buffer.prototype.compare = function compare(target,
   if (start === undefined)
     start = 0;
   else if (start < 0)
-    throw new RangeError('out of range index');
+    throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
   else
     start >>>= 0;
 
   if (end === undefined)
     end = target.length;
   else if (end > target.length)
-    throw new RangeError('out of range index');
+    throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
   else
     end >>>= 0;
 
   if (thisStart === undefined)
     thisStart = 0;
   else if (thisStart < 0)
-    throw new RangeError('out of range index');
+    throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
   else
     thisStart >>>= 0;
 
   if (thisEnd === undefined)
     thisEnd = this.length;
   else if (thisEnd > this.length)
-    throw new RangeError('out of range index');
+    throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
   else
     thisEnd >>>= 0;
 
@@ -795,7 +796,7 @@ Buffer.prototype.fill = function fill(val, start, end, encoding) {
 
   // Invalid ranges are not set to a default, so can range check early.
   if (start < 0 || end > this.length)
-    throw new RangeError('Out of range index');
+    throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
 
   if (end <= start)
     return this;
@@ -931,7 +932,7 @@ Buffer.prototype.slice = function slice(start, end) {
 
 function checkOffset(offset, ext, length) {
   if (offset + ext > length)
-    throw new RangeError('Index out of range');
+    throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
 }
 
 
@@ -1141,7 +1142,7 @@ function checkInt(buffer, value, offset, ext, max, min) {
   if (value > max || value < min)
     throw new TypeError('"value" argument is out of bounds');
   if (offset + ext > buffer.length)
-    throw new RangeError('Index out of range');
+    throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
 }
 
 

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -114,6 +114,7 @@ E('ERR_ARG_NOT_ITERABLE', '%s must be iterable');
 E('ERR_ASSERTION', (msg) => msg);
 E('ERR_CONSOLE_WRITABLE_STREAM',
   (name) => `Console expects a writable stream instance for ${name}`);
+E('ERR_INDEX_OUT_OF_RANGE', 'Index out of range');
 E('ERR_INVALID_ARG_TYPE', invalidArgType);
 E('ERR_INVALID_CALLBACK', 'callback must be a function');
 E('ERR_INVALID_FILE_URL_HOST', 'File URL host %s');

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -40,7 +40,7 @@
 
 #define THROW_AND_RETURN_IF_OOB(r)                                          \
   do {                                                                      \
-    if (!(r)) return env->ThrowRangeError("out of range index");            \
+    if (!(r)) return env->ThrowRangeError("Index out of range");            \
   } while (0)
 
 #define SLICE_START_END(start_arg, end_arg, end_max)                        \
@@ -564,7 +564,7 @@ void Copy(const FunctionCallbackInfo<Value> &args) {
     return args.GetReturnValue().Set(0);
 
   if (source_start > ts_obj_length)
-    return env->ThrowRangeError("out of range index");
+    return env->ThrowRangeError("Index out of range");
 
   if (source_end - source_start > target_length - target_start)
     source_end = source_start + target_length - target_start;
@@ -878,9 +878,9 @@ void CompareOffset(const FunctionCallbackInfo<Value> &args) {
   THROW_AND_RETURN_IF_OOB(ParseArrayIndex(args[5], ts_obj_length, &source_end));
 
   if (source_start > ts_obj_length)
-    return env->ThrowRangeError("out of range index");
+    return env->ThrowRangeError("Index out of range");
   if (target_start > target_length)
-    return env->ThrowRangeError("out of range index");
+    return env->ThrowRangeError("Index out of range");
 
   CHECK_LE(source_start, source_end);
   CHECK_LE(target_start, target_end);

diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js
@@ -963,7 +963,8 @@ assert.throws(() => {
   const a = Buffer.alloc(1);
   const b = Buffer.alloc(1);
   a.copy(b, 0, 0x100000000, 0x100000001);
-}, /out of range index/);
+}, common.expectsError(
+  {code: undefined, type: RangeError, message: 'Index out of range'}));
 
 // Unpooled buffer (replaces SlowBuffer)
 {

diff --git a/test/parallel/test-buffer-compare-offset.js b/test/parallel/test-buffer-compare-offset.js
@@ -1,6 +1,6 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 
 const a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 0]);
@@ -57,7 +57,7 @@ assert.strictEqual(1, a.compare(b, Infinity, -Infinity));
 // zero length target because default for targetEnd <= targetSource
 assert.strictEqual(1, a.compare(b, '0xff'));
 
-const oor = /out of range index/;
+const oor = common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'});
 
 assert.throws(() => a.compare(b, 0, 100, 0), oor);
 assert.throws(() => a.compare(b, 0, 1, 0, 100), oor);

diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js
@@ -1,7 +1,8 @@
+// Flags: --expose-internals
 'use strict';
-
-require('../common');
+const common = require('../common');
 const assert = require('assert');
+const errors = require('internal/errors');
 const SIZE = 28;
 
 const buf1 = Buffer.allocUnsafe(SIZE);
@@ -191,25 +192,30 @@ deepStrictEqualValues(genBuffer(4, [hexBufFill, 1, -1]), [0, 0, 0, 0]);
 
 
 // Check exceptions
-assert.throws(() => buf1.fill(0, -1), /^RangeError: Out of range index$/);
-assert.throws(() =>
-             buf1.fill(0, 0, buf1.length + 1),
-              /^RangeError: Out of range index$/);
-assert.throws(() => buf1.fill('', -1), /^RangeError: Out of range index$/);
-assert.throws(() =>
-             buf1.fill('', 0, buf1.length + 1),
-              /^RangeError: Out of range index$/);
-assert.throws(() =>
-             buf1.fill('a', 0, buf1.length, 'node rocks!'),
-              /^TypeError: Unknown encoding: node rocks!$/);
-assert.throws(() =>
-             buf1.fill('a', 0, 0, NaN),
-              /^TypeError: encoding must be a string$/);
-assert.throws(() =>
-             buf1.fill('a', 0, 0, null),
-              /^TypeError: encoding must be a string$/);
-assert.throws(() =>
-             buf1.fill('a', 0, 0, 'foo'), /^TypeError: Unknown encoding: foo$/);
+assert.throws(
+  () => buf1.fill(0, -1),
+  common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'}));
+assert.throws(
+  () => buf1.fill(0, 0, buf1.length + 1),
+  common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'}));
+assert.throws(
+  () => buf1.fill('', -1),
+  common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'}));
+assert.throws(
+  () => buf1.fill('', 0, buf1.length + 1),
+  common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'}));
+assert.throws(
+  () => buf1.fill('a', 0, buf1.length, 'node rocks!'),
+  /^TypeError: Unknown encoding: node rocks!$/);
+assert.throws(
+  () => buf1.fill('a', 0, 0, NaN),
+  /^TypeError: encoding must be a string$/);
+assert.throws(
+  () => buf1.fill('a', 0, 0, null),
+  /^TypeError: encoding must be a string$/);
+assert.throws(
+  () => buf1.fill('a', 0, 0, 'foo'),
+  /^TypeError: Unknown encoding: foo$/);
 
 
 function genBuffer(size, args) {
@@ -241,7 +247,7 @@ function writeToFill(string, offset, end, encoding) {
   }
 
   if (offset < 0 || end > buf2.length)
-    throw new RangeError('Out of range index');
+    throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
 
   if (end <= offset)
     return buf2;
@@ -279,12 +285,12 @@ function testBufs(string, offset, length, encoding) {
 }
 
 // Make sure these throw.
-assert.throws(() =>
-             Buffer.allocUnsafe(8).fill('a', -1),
-              /^RangeError: Out of range index$/);
-assert.throws(() =>
-             Buffer.allocUnsafe(8).fill('a', 0, 9),
-              /^RangeError: Out of range index$/);
+assert.throws(
+  () => Buffer.allocUnsafe(8).fill('a', -1),
+  common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'}));
+assert.throws(
+  () => Buffer.allocUnsafe(8).fill('a', 0, 9),
+  common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'}));
 
 // Make sure this doesn't hang indefinitely.
 Buffer.allocUnsafe(8).fill('');
@@ -350,7 +356,8 @@ Buffer.alloc(8, '');
       }
     };
     Buffer.alloc(1).fill(Buffer.alloc(1), start, 1);
-  }, /out of range index/);
+  }, common.expectsError(
+    {code: undefined, type: RangeError, message: 'Index out of range'}));
   // Make sure -1 is making it to Buffer::Fill().
   assert.ok(elseWasLast,
             'internal API changed, -1 no longer in correct location');
@@ -360,7 +367,8 @@ Buffer.alloc(8, '');
 // around.
 assert.throws(() => {
   process.binding('buffer').fill(Buffer.alloc(1), 1, -1, 0, 1);
-}, /out of range index/);
+}, common.expectsError(
+  {code: undefined, type: RangeError, message: 'Index out of range'}));
 
 // Make sure "end" is properly checked, even if it's magically mangled using
 // Symbol.toPrimitive.
@@ -383,7 +391,8 @@ assert.throws(() => {
       }
     };
     Buffer.alloc(1).fill(Buffer.alloc(1), 0, end);
-  }, /^RangeError: out of range index$/);
+  }, common.expectsError(
+    {code: undefined, type: RangeError, message: 'Index out of range'}));
   // Make sure -1 is making it to Buffer::Fill().
   assert.ok(elseWasLast,
             'internal API changed, -1 no longer in correct location');
@@ -393,7 +402,8 @@ assert.throws(() => {
 // around.
 assert.throws(() => {
   process.binding('buffer').fill(Buffer.alloc(1), 1, 1, -2, 1);
-}, /out of range index/);
+}, common.expectsError(
+  { code: undefined, type: RangeError, message: 'Index out of range'}));
 
 // Test that bypassing 'length' won't cause an abort.
 assert.throws(() => {
@@ -403,7 +413,8 @@ assert.throws(() => {
     enumerable: true
   });
   buf.fill('');
-}, /^RangeError: out of range index$/);
+}, common.expectsError(
+  { code: undefined, type: RangeError, message: 'Index out of range'}));
 
 assert.deepStrictEqual(
     Buffer.allocUnsafeSlow(16).fill('ab', 'utf16le'),

diff --git a/test/parallel/test-buffer-read.js b/test/parallel/test-buffer-read.js
@@ -1,5 +1,5 @@
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 
 // testing basic buffer read functions
@@ -10,7 +10,7 @@ function read(buff, funx, args, expected) {
   assert.strictEqual(buff[funx](...args), expected);
   assert.throws(
     () => buff[funx](-1),
-    /^RangeError: Index out of range$/
+    common.expectsError({code: 'ERR_INDEX_OUT_OF_RANGE'})
   );
 
   assert.doesNotThrow(

diff --git a/test/parallel/test-buffer-write-noassert.js b/test/parallel/test-buffer-write-noassert.js
@@ -4,7 +4,7 @@ const assert = require('assert');
 
 // testing buffer write functions
 
-const outOfRange = /^RangeError: (?:Index )?out of range(?: index)?$/;
+const outOfRange = /^RangeError\b.*\bIndex out of range$/;
 
 function write(funx, args, result, res) {
   {