Skip to content

Commit

Permalink
buffer: remove support for unknown objects
Browse files Browse the repository at this point in the history
So far objects with a `length` property that was not an integer or
positive resulted in an empty buffer being created. That is not ideal
as it's not really clear what the intention in was.
This throws an error from now on instead.
  • Loading branch information
BridgeAR committed Jan 12, 2020
1 parent 0abf9b0 commit 1ed3193
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 11 deletions.
7 changes: 3 additions & 4 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {
MathFloor,
MathMin,
MathTrunc,
NumberIsInteger,
NumberIsNaN,
NumberMAX_SAFE_INTEGER,
NumberMIN_SAFE_INTEGER,
Expand Down Expand Up @@ -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);
}

Expand Down
18 changes: 12 additions & 6 deletions test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-buffer-arraybuffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,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' }
);

0 comments on commit 1ed3193

Please sign in to comment.