buffer: validate list elements in Buffer.concat

targos · targos · commit d2dc234def9f · 2016-01-31T09:31:08.000+01:00
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: #4949 PR-URL: #4951 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
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';
+  });
+}

Original file line number	Diff line number	Diff line change
`@@ -241,6 +241,8 @@ Buffer.concat = function(list, length) {`
`241`	`241`	`var pos = 0;`
`242`	`242`	`for (let i = 0; i < list.length; i++) {`
`243`	`243`	`var buf = list[i];`
	`244`	`+ if (!Buffer.isBuffer(buf))`
	`245`	`+ throw new TypeError('"list" argument must be an Array of Buffers');`
`244`	`246`	`buf.copy(buffer, pos);`
`245`	`247`	`pos += buf.length;`
`246`	`248`	`}`