From f8d69911bef0ddd150d0f66f7a75b4655cf60b00 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Wed, 19 Sep 2018 20:04:47 -0700
Subject: [PATCH] errors: use ERR_OUT_OF_RANGE for index errors

Remove ERR_INDEX_OUT_OF_RANGE in favor of ERR_OUT_OF_RANGE which is
capable of providing more detail. (In one instance, use
ERR_BUFFER_OUT_OF_BOUNDS which is more accurate in that one instance.)

PR-URL: https://github.com/nodejs/node/pull/22969
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
---
 doc/api/buffer.md                           |   9 +-
 doc/api/errors.md                           |  13 ++-
 lib/buffer.js                               | 111 ++++++++++----------
 lib/internal/errors.js                      |   1 -
 src/node_buffer.cc                          |  17 +--
 src/node_errors.h                           |  10 +-
 test/parallel/test-buffer-alloc.js          |   5 +-
 test/parallel/test-buffer-compare-offset.js |   2 +-
 test/parallel/test-buffer-fill.js           |  21 ++--
 9 files changed, 104 insertions(+), 85 deletions(-)

diff --git a/doc/api/buffer.md b/doc/api/buffer.md
index 56999a1955d0a1..11f3795741b3d3 100644
--- a/doc/api/buffer.md
+++ b/doc/api/buffer.md
@@ -1085,7 +1085,7 @@ console.log(buf1.compare(buf2, 5, 6, 5));
 // Prints: 1
 ```
 
-[`ERR_INDEX_OUT_OF_RANGE`] is thrown if `targetStart < 0`, `sourceStart < 0`,
+[`ERR_OUT_OF_RANGE`] is thrown if `targetStart < 0`, `sourceStart < 0`,
 `targetEnd > target.byteLength`, or `sourceEnd > source.byteLength`.
 
 ### buf.copy(target[, targetStart[, sourceStart[, sourceEnd]]])
@@ -1197,6 +1197,9 @@ console.log(buf1.equals(buf3));
 <!-- YAML
 added: v0.5.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/22969
+    description: Throws `ERR_OUT_OF_RANGE` instead of `ERR_INDEX_OUT_OF_RANGE`.
   - version: v10.0.0
     pr-url: https://github.com/nodejs/node/pull/18790
     description: Negative `end` values throw an `ERR_INDEX_OUT_OF_RANGE` error.
@@ -1708,7 +1711,7 @@ console.log(buf.readIntLE(0, 6).toString(16));
 console.log(buf.readIntBE(0, 6).toString(16));
 // Prints: 1234567890ab
 console.log(buf.readIntBE(1, 6).toString(16));
-// Throws ERR_INDEX_OUT_OF_RANGE
+// Throws ERR_OUT_OF_RANGE
 console.log(buf.readIntBE(1, 0).toString(16));
 // Throws ERR_OUT_OF_RANGE
 ```
@@ -2640,9 +2643,9 @@ This value may depend on the JS engine that is being used.
 [`Buffer.from(string)`]: #buffer_class_method_buffer_from_string_encoding
 [`Buffer.poolSize`]: #buffer_class_property_buffer_poolsize
 [`DataView`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DataView
-[`ERR_INDEX_OUT_OF_RANGE`]: errors.html#ERR_INDEX_OUT_OF_RANGE
 [`ERR_INVALID_BUFFER_SIZE`]: errors.html#ERR_INVALID_BUFFER_SIZE
 [`ERR_INVALID_OPT_VALUE`]: errors.html#ERR_INVALID_OPT_VALUE
+[`ERR_OUT_OF_RANGE`]: errors.html#ERR_OUT_OF_RANGE
 [`JSON.stringify()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify
 [`SharedArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
 [`String#indexOf()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/indexOf
diff --git a/doc/api/errors.md b/doc/api/errors.md
index ccbf0e5381889a..f9005c3bb19940 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -1102,11 +1102,6 @@ is set for the `Http2Stream`.
 `http2.connect()` was passed a URL that uses any protocol other than `http:` or
 `https:`.
 
-<a id="ERR_INDEX_OUT_OF_RANGE"></a>
-### ERR_INDEX_OUT_OF_RANGE
-
-A given index was out of the accepted range (e.g. negative offsets).
-
 <a id="ERR_INSPECTOR_ALREADY_CONNECTED"></a>
 ### ERR_INSPECTOR_ALREADY_CONNECTED
 
@@ -1930,6 +1925,14 @@ removed: v10.0.0
 Used when an invalid character is found in an HTTP response status message
 (reason phrase).
 
+<a id="ERR_INDEX_OUT_OF_RANGE"></a>
+### ERR_INDEX_OUT_OF_RANGE
+<!-- YAML
+  added: v10.0.0
+  removed: REPLACEME
+-->
+A given index was out of the accepted range (e.g. negative offsets).
+
 <a id="ERR_NAPI_CONS_PROTOTYPE_OBJECT"></a>
 ### ERR_NAPI_CONS_PROTOTYPE_OBJECT
 <!-- YAML
diff --git a/lib/buffer.js b/lib/buffer.js
index 5a21732c35a798..da1563ec3ab91c 100644
--- a/lib/buffer.js
+++ b/lib/buffer.js
@@ -61,7 +61,7 @@ const {
 } = process.binding('config');
 const {
   ERR_BUFFER_OUT_OF_BOUNDS,
-  ERR_INDEX_OUT_OF_RANGE,
+  ERR_OUT_OF_RANGE,
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_ARG_VALUE,
   ERR_INVALID_BUFFER_SIZE,
@@ -692,50 +692,51 @@ Buffer.prototype[customInspectSymbol] = function inspect() {
 Buffer.prototype.inspect = Buffer.prototype[customInspectSymbol];
 
 Buffer.prototype.compare = function compare(target,
-                                            start,
-                                            end,
-                                            thisStart,
-                                            thisEnd) {
+                                            targetStart,
+                                            targetEnd,
+                                            sourceStart,
+                                            sourceEnd) {
   if (!isUint8Array(target)) {
     throw new ERR_INVALID_ARG_TYPE('target', ['Buffer', 'Uint8Array'], target);
   }
   if (arguments.length === 1)
     return _compare(this, target);
 
-  if (start === undefined)
-    start = 0;
-  else if (start < 0)
-    throw new ERR_INDEX_OUT_OF_RANGE();
+  if (targetStart === undefined)
+    targetStart = 0;
+  else if (targetStart < 0)
+    throw new ERR_OUT_OF_RANGE('targetStart', '>= 0', targetStart);
   else
-    start >>>= 0;
+    targetStart >>>= 0;
 
-  if (end === undefined)
-    end = target.length;
-  else if (end > target.length)
-    throw new ERR_INDEX_OUT_OF_RANGE();
+  if (targetEnd === undefined)
+    targetEnd = target.length;
+  else if (targetEnd > target.length)
+    throw new ERR_OUT_OF_RANGE('targetEnd', `<= ${target.length}`, targetEnd);
   else
-    end >>>= 0;
+    targetEnd >>>= 0;
 
-  if (thisStart === undefined)
-    thisStart = 0;
-  else if (thisStart < 0)
-    throw new ERR_INDEX_OUT_OF_RANGE();
+  if (sourceStart === undefined)
+    sourceStart = 0;
+  else if (sourceStart < 0)
+    throw new ERR_OUT_OF_RANGE('sourceStart', '>= 0', sourceStart);
   else
-    thisStart >>>= 0;
+    sourceStart >>>= 0;
 
-  if (thisEnd === undefined)
-    thisEnd = this.length;
-  else if (thisEnd > this.length)
-    throw new ERR_INDEX_OUT_OF_RANGE();
+  if (sourceEnd === undefined)
+    sourceEnd = this.length;
+  else if (sourceEnd > this.length)
+    throw new ERR_OUT_OF_RANGE('sourceEnd', `<= ${this.length}`, sourceEnd);
   else
-    thisEnd >>>= 0;
+    sourceEnd >>>= 0;
 
-  if (thisStart >= thisEnd)
-    return (start >= end ? 0 : -1);
-  else if (start >= end)
+  if (sourceStart >= sourceEnd)
+    return (targetStart >= targetEnd ? 0 : -1);
+  else if (targetStart >= targetEnd)
     return 1;
 
-  return compareOffset(this, target, start, thisStart, end, thisEnd);
+  return compareOffset(this, target, targetStart, sourceStart, targetEnd,
+                       sourceEnd);
 };
 
 // Finds either the first index of `val` in `buffer` at offset >= `byteOffset`,
@@ -827,15 +828,15 @@ Buffer.prototype.includes = function includes(val, byteOffset, encoding) {
 //    buffer.fill(number[, offset[, end]])
 //    buffer.fill(buffer[, offset[, end]])
 //    buffer.fill(string[, offset[, end]][, encoding])
-Buffer.prototype.fill = function fill(val, start, end, encoding) {
-  return _fill(this, val, start, end, encoding);
+Buffer.prototype.fill = function fill(value, offset, end, encoding) {
+  return _fill(this, value, offset, end, encoding);
 };
 
-function _fill(buf, val, start, end, encoding) {
-  if (typeof val === 'string') {
-    if (start === undefined || typeof start === 'string') {
-      encoding = start;
-      start = 0;
+function _fill(buf, value, offset, end, encoding) {
+  if (typeof value === 'string') {
+    if (offset === undefined || typeof offset === 'string') {
+      encoding = offset;
+      offset = 0;
       end = buf.length;
     } else if (typeof end === 'string') {
       encoding = end;
@@ -848,48 +849,48 @@ function _fill(buf, val, start, end, encoding) {
       throw new ERR_UNKNOWN_ENCODING(encoding);
     }
 
-    if (val.length === 0) {
-      // If val === '' default to zero.
-      val = 0;
-    } else if (val.length === 1) {
-      // Fast path: If `val` fits into a single byte, use that numeric value.
+    if (value.length === 0) {
+      // If value === '' default to zero.
+      value = 0;
+    } else if (value.length === 1) {
+      // Fast path: If `value` fits into a single byte, use that numeric value.
       if (normalizedEncoding === 'utf8') {
-        const code = val.charCodeAt(0);
+        const code = value.charCodeAt(0);
         if (code < 128) {
-          val = code;
+          value = code;
         }
       } else if (normalizedEncoding === 'latin1') {
-        val = val.charCodeAt(0);
+        value = value.charCodeAt(0);
       }
     }
   } else {
     encoding = undefined;
   }
 
-  if (start === undefined) {
-    start = 0;
+  if (offset === undefined) {
+    offset = 0;
     end = buf.length;
   } else {
     // Invalid ranges are not set to a default, so can range check early.
+    if (offset < 0)
+      throw new ERR_OUT_OF_RANGE('offset', '>= 0', offset);
     if (end === undefined) {
-      if (start < 0)
-        throw new ERR_INDEX_OUT_OF_RANGE();
       end = buf.length;
     } else {
-      if (start < 0 || end > buf.length || end < 0)
-        throw new ERR_INDEX_OUT_OF_RANGE();
+      if (end > buf.length || end < 0)
+        throw new ERR_OUT_OF_RANGE('end', `>= 0 and <= ${buf.length}`, end);
       end = end >>> 0;
     }
-    start = start >>> 0;
-    if (start >= end)
+    offset = offset >>> 0;
+    if (offset >= end)
       return buf;
   }
 
-  const res = bindingFill(buf, val, start, end, encoding);
+  const res = bindingFill(buf, value, offset, end, encoding);
   if (res < 0) {
     if (res === -1)
-      throw new ERR_INVALID_ARG_VALUE('value', val);
-    throw new ERR_INDEX_OUT_OF_RANGE();
+      throw new ERR_INVALID_ARG_VALUE('value', value);
+    throw new ERR_BUFFER_OUT_OF_BOUNDS();
   }
 
   return buf;
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 1e85bd4b87e0b3..1914f2ec8db000 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -625,7 +625,6 @@ E('ERR_HTTP_INVALID_HEADER_VALUE',
 E('ERR_HTTP_INVALID_STATUS_CODE', 'Invalid status code: %s', RangeError);
 E('ERR_HTTP_TRAILER_INVALID',
   'Trailers are invalid with this transfer encoding', Error);
-E('ERR_INDEX_OUT_OF_RANGE', 'Index out of range', RangeError);
 E('ERR_INSPECTOR_ALREADY_CONNECTED', '%s is already connected', Error);
 E('ERR_INSPECTOR_CLOSED', 'Session was closed', Error);
 E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available', Error);
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index 8642477db4d927..1a7f01a734219b 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -36,12 +36,14 @@
 #define MIN(a, b) ((a) < (b) ? (a) : (b))
 
 #define THROW_AND_RETURN_UNLESS_BUFFER(env, obj)                            \
-  THROW_AND_RETURN_IF_NOT_BUFFER(env, obj, "argument")
+  THROW_AND_RETURN_IF_NOT_BUFFER(env, obj, "argument")                      \
 
 #define THROW_AND_RETURN_IF_OOB(r)                                          \
   do {                                                                      \
-    if (!(r)) return node::THROW_ERR_INDEX_OUT_OF_RANGE(env);               \
-  } while (0)
+    if (!(r))                                                               \
+      return node::THROW_ERR_OUT_OF_RANGE_WITH_TEXT(env,                    \
+                                                    "Index out of range");  \
+  } while (0)                                                               \
 
 #define SLICE_START_END(start_arg, end_arg, end_max)                        \
   size_t start;                                                             \
@@ -494,7 +496,8 @@ void Copy(const FunctionCallbackInfo<Value> &args) {
     return args.GetReturnValue().Set(0);
 
   if (source_start > ts_obj_length)
-    return node::THROW_ERR_INDEX_OUT_OF_RANGE(env);
+    return node::THROW_ERR_OUT_OF_RANGE_WITH_TEXT(
+        env, "The value of \"sourceStart\" is out of range.");
 
   if (source_end - source_start > target_length - target_start)
     source_end = source_start + target_length - target_start;
@@ -684,9 +687,11 @@ 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 node::THROW_ERR_INDEX_OUT_OF_RANGE(env);
+    return node::THROW_ERR_OUT_OF_RANGE_WITH_TEXT(
+        env, "The value of \"sourceStart\" is out of range.");
   if (target_start > target_length)
-    return node::THROW_ERR_INDEX_OUT_OF_RANGE(env);
+    return node::THROW_ERR_OUT_OF_RANGE_WITH_TEXT(
+        env, "The value of \"targetStart\" is out of range.");
 
   CHECK_LE(source_start, source_end);
   CHECK_LE(target_start, target_end);
diff --git a/src/node_errors.h b/src/node_errors.h
index fdfb670af636af..233a0f7532c717 100644
--- a/src/node_errors.h
+++ b/src/node_errors.h
@@ -26,7 +26,6 @@ namespace node {
   V(ERR_CANNOT_TRANSFER_OBJECT, TypeError)                                   \
   V(ERR_CLOSED_MESSAGE_PORT, Error)                                          \
   V(ERR_CONSTRUCT_CALL_REQUIRED, Error)                                      \
-  V(ERR_INDEX_OUT_OF_RANGE, RangeError)                                      \
   V(ERR_INVALID_ARG_VALUE, TypeError)                                        \
   V(ERR_INVALID_ARG_TYPE, TypeError)                                         \
   V(ERR_INVALID_TRANSFER_OBJECT, TypeError)                                  \
@@ -35,6 +34,7 @@ namespace node {
   V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError)                    \
   V(ERR_MISSING_MODULE, Error)                                               \
   V(ERR_MISSING_PLATFORM_FOR_WORKER, Error)                                  \
+  V(ERR_OUT_OF_RANGE, RangeError)                                            \
   V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error)                                 \
   V(ERR_SCRIPT_EXECUTION_TIMEOUT, Error)                                     \
   V(ERR_STRING_TOO_LONG, Error)                                              \
@@ -64,7 +64,6 @@ namespace node {
   V(ERR_CANNOT_TRANSFER_OBJECT, "Cannot transfer object of unsupported type")\
   V(ERR_CLOSED_MESSAGE_PORT, "Cannot send data on closed MessagePort")       \
   V(ERR_CONSTRUCT_CALL_REQUIRED, "Cannot call constructor without `new`")    \
-  V(ERR_INDEX_OUT_OF_RANGE, "Index out of range")                            \
   V(ERR_INVALID_TRANSFER_OBJECT, "Found invalid object in transferList")     \
   V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory")               \
   V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST,                               \
@@ -96,6 +95,13 @@ inline void THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(Environment* env,
   THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(env, message.str().c_str());
 }
 
+inline void THROW_ERR_OUT_OF_RANGE_WITH_TEXT(Environment* env,
+                                             const char* messageText) {
+  std::ostringstream message;
+  message << messageText;
+  THROW_ERR_OUT_OF_RANGE(env, message.str().c_str());
+}
+
 inline v8::Local<v8::Value> ERR_BUFFER_TOO_LARGE(v8::Isolate* isolate) {
   char message[128];
   snprintf(message, sizeof(message),
diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js
index 023852b69a850a..5088e3c06ff471 100644
--- a/test/parallel/test-buffer-alloc.js
+++ b/test/parallel/test-buffer-alloc.js
@@ -1011,9 +1011,8 @@ common.expectsError(() => {
   const b = Buffer.alloc(1);
   a.copy(b, 0, 0x100000000, 0x100000001);
 }, {
-  code: 'ERR_INDEX_OUT_OF_RANGE',
-  type: RangeError,
-  message: 'Index out of range'
+  code: 'ERR_OUT_OF_RANGE',
+  type: RangeError
 });
 
 // Unpooled buffer (replaces SlowBuffer)
diff --git a/test/parallel/test-buffer-compare-offset.js b/test/parallel/test-buffer-compare-offset.js
index 7f3121bef17ab5..0fee87d71062fa 100644
--- a/test/parallel/test-buffer-compare-offset.js
+++ b/test/parallel/test-buffer-compare-offset.js
@@ -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 = common.expectsError({ code: 'ERR_INDEX_OUT_OF_RANGE' }, 7);
+const oor = common.expectsError({ code: 'ERR_OUT_OF_RANGE' }, 7);
 
 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
index 4eef0edefcbb22..3daaa91d8f1bad 100644
--- a/test/parallel/test-buffer-fill.js
+++ b/test/parallel/test-buffer-fill.js
@@ -2,7 +2,7 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const { codes: { ERR_INDEX_OUT_OF_RANGE } } = require('internal/errors');
+const { codes: { ERR_OUT_OF_RANGE } } = require('internal/errors');
 const SIZE = 28;
 
 const buf1 = Buffer.allocUnsafe(SIZE);
@@ -173,7 +173,7 @@ deepStrictEqualValues(genBuffer(4, [hexBufFill, 1, 1]), [0, 0, 0, 0]);
 ].forEach((args) => {
   common.expectsError(
     () => buf1.fill(...args),
-    { code: 'ERR_INDEX_OUT_OF_RANGE' }
+    { code: 'ERR_OUT_OF_RANGE' }
   );
 });
 
@@ -237,7 +237,7 @@ function writeToFill(string, offset, end, encoding) {
 
   // Should never be reached.
   if (offset < 0 || end > buf2.length)
-    throw new ERR_INDEX_OUT_OF_RANGE();
+    throw new ERR_OUT_OF_RANGE();
 
   if (end <= offset)
     return buf2;
@@ -276,10 +276,10 @@ function testBufs(string, offset, length, encoding) {
 // Make sure these throw.
 common.expectsError(
   () => Buffer.allocUnsafe(8).fill('a', -1),
-  { code: 'ERR_INDEX_OUT_OF_RANGE' });
+  { code: 'ERR_OUT_OF_RANGE' });
 common.expectsError(
   () => Buffer.allocUnsafe(8).fill('a', 0, 9),
-  { code: 'ERR_INDEX_OUT_OF_RANGE' });
+  { code: 'ERR_OUT_OF_RANGE' });
 
 // Make sure this doesn't hang indefinitely.
 Buffer.allocUnsafe(8).fill('');
@@ -333,6 +333,9 @@ assert.strictEqual(
 // Symbol.toPrimitive.
 {
   let elseWasLast = false;
+  const expectedErrorMessage =
+    'The value of "end" is out of range. It must be >= 0 and <= 1. Received -1';
+
   common.expectsError(() => {
     let ctr = 0;
     const end = {
@@ -350,9 +353,9 @@ assert.strictEqual(
     };
     Buffer.alloc(1).fill(Buffer.alloc(1), 0, end);
   }, {
-    code: 'ERR_INDEX_OUT_OF_RANGE',
+    code: 'ERR_OUT_OF_RANGE',
     type: RangeError,
-    message: 'Index out of range'
+    message: expectedErrorMessage
   });
   // Make sure -1 is making it to Buffer::Fill().
   assert.ok(elseWasLast,
@@ -373,9 +376,9 @@ common.expectsError(() => {
   });
   buf.fill('');
 }, {
-  code: 'ERR_INDEX_OUT_OF_RANGE',
+  code: 'ERR_BUFFER_OUT_OF_BOUNDS',
   type: RangeError,
-  message: 'Index out of range'
+  message: 'Attempt to write outside buffer bounds'
 });
 
 assert.deepStrictEqual(