Skip to content

Commit 3d353c7

Browse files
joyeecheungaddaleax
authored andcommitted
buffer: consistent error for length > kMaxLength
- Always return the same error message(hopefully more informative) for buffer length > kMaxLength and avoid getting into V8 C++ land for unnecessary checks. - Use accurate RegExp(reusable as `common.bufferMaxSizeMsg`) in tests for this error. - Separate related tests from test-buffer-alloc. PR-URL: #10152 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 8329605 commit 3d353c7

6 files changed

+49
-26
lines changed

lib/buffer.js

+4
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ function assertSize(size) {
115115
err = new TypeError('"size" argument must be a number');
116116
else if (size < 0)
117117
err = new RangeError('"size" argument must not be negative');
118+
else if (size > binding.kMaxLength)
119+
err = new RangeError('"size" argument must not be larger ' +
120+
'than ' + binding.kMaxLength);
118121

119122
if (err) {
120123
// The following hides the 'assertSize' method from the
@@ -167,6 +170,7 @@ Buffer.allocUnsafeSlow = function(size) {
167170
function SlowBuffer(length) {
168171
if (+length != length)
169172
length = 0;
173+
assertSize(+length);
170174
return createUnsafeBuffer(+length);
171175
}
172176

test/common.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const assert = require('assert');
66
const os = require('os');
77
const child_process = require('child_process');
88
const stream = require('stream');
9+
const buffer = require('buffer');
910
const util = require('util');
1011
const Timer = process.binding('timer_wrap').Timer;
1112

@@ -29,7 +30,9 @@ exports.isLinux = process.platform === 'linux';
2930
exports.isOSX = process.platform === 'darwin';
3031

3132
exports.enoughTestMem = os.totalmem() > 0x40000000; /* 1 Gb */
32-
33+
exports.bufferMaxSizeMsg = new RegExp('^RangeError: "size" argument' +
34+
' must not be larger than ' +
35+
buffer.kMaxLength + '$');
3336
const cpus = os.cpus();
3437
exports.enoughTestCpu = Array.isArray(cpus) &&
3538
(cpus.length > 1 || cpus[0].speed > 999);

test/parallel/test-buffer-alloc.js

+4-10
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ const common = require('../common');
33
const assert = require('assert');
44
const vm = require('vm');
55

6-
const Buffer = require('buffer').Buffer;
7-
const SlowBuffer = require('buffer').SlowBuffer;
6+
const buffer = require('buffer');
7+
const Buffer = buffer.Buffer;
8+
const SlowBuffer = buffer.SlowBuffer;
9+
810

911
const b = Buffer.allocUnsafe(1024);
1012
assert.strictEqual(1024, b.length);
@@ -791,10 +793,6 @@ Buffer.from(Buffer.allocUnsafe(0), 0, 0);
791793
assert(buf.equals(copy));
792794
}
793795

794-
// issue GH-4331
795-
assert.throws(() => Buffer.allocUnsafe(0xFFFFFFFF), RangeError);
796-
assert.throws(() => Buffer.allocUnsafe(0xFFFFFFFFF), RangeError);
797-
798796
// issue GH-5587
799797
assert.throws(() => Buffer.alloc(8).writeFloatLE(0, 5), RangeError);
800798
assert.throws(() => Buffer.alloc(16).writeDoubleLE(0, 9), RangeError);
@@ -1002,10 +1000,6 @@ assert.throws(() => Buffer.from('', 'buffer'), TypeError);
10021000
}
10031001
}
10041002

1005-
assert.throws(() => Buffer.allocUnsafe((-1 >>> 0) + 1), RangeError);
1006-
assert.throws(() => Buffer.allocUnsafeSlow((-1 >>> 0) + 1), RangeError);
1007-
assert.throws(() => SlowBuffer((-1 >>> 0) + 1), RangeError);
1008-
10091003
if (common.hasCrypto) {
10101004
// Test truncation after decode
10111005
const crypto = require('crypto');
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
5+
const buffer = require('buffer');
6+
const Buffer = buffer.Buffer;
7+
const SlowBuffer = buffer.SlowBuffer;
8+
9+
const kMaxLength = buffer.kMaxLength;
10+
const bufferMaxSizeMsg = common.bufferMaxSizeMsg;
11+
12+
assert.throws(() => Buffer((-1 >>> 0) + 1), bufferMaxSizeMsg);
13+
assert.throws(() => SlowBuffer((-1 >>> 0) + 1), bufferMaxSizeMsg);
14+
assert.throws(() => Buffer.alloc((-1 >>> 0) + 1), bufferMaxSizeMsg);
15+
assert.throws(() => Buffer.allocUnsafe((-1 >>> 0) + 1), bufferMaxSizeMsg);
16+
assert.throws(() => Buffer.allocUnsafeSlow((-1 >>> 0) + 1), bufferMaxSizeMsg);
17+
18+
assert.throws(() => Buffer(kMaxLength + 1), bufferMaxSizeMsg);
19+
assert.throws(() => SlowBuffer(kMaxLength + 1), bufferMaxSizeMsg);
20+
assert.throws(() => Buffer.alloc(kMaxLength + 1), bufferMaxSizeMsg);
21+
assert.throws(() => Buffer.allocUnsafe(kMaxLength + 1), bufferMaxSizeMsg);
22+
assert.throws(() => Buffer.allocUnsafeSlow(kMaxLength + 1), bufferMaxSizeMsg);
23+
24+
// issue GH-4331
25+
assert.throws(() => Buffer.allocUnsafe(0xFFFFFFFF), bufferMaxSizeMsg);
26+
assert.throws(() => Buffer.allocUnsafe(0xFFFFFFFFF), bufferMaxSizeMsg);
+7-11
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,14 @@
11
'use strict';
22

3-
require('../common');
3+
const common = require('../common');
44
const assert = require('assert');
55
const SlowBuffer = require('buffer').SlowBuffer;
66

77
// Regression test for https://github.com/nodejs/node/issues/649.
88
const len = 1422561062959;
9-
const lenLimitMsg = new RegExp('^RangeError: (Invalid typed array length' +
10-
'|Array buffer allocation failed' +
11-
'|Invalid array buffer length)$');
12-
13-
assert.throws(() => Buffer(len).toString('utf8'), lenLimitMsg);
14-
assert.throws(() => SlowBuffer(len).toString('utf8'), lenLimitMsg);
15-
assert.throws(() => Buffer.alloc(len).toString('utf8'), lenLimitMsg);
16-
assert.throws(() => Buffer.allocUnsafe(len).toString('utf8'), lenLimitMsg);
17-
assert.throws(() => Buffer.allocUnsafeSlow(len).toString('utf8'),
18-
lenLimitMsg);
9+
const message = common.bufferMaxSizeMsg;
10+
assert.throws(() => Buffer(len).toString('utf8'), message);
11+
assert.throws(() => SlowBuffer(len).toString('utf8'), message);
12+
assert.throws(() => Buffer.alloc(len).toString('utf8'), message);
13+
assert.throws(() => Buffer.allocUnsafe(len).toString('utf8'), message);
14+
assert.throws(() => Buffer.allocUnsafeSlow(len).toString('utf8'), message);

test/parallel/test-buffer-slow.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
require('../common');
3+
const common = require('../common');
44
const assert = require('assert');
55
const buffer = require('buffer');
66
const Buffer = buffer.Buffer;
@@ -51,10 +51,10 @@ assert.strictEqual(SlowBuffer('string').length, 0);
5151
// should throw with invalid length
5252
assert.throws(function() {
5353
SlowBuffer(Infinity);
54-
}, /^RangeError: Invalid array buffer length$/);
54+
}, common.bufferMaxSizeMsg);
5555
assert.throws(function() {
5656
SlowBuffer(-1);
57-
}, /^RangeError: Invalid array buffer length$/);
57+
}, /^RangeError: "size" argument must not be negative$/);
5858
assert.throws(function() {
5959
SlowBuffer(buffer.kMaxLength + 1);
60-
}, /^RangeError: (Invalid typed array length|Array buffer allocation failed)$/);
60+
}, common.bufferMaxSizeMsg);

0 commit comments

Comments
 (0)