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

errors: use ERR_OUT_OF_RANGE for index errors #22969

Closed
wants to merge 10 commits into from
9 changes: 6 additions & 3 deletions doc/api/buffer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]]])
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
```
Expand Down Expand Up @@ -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
Expand Down
13 changes: 8 additions & 5 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1087,11 +1087,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

Expand Down Expand Up @@ -1915,6 +1910,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
Expand Down
111 changes: 56 additions & 55 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Trott marked this conversation as resolved.
Show resolved Hide resolved
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`,
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
1 change: 0 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,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);
Expand Down
17 changes: 11 additions & 6 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,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; \
Expand Down Expand Up @@ -497,7 +499,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;
Expand Down Expand Up @@ -687,9 +690,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);
Expand Down
10 changes: 8 additions & 2 deletions src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand All @@ -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) \
Expand Down Expand Up @@ -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, \
Expand Down Expand Up @@ -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),
Expand Down
5 changes: 2 additions & 3 deletions test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-buffer-compare-offset.js
Original file line number Diff line number Diff line change
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 = 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);
Expand Down
Loading