From c31be4298c7bc4e4291b8322a8f8e37402dbc69c Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Fri, 21 Oct 2016 22:22:37 -0400 Subject: [PATCH 1/5] buffer: fix `fill` with encoding in Buffer.alloc() Previously, the implementation of Buffer.alloc() called Buffer#fill() with another Buffer as an argument. However, in v4.x, Buffer#fill does not support a Buffer as a parameter. As a workaround, call binding.fill() directly in the Buffer.alloc() implementation. Fixes: https://github.com/nodejs/node/issues/9226 --- lib/buffer.js | 6 +++++- test/parallel/test-buffer-alloc.js | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/buffer.js b/lib/buffer.js index 8efb4cd789492b..0a1891f2951b7d 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -105,7 +105,11 @@ Buffer.alloc = function(size, fill, encoding) { if (typeof fill !== 'number' && typeof encoding === 'string') fill = Buffer.from(fill, encoding); - return createBuffer(size, true).fill(fill); + const buf = createBuffer(size, true); + // Buffer.prototype.fill does not support filling with other buffers in v4. + // Instead, call binding.fill directly. + binding.fill(buf, fill, 0, buf.length); + return buf; } return createBuffer(size); }; diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index a90625e163c5cc..f80f317275f09d 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -1060,6 +1060,17 @@ assert.throws(function() { Buffer.allocUnsafe(0xFFFFFFFFF); }, RangeError); +// issue GH-9226 +{ + const buf = Buffer.alloc(4, 'YQ==', 'base64'); + const expectedBuf = new Buffer([97, 97, 97, 97]); + assert(buf.equals(expectedBuf)); +} +{ + const buf = Buffer.alloc(4, 'ab', 'ascii'); + const expectedBuf = new Buffer([97, 98, 97, 98]); + assert(buf.equals(expectedBuf)); +} // attempt to overflow buffers, similar to previous bug in array buffers assert.throws(function() { From 275b17b7bb71f5228d07273de2a111190cce70a2 Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Fri, 21 Oct 2016 23:15:05 -0400 Subject: [PATCH 2/5] squash: use Buffer.from --- test/parallel/test-buffer-alloc.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index f80f317275f09d..5f8d88e9a09475 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -1063,12 +1063,12 @@ assert.throws(function() { // issue GH-9226 { const buf = Buffer.alloc(4, 'YQ==', 'base64'); - const expectedBuf = new Buffer([97, 97, 97, 97]); + const expectedBuf = Buffer.from([97, 97, 97, 97]); assert(buf.equals(expectedBuf)); } { const buf = Buffer.alloc(4, 'ab', 'ascii'); - const expectedBuf = new Buffer([97, 98, 97, 98]); + const expectedBuf = Buffer.from([97, 98, 97, 98]); assert(buf.equals(expectedBuf)); } From 54c9dad545f8eb01e74815d74d60237e339ba529 Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Fri, 21 Oct 2016 23:15:23 -0400 Subject: [PATCH 3/5] squash: link to GitHub issue in comment --- test/parallel/test-buffer-alloc.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index 5f8d88e9a09475..54acde73f5c0e8 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -1060,7 +1060,7 @@ assert.throws(function() { Buffer.allocUnsafe(0xFFFFFFFFF); }, RangeError); -// issue GH-9226 +// https://github.com/nodejs/node/issues/9226 { const buf = Buffer.alloc(4, 'YQ==', 'base64'); const expectedBuf = Buffer.from([97, 97, 97, 97]); From 90af46a74f5d26c6048b1ed581f7a2e96f38f74a Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Sat, 22 Oct 2016 00:23:33 -0400 Subject: [PATCH 4/5] squash: ensure Buffer.alloc is inlineable --- lib/buffer.js | 12 ++++++++---- test/parallel/test-buffer-alloc.js | 2 ++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 0a1891f2951b7d..592f57396c8989 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -91,6 +91,14 @@ Object.setPrototypeOf(Buffer, Uint8Array); /** * Creates a new filled Buffer instance. * alloc(size[, fill[, encoding]]) + * + * Only pay attention to encoding if it's a string. This + * prevents accidentally sending in a number that would + * be interpretted as a start offset. + * Also, don't apply encoding if fill is a number. + * + * These comments are placed before the function to keep the text length + * down, to ensure that it remains inlineable by V8. **/ Buffer.alloc = function(size, fill, encoding) { if (typeof size !== 'number') @@ -98,10 +106,6 @@ Buffer.alloc = function(size, fill, encoding) { if (size <= 0) return createBuffer(size); if (fill !== undefined) { - // Only pay attention to encoding if it's a string. This - // prevents accidentally sending in a number that would - // be interpretted as a start offset. - // Also, don't apply encoding if fill is a number. if (typeof fill !== 'number' && typeof encoding === 'string') fill = Buffer.from(fill, encoding); diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index 54acde73f5c0e8..b2ca5b96e661ad 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -1060,6 +1060,8 @@ assert.throws(function() { Buffer.allocUnsafe(0xFFFFFFFFF); }, RangeError); +assert(Buffer.alloc.toString().length < 600, 'Buffer.alloc is not inlineable'); + // https://github.com/nodejs/node/issues/9226 { const buf = Buffer.alloc(4, 'YQ==', 'base64'); From baad1f67e9262921fdfbb224a794524a1cee8bb3 Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Sat, 22 Oct 2016 01:33:03 -0400 Subject: [PATCH 5/5] squash: fix comment typo --- lib/buffer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/buffer.js b/lib/buffer.js index 592f57396c8989..7bad03939f5e5e 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -94,7 +94,7 @@ Object.setPrototypeOf(Buffer, Uint8Array); * * Only pay attention to encoding if it's a string. This * prevents accidentally sending in a number that would - * be interpretted as a start offset. + * be interpreted as a start offset. * Also, don't apply encoding if fill is a number. * * These comments are placed before the function to keep the text length