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: https://github.com/nodejs/node/pull/10152
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/buffer.js

@@ -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
@@ -167,6 +170,7 @@ Buffer.allocUnsafeSlow = function(size) {
 function SlowBuffer(length) {
   if (+length != length)
     length = 0;
+  assertSize(+length);
   return createUnsafeBuffer(+length);
 }
 

test/common.js

@@ -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;
 
@@ -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);

test/parallel/test-buffer-alloc.js

@@ -3,8 +3,10 @@ const common = require('../common');
 const assert = require('assert');
 const vm = require('vm');
 
-const Buffer = require('buffer').Buffer;
+const buffer = require('buffer');
-const SlowBuffer = require('buffer').SlowBuffer;
+const Buffer = buffer.Buffer;
+const SlowBuffer = buffer.SlowBuffer;
+
 
 const b = Buffer.allocUnsafe(1024);
 assert.strictEqual(1024, b.length);
@@ -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);
@@ -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');

test/parallel/test-buffer-over-max-length.js

@@ -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);

test/parallel/test-buffer-regression-649.js

@@ -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' +
+const message = common.bufferMaxSizeMsg;
-                               '|Array buffer allocation failed' +
+assert.throws(() => Buffer(len).toString('utf8'), message);
-                               '|Invalid array buffer length)$');
+assert.throws(() => SlowBuffer(len).toString('utf8'), message);
-
+assert.throws(() => Buffer.alloc(len).toString('utf8'), message);
-assert.throws(() => Buffer(len).toString('utf8'), lenLimitMsg);
+assert.throws(() => Buffer.allocUnsafe(len).toString('utf8'), message);
-assert.throws(() => SlowBuffer(len).toString('utf8'), lenLimitMsg);
+assert.throws(() => Buffer.allocUnsafeSlow(len).toString('utf8'), message);
-assert.throws(() => Buffer.alloc(len).toString('utf8'), lenLimitMsg);
-assert.throws(() => Buffer.allocUnsafe(len).toString('utf8'), lenLimitMsg);
-assert.throws(() => Buffer.allocUnsafeSlow(len).toString('utf8'),
-              lenLimitMsg);

test/parallel/test-buffer-slow.js

@@ -1,6 +1,6 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const buffer = require('buffer');
 const Buffer = buffer.Buffer;
@@ -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);