Skip to content

Commit

Permalink
buffer: SlowBuffer only accept valid numeric values
Browse files Browse the repository at this point in the history
Fixes a regression that appeared with the new Buffer implementation in v3.
Without this change, calling the SlowBuffer constructor with something else
than a number would abort on the C++ side. This makes sure that the length
argument is coerced to number or is 0.

Fixes: #2634
PR-URL: #2635
Reviewed-By: trevnorris - Trevor Norris <[email protected]>
Reviewed-By: cjihrig - Colin Ihrig <[email protected]>
  • Loading branch information
targos authored and rvagg committed Sep 6, 2015
1 parent 384effe commit d3178d8
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ createPool();


function SlowBuffer(length) {
if (length < 0)
if (+length != length)
length = 0;
return binding.create(length);
return binding.create(+length);
};

SlowBuffer.prototype.__proto__ = Buffer.prototype;
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-buffer-slow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use strict';

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

const ones = [1, 1, 1, 1];

// should create a Buffer
let sb = new SlowBuffer(4);
assert(sb instanceof Buffer);
assert.strictEqual(sb.length, 4);
sb.fill(1);
assert.deepEqual(sb, ones);

// underlying ArrayBuffer should have the same length
assert.strictEqual(sb.buffer.byteLength, 4);

// should work without new
sb = SlowBuffer(4);
assert(sb instanceof Buffer);
assert.strictEqual(sb.length, 4);
sb.fill(1);
assert.deepEqual(sb, ones);

// should work with edge cases
assert.strictEqual(SlowBuffer(0).length, 0);
try {
assert.strictEqual(SlowBuffer(buffer.kMaxLength).length, buffer.kMaxLength);
} catch (e) {
assert.equal(e.message, 'Buffer allocation failed - process out of memory');
}

// should work with number-coercible values
assert.strictEqual(SlowBuffer('6').length, 6);
assert.strictEqual(SlowBuffer(true).length, 1);

// should create zero-length buffer if parameter is not a number
assert.strictEqual(SlowBuffer().length, 0);
assert.strictEqual(SlowBuffer(NaN).length, 0);
assert.strictEqual(SlowBuffer({}).length, 0);
assert.strictEqual(SlowBuffer('string').length, 0);

// should throw with invalid length
assert.throws(function() {
new SlowBuffer(Infinity);
}, 'invalid Buffer length');
assert.throws(function() {
new SlowBuffer(-1);
}, 'invalid Buffer length');
assert.throws(function() {
new SlowBuffer(buffer.kMaxLength + 1);
}, 'invalid Buffer length');

0 comments on commit d3178d8

Please sign in to comment.