From d63e02e08dd4217a9e6cdf611e42438b31b0d1be Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Thu, 17 Sep 2015 10:48:16 -0600 Subject: [PATCH] buffer: don't set zero fill for zero-length buffer 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: https://github.com/nodejs/node/issues/2930 PR-URL: https://github.com/nodejs/node/pull/2931 Reviewed-By: Rod Vagg --- lib/buffer.js | 19 ++++++++++++++----- test/parallel/test-buffer-zero-fill-reset.js | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-buffer-zero-fill-reset.js diff --git a/lib/buffer.js b/lib/buffer.js index 26b51a8..fc4f269 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -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; @@ -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; @@ -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(); @@ -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; diff --git a/test/parallel/test-buffer-zero-fill-reset.js b/test/parallel/test-buffer-zero-fill-reset.js new file mode 100644 index 0000000..52203a9 --- /dev/null +++ b/test/parallel/test-buffer-zero-fill-reset.js @@ -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'); +} -- 2.7.4