Skip to content

Commit

Permalink
buffer: don't set zero fill for zero-length buffer
Browse files Browse the repository at this point in the history
Instantiating a Buffer of length zero would set the kNoZeroFill flag to
true but never actually call ArrayBuffer::Allocator(). Which means the
flag was never set back to false. The result was that the next
allocation would unconditionally not be zero filled.

Add test to ensure Uint8Array's are zero-filled after creating a Buffer
of length zero. This test may falsely succeed, but will not falsely fail.

Fix: #2930
PR-URL: #2931
Reviewed-By: Rod Vagg <rod@vagg.org>
  • Loading branch information
trevnorris committed Sep 18, 2015
1 parent e0c3d2a commit 0a329d2
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
19 changes: 14 additions & 5 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ const kNoZeroFill = 0;

function createPool() {
poolSize = Buffer.poolSize;
flags[kNoZeroFill] = 1;
if (poolSize > 0)
flags[kNoZeroFill] = 1;
allocPool = new Uint8Array(poolSize);
Object.setPrototypeOf(allocPool, Buffer.prototype);
poolOffset = 0;
Expand Down Expand Up @@ -64,7 +65,8 @@ Buffer.__proto__ = Uint8Array;
function SlowBuffer(length) {
if (+length != length)
length = 0;
flags[kNoZeroFill] = 1;
if (length > 0)
flags[kNoZeroFill] = 1;
const ui8 = new Uint8Array(+length);
Object.setPrototypeOf(ui8, Buffer.prototype);
return ui8;
Expand All @@ -75,8 +77,11 @@ SlowBuffer.__proto__ = Buffer;


function allocate(size) {
if (size === 0)
return SlowBuffer(0);
if (size === 0) {
const ui8 = new Uint8Array(size);
Object.setPrototypeOf(ui8, Buffer.prototype);
return ui8;
}
if (size < (Buffer.poolSize >>> 1)) {
if (size > (poolSize - poolOffset))
createPool();
Expand All @@ -85,7 +90,11 @@ function allocate(size) {
alignPool();
return b;
} else {
flags[kNoZeroFill] = 1;
// Even though this is checked above, the conditional is a safety net and
// sanity check to prevent any subsequent typed array allocation from not
// being zero filled.
if (size > 0)
flags[kNoZeroFill] = 1;
const ui8 = new Uint8Array(size);
Object.setPrototypeOf(ui8, Buffer.prototype);
return ui8;
Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-buffer-zero-fill-reset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

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


function testUint8Array(ui) {
const length = ui.length;
for (let i = 0; i < length; i++)
if (ui[i] !== 0) return false;
return true;
}


for (let i = 0; i < 100; i++) {
new Buffer(0);
let ui = new Uint8Array(65);
assert.ok(testUint8Array(ui), 'Uint8Array is not zero-filled');
}

1 comment on commit 0a329d2

@kzc
Copy link

@kzc kzc commented on 0a329d2 Sep 19, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The title of this PR should be "don't set no-zero-fill for zero-length buffer".

Not a fan of negative variable names.

Please sign in to comment.