Skip to content

Commit

Permalink
lib,src: refactor buffer out of range index
Browse files Browse the repository at this point in the history
PR-URL: #11296
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
larissayvette authored and Trott committed May 28, 2017
1 parent 1474b7a commit 234353a
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 50 deletions.
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

This comment has been minimized.

Copy link
@TimothyGu

TimothyGu May 28, 2017

Member

@Trott The error code seems to be out-of-place, as we have added other error codes before the PR was landed but after it waa created.

This comment has been minimized.

Copy link
@Trott

Trott May 29, 2017

Member

Argh, sorry, thanks for noting this, will send a PR to fix.


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

0 comments on commit 234353a

Please sign in to comment.