buffer: validate list elements in Buffer.concat

Without this change, if any of the elements in the list to be concatenated is not a Buffer instance, the method fails with "buf.copy is not a function". Make an isBuffer check before using the copy method so that we can throw with a better message. Fixes: nodejs#4949 PR-URL: nodejs#4951 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Roman Klauke <[email protected]>
GarrettS · Jan 31, 2016 · d2dc234 · d2dc234
1 parent de3e94b
commit d2dc234
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 4 deletions.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -241,6 +241,8 @@ Buffer.concat = function(list, length) {
   var pos = 0;
   for (let i = 0; i < list.length; i++) {
     var buf = list[i];
+    if (!Buffer.isBuffer(buf))
+      throw new TypeError('"list" argument must be an Array of Buffers');
     buf.copy(buffer, pos);
     pos += buf.length;
   }

diff --git a/test/parallel/test-buffer-concat.js b/test/parallel/test-buffer-concat.js
@@ -20,8 +20,18 @@ 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);
+assertWrongList();
+assertWrongList(null);
+assertWrongList(new Buffer('hello'));
+assertWrongList([42]);
+assertWrongList(['hello', 'world']);
+assertWrongList(['hello', new Buffer('world')]);
 
-console.log('ok');
+function assertWrongList(value) {
+  assert.throws(function() {
+    Buffer.concat(value);
+  }, function(err) {
+    return err instanceof TypeError &&
+           err.message === '"list" argument must be an Array of Buffers';
+  });
+}