From eea66e2a7b34f10e723b5f3efa3efb2e2d30aafe 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 c082ed8bd57b1b..97df88e17e4c60 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 d892fd67a23069..2d8d00249ee642 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');