Skip to content

Commit

Permalink
buffer: consistent error for length > kMaxLength
Browse files Browse the repository at this point in the history
- 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: nodejs#10152
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
joyeecheung authored and AnnaMag committed Dec 13, 2016
1 parent db5d910 commit f06375b
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 26 deletions.
4 changes: 4 additions & 0 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ function assertSize(size) {
err = new TypeError('"size" argument must be a number');
else if (size < 0)
err = new RangeError('"size" argument must not be negative');
else if (size > binding.kMaxLength)
err = new RangeError('"size" argument must not be larger ' +
'than ' + binding.kMaxLength);

if (err) {
// The following hides the 'assertSize' method from the
Expand Down Expand Up @@ -167,6 +170,7 @@ Buffer.allocUnsafeSlow = function(size) {
function SlowBuffer(length) {
if (+length != length)
length = 0;
assertSize(+length);
return createUnsafeBuffer(+length);
}

Expand Down
5 changes: 4 additions & 1 deletion test/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const assert = require('assert');
const os = require('os');
const child_process = require('child_process');
const stream = require('stream');
const buffer = require('buffer');
const util = require('util');
const Timer = process.binding('timer_wrap').Timer;

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

exports.enoughTestMem = os.totalmem() > 0x40000000; /* 1 Gb */

exports.bufferMaxSizeMsg = new RegExp('^RangeError: "size" argument' +
' must not be larger than ' +
buffer.kMaxLength + '$');
const cpus = os.cpus();
exports.enoughTestCpu = Array.isArray(cpus) &&
(cpus.length > 1 || cpus[0].speed > 999);
Expand Down
14 changes: 4 additions & 10 deletions test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ const common = require('../common');
const assert = require('assert');
const vm = require('vm');

const Buffer = require('buffer').Buffer;
const SlowBuffer = require('buffer').SlowBuffer;
const buffer = require('buffer');
const Buffer = buffer.Buffer;
const SlowBuffer = buffer.SlowBuffer;


const b = Buffer.allocUnsafe(1024);
assert.strictEqual(1024, b.length);
Expand Down Expand Up @@ -791,10 +793,6 @@ Buffer.from(Buffer.allocUnsafe(0), 0, 0);
assert(buf.equals(copy));
}

// issue GH-4331
assert.throws(() => Buffer.allocUnsafe(0xFFFFFFFF), RangeError);
assert.throws(() => Buffer.allocUnsafe(0xFFFFFFFFF), RangeError);

// issue GH-5587
assert.throws(() => Buffer.alloc(8).writeFloatLE(0, 5), RangeError);
assert.throws(() => Buffer.alloc(16).writeDoubleLE(0, 9), RangeError);
Expand Down Expand Up @@ -1002,10 +1000,6 @@ assert.throws(() => Buffer.from('', 'buffer'), TypeError);
}
}

assert.throws(() => Buffer.allocUnsafe((-1 >>> 0) + 1), RangeError);
assert.throws(() => Buffer.allocUnsafeSlow((-1 >>> 0) + 1), RangeError);
assert.throws(() => SlowBuffer((-1 >>> 0) + 1), RangeError);

if (common.hasCrypto) {
// Test truncation after decode
const crypto = require('crypto');
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-buffer-over-max-length.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';
const common = require('../common');
const assert = require('assert');

const buffer = require('buffer');
const Buffer = buffer.Buffer;
const SlowBuffer = buffer.SlowBuffer;

const kMaxLength = buffer.kMaxLength;
const bufferMaxSizeMsg = common.bufferMaxSizeMsg;

assert.throws(() => Buffer((-1 >>> 0) + 1), bufferMaxSizeMsg);
assert.throws(() => SlowBuffer((-1 >>> 0) + 1), bufferMaxSizeMsg);
assert.throws(() => Buffer.alloc((-1 >>> 0) + 1), bufferMaxSizeMsg);
assert.throws(() => Buffer.allocUnsafe((-1 >>> 0) + 1), bufferMaxSizeMsg);
assert.throws(() => Buffer.allocUnsafeSlow((-1 >>> 0) + 1), bufferMaxSizeMsg);

assert.throws(() => Buffer(kMaxLength + 1), bufferMaxSizeMsg);
assert.throws(() => SlowBuffer(kMaxLength + 1), bufferMaxSizeMsg);
assert.throws(() => Buffer.alloc(kMaxLength + 1), bufferMaxSizeMsg);
assert.throws(() => Buffer.allocUnsafe(kMaxLength + 1), bufferMaxSizeMsg);
assert.throws(() => Buffer.allocUnsafeSlow(kMaxLength + 1), bufferMaxSizeMsg);

// issue GH-4331
assert.throws(() => Buffer.allocUnsafe(0xFFFFFFFF), bufferMaxSizeMsg);
assert.throws(() => Buffer.allocUnsafe(0xFFFFFFFFF), bufferMaxSizeMsg);
18 changes: 7 additions & 11 deletions test/parallel/test-buffer-regression-649.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
'use strict';

require('../common');
const common = require('../common');
const assert = require('assert');
const SlowBuffer = require('buffer').SlowBuffer;

// Regression test for https://github.com/nodejs/node/issues/649.
const len = 1422561062959;
const lenLimitMsg = new RegExp('^RangeError: (Invalid typed array length' +
'|Array buffer allocation failed' +
'|Invalid array buffer length)$');

assert.throws(() => Buffer(len).toString('utf8'), lenLimitMsg);
assert.throws(() => SlowBuffer(len).toString('utf8'), lenLimitMsg);
assert.throws(() => Buffer.alloc(len).toString('utf8'), lenLimitMsg);
assert.throws(() => Buffer.allocUnsafe(len).toString('utf8'), lenLimitMsg);
assert.throws(() => Buffer.allocUnsafeSlow(len).toString('utf8'),
lenLimitMsg);
const message = common.bufferMaxSizeMsg;
assert.throws(() => Buffer(len).toString('utf8'), message);
assert.throws(() => SlowBuffer(len).toString('utf8'), message);
assert.throws(() => Buffer.alloc(len).toString('utf8'), message);
assert.throws(() => Buffer.allocUnsafe(len).toString('utf8'), message);
assert.throws(() => Buffer.allocUnsafeSlow(len).toString('utf8'), message);
8 changes: 4 additions & 4 deletions test/parallel/test-buffer-slow.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 buffer = require('buffer');
const Buffer = buffer.Buffer;
Expand Down Expand Up @@ -51,10 +51,10 @@ assert.strictEqual(SlowBuffer('string').length, 0);
// should throw with invalid length
assert.throws(function() {
SlowBuffer(Infinity);
}, /^RangeError: Invalid array buffer length$/);
}, common.bufferMaxSizeMsg);
assert.throws(function() {
SlowBuffer(-1);
}, /^RangeError: Invalid array buffer length$/);
}, /^RangeError: "size" argument must not be negative$/);
assert.throws(function() {
SlowBuffer(buffer.kMaxLength + 1);
}, /^RangeError: (Invalid typed array length|Array buffer allocation failed)$/);
}, common.bufferMaxSizeMsg);

0 comments on commit f06375b

Please sign in to comment.