Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/src: refactoring out of range index and and migrate error for consistency #11296

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
15 changes: 8 additions & 7 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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');
}


Expand Down Expand Up @@ -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');
}


Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
8 changes: 4 additions & 4 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-buffer-compare-offset.js
Original file line number Diff line number Diff line change
@@ -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]);
Expand Down Expand Up @@ -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);
Expand Down
77 changes: 44 additions & 33 deletions test/parallel/test-buffer-fill.js
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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('');
Expand Down Expand Up @@ -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');
Expand All @@ -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.
Expand All @@ -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');
Expand All @@ -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(() => {
Expand All @@ -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'),
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-buffer-read.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');

// testing basic buffer read functions
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-buffer-write-noassert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
{
Expand Down