From f4f16bf03980df4d4366697d40e80da805a38bf8 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Wed, 10 Jun 2015 11:06:15 -0600 Subject: [PATCH] buffer: fix case of one buffer passed to concat Fix Buffer.concat() so a copy is always returned regardless of the number of buffers that were passed. Previously if the array length was one then the same same buffer was returned. This created a special case for the user where there was a chance mutating the buffer returned by .concat() could mutate the buffer passed in. Also fixes an inconsistency when throwing if an array member was not a Buffer instance. For example: Buffer.concat([42]); // Returns 42 Buffer.concat([42, 1]); // Throws a TypeError Now .concat() will always throw if an array member is not a Buffer instance. See: https://github.com/nodejs/io.js/issues/1891 PR-URL: https://github.com/nodejs/io.js/pull/1937 Reviewed-By: Trevor Norris Reviewed-By: Colin Ihrig --- doc/api/buffer.markdown | 5 ----- lib/internal/buffer_new.js | 2 -- lib/internal/buffer_old.js | 2 -- test/parallel/test-buffer-concat.js | 8 +++++++- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/doc/api/buffer.markdown b/doc/api/buffer.markdown index 106097451b73d1..c6d648c50db4e0 100644 --- a/doc/api/buffer.markdown +++ b/doc/api/buffer.markdown @@ -130,11 +130,6 @@ the list together. If the list has no items, or if the totalLength is 0, then it returns a zero-length buffer. -If the list has exactly one item, then the first item of the list is -returned. - -If the list has more than one item, then a new Buffer is created. - If totalLength is not provided, it is read from the buffers in the list. However, this adds an additional loop to the function, so it is faster to provide the length explicitly. diff --git a/lib/internal/buffer_new.js b/lib/internal/buffer_new.js index 0b7eba28d8de8d..373f50c7e3211d 100644 --- a/lib/internal/buffer_new.js +++ b/lib/internal/buffer_new.js @@ -183,8 +183,6 @@ Buffer.concat = function(list, length) { if (list.length === 0) return new Buffer(0); - else if (list.length === 1) - return list[0]; if (length === undefined) { length = 0; diff --git a/lib/internal/buffer_old.js b/lib/internal/buffer_old.js index 1b9c68465d6b03..df58bd7a375ba5 100644 --- a/lib/internal/buffer_old.js +++ b/lib/internal/buffer_old.js @@ -249,8 +249,6 @@ Buffer.concat = function(list, length) { if (list.length === 0) return new Buffer(0); - else if (list.length === 1) - return list[0]; if (length === undefined) { length = 0; diff --git a/test/parallel/test-buffer-concat.js b/test/parallel/test-buffer-concat.js index b023dba4cdd3e3..8d0c0eebbd14e6 100644 --- a/test/parallel/test-buffer-concat.js +++ b/test/parallel/test-buffer-concat.js @@ -14,8 +14,14 @@ var flatLongLen = Buffer.concat(long, 40); assert(flatZero.length === 0); assert(flatOne.toString() === 'asdf'); -assert(flatOne === one[0]); +// A special case where concat used to return the first item, +// if the length is one. This check is to make sure that we don't do that. +assert(flatOne !== one[0]); assert(flatLong.toString() === (new Array(10 + 1).join('asdf'))); assert(flatLongLen.toString() === (new Array(10 + 1).join('asdf'))); +assert.throws(function() { + Buffer.concat([42]); +}, TypeError); + console.log('ok');