buffer: stricter buffer from

doc/api/buffer.md

@@ -884,15 +884,18 @@ appropriate for `Buffer.from()` variants.
 
 ### Class Method: `Buffer.from(object[, offsetOrEncoding[, length]])`
 <!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/27051
+    description: Dropped support for `Symbol.toPrimitive`.
 added: v8.2.0
 -->
 
-* `object` {Object} An object supporting `Symbol.toPrimitive` or `valueOf()`.
+* `object` {Object} An object supporting `valueOf()`.
 * `offsetOrEncoding` {integer|string} A byte-offset or encoding, depending on
-  the value returned either by `object.valueOf()` or
-  `object[Symbol.toPrimitive]()`.
-* `length` {integer} A length, depending on the value returned either by
-  `object.valueOf()` or `object[Symbol.toPrimitive]()`.
+  the value returned by `object.valueOf()`.
+* `length` {integer} A length, depending on the value returned by
+  `object.valueOf()`.
 
 For objects whose `valueOf()` function returns a value not strictly equal to
 `object`, returns `Buffer.from(object.valueOf(), offsetOrEncoding, length)`.
@@ -902,20 +905,6 @@ const buf = Buffer.from(new String('this is a test'));
 // Prints: <Buffer 74 68 69 73 20 69 73 20 61 20 74 65 73 74>
 ```
 
-For objects that support `Symbol.toPrimitive`, returns
-`Buffer.from(object[Symbol.toPrimitive](), offsetOrEncoding, length)`.
-
-```js
-class Foo {
-  [Symbol.toPrimitive]() {
-    return 'this is a test';
-  }
-}
-
-const buf = Buffer.from(new Foo(), 'utf8');
-// Prints: <Buffer 74 68 69 73 20 69 73 20 61 20 74 65 73 74>
-```
-
 A `TypeError` will be thrown if `object` has not mentioned methods or is not of
 other type appropriate for `Buffer.from()` variants.
 

lib/buffer.js

@@ -28,6 +28,7 @@ const {
   MathFloor,
   MathMin,
   MathTrunc,
+  NumberIsInteger,
   NumberIsNaN,
   NumberMAX_SAFE_INTEGER,
   NumberMIN_SAFE_INTEGER,
@@ -453,7 +454,7 @@ function fromArrayBuffer(obj, byteOffset, length) {
   } else {
     byteOffset = +byteOffset;
     if (NumberIsNaN(byteOffset))
-      byteOffset = 0;
+      throw new ERR_INVALID_ARG_VALUE('byteOffset', byteOffset);
   }
 
   const maxLength = obj.byteLength - byteOffset;
@@ -488,10 +489,8 @@ function fromObject(obj) {
     return b;
   }
 
-  if (obj.length !== undefined || isAnyArrayBuffer(obj.buffer)) {
-    if (typeof obj.length !== 'number') {
-      return new FastBuffer();
-    }
+  if ((NumberIsInteger(obj.length) && obj.length >= 0) ||
+      isAnyArrayBuffer(obj.buffer)) {
     return fromArrayLike(obj);
   }
 

test/parallel/test-buffer-alloc.js

@@ -769,8 +769,14 @@ Buffer.allocUnsafe(3.3).fill().toString();
 // Throws bad argument error in commit 43cb4ec
 Buffer.alloc(3.3).fill().toString();
 assert.strictEqual(Buffer.allocUnsafe(3.3).length, 3);
-assert.strictEqual(Buffer.from({ length: 3.3 }).length, 3);
-assert.strictEqual(Buffer.from({ length: 'BAM' }).length, 0);
+assert.throws(
+  () => Buffer.from({ length: 3.3 }),
+  { code: 'ERR_INVALID_ARG_TYPE' }
+);
+assert.throws(
+  () => Buffer.from({ length: 'BAM' }),
+  { code: 'ERR_INVALID_ARG_TYPE' }
+);
 
 // Make sure that strings are not coerced to numbers.
 assert.strictEqual(Buffer.from('99').length, 2);
@@ -993,10 +999,10 @@ assert.strictEqual(SlowBuffer.prototype.offset, undefined);
   // Test that large negative Buffer length inputs don't affect the pool offset.
   // Use the fromArrayLike() variant here because it's more lenient
   // about its input and passes the length directly to allocate().
-  assert.deepStrictEqual(Buffer.from({ length: -Buffer.poolSize }),
-                         Buffer.from(''));
-  assert.deepStrictEqual(Buffer.from({ length: -100 }),
-                         Buffer.from(''));
+  assert.throws(
+    () => Buffer.from({ length: -Buffer.poolSize }),
+    { code: 'ERR_INVALID_ARG_TYPE' }
+  );
 
   // Check pool offset after that by trying to write string into the pool.
   Buffer.from('abc');

test/parallel/test-buffer-arraybuffer.js

@@ -107,11 +107,12 @@ assert.throws(function() {
 {
   // If byteOffset is not numeric, it defaults to 0.
   const ab = new ArrayBuffer(10);
-  const expected = Buffer.from(ab, 0);
-  assert.deepStrictEqual(Buffer.from(ab, 'fhqwhgads'), expected);
-  assert.deepStrictEqual(Buffer.from(ab, NaN), expected);
-  assert.deepStrictEqual(Buffer.from(ab, {}), expected);
-  assert.deepStrictEqual(Buffer.from(ab, []), expected);
+  ['fhqwhgads', NaN, {}].forEach((invalidByteOffset) => {
+    assert.throws(
+      () => Buffer.from(ab, invalidByteOffset),
+      { code: 'ERR_INVALID_ARG_VALUE' }
+    );
+  });
 
   // If byteOffset can be converted to a number, it will be.
   assert.deepStrictEqual(Buffer.from(ab, [1]), Buffer.from(ab, 1));
@@ -149,4 +150,7 @@ assert.throws(function() {
 }
 
 // Test an array like entry with the length set to NaN.
-assert.deepStrictEqual(Buffer.from({ length: NaN }), Buffer.alloc(0));
+assert.throws(
+  () => Buffer.from({ length: NaN }),
+  { code: 'ERR_INVALID_ARG_TYPE' }
+);