buffer: prevent failed assertions

This commit adds assert() calls in JS, which prevents non-buffer
instances from triggering failed C++ assertions (killing the process).

They will now die with an AssertionError. Buffer.prototype.copy had to
be wrapped in JS to allow for the assertion, with a modification to the
arguments passed.

Fixes: nodejs#1485

lib/buffer.js

@@ -3,6 +3,7 @@
 const binding = process.binding('buffer');
 const smalloc = process.binding('smalloc');
 const util = require('util');
+const assert = require('assert');
 const internalUtil = require('internal/util');
 const alloc = smalloc.alloc;
 const truncate = smalloc.truncate;
@@ -335,6 +336,14 @@ Buffer.byteLength = byteLength;
 
 // toString(encoding, start=0, end=buffer.length)
 Buffer.prototype.toString = function(encoding, start, end) {
+  // .toString() can be called automatically on non-Buffers.
+  if (!(this instanceof Buffer)) {
+    if (arguments.length === 0)
+      return Object.prototype.toString.call(this);
+    else
+      assert(this instanceof Buffer);
+  }
+
   var loweredCase = false;
 
   start = start >>> 0;
@@ -380,8 +389,7 @@ Buffer.prototype.toString = function(encoding, start, end) {
 
 
 Buffer.prototype.equals = function equals(b) {
-  if (!(b instanceof Buffer))
-    throw new TypeError('Argument must be a Buffer');
+  assert(this instanceof Buffer && b instanceof Buffer);
 
   if (this === b)
     return true;
@@ -404,8 +412,7 @@ Buffer.prototype.inspect = function inspect() {
 
 
 Buffer.prototype.compare = function compare(b) {
-  if (!(b instanceof Buffer))
-    throw new TypeError('Argument must be a Buffer');
+  assert(this instanceof Buffer && b instanceof Buffer);
 
   if (this === b)
     return 0;
@@ -415,6 +422,8 @@ Buffer.prototype.compare = function compare(b) {
 
 
 Buffer.prototype.indexOf = function indexOf(val, byteOffset) {
+  assert(this instanceof Buffer);
+
   if (byteOffset > 0x7fffffff)
     byteOffset = 0x7fffffff;
   else if (byteOffset < -0x80000000)
@@ -433,6 +442,8 @@ Buffer.prototype.indexOf = function indexOf(val, byteOffset) {
 
 
 Buffer.prototype.fill = function fill(val, start, end) {
+  assert(this instanceof Buffer);
+
   start = start >> 0;
   end = (end === undefined) ? this.length : end >> 0;
 
@@ -455,6 +466,13 @@ Buffer.prototype.fill = function fill(val, start, end) {
 };
 
 
+Buffer.prototype.copy = function copy(target, start, sourceStart, sourceEnd) {
+  assert(this instanceof Buffer && target instanceof Buffer);
+
+  return binding.copy(this, target, start, sourceStart, sourceEnd);
+};
+
+
 // XXX remove in v0.13
 Buffer.prototype.get = util.deprecate(function get(offset) {
   offset = ~~offset;
@@ -779,6 +797,8 @@ Buffer.prototype.readInt32BE = function(offset, noAssert) {
 
 
 Buffer.prototype.readFloatLE = function readFloatLE(offset, noAssert) {
+  assert(this instanceof Buffer);
+
   offset = offset >>> 0;
   if (!noAssert)
     checkOffset(offset, 4, this.length);
@@ -787,6 +807,8 @@ Buffer.prototype.readFloatLE = function readFloatLE(offset, noAssert) {
 
 
 Buffer.prototype.readFloatBE = function readFloatBE(offset, noAssert) {
+  assert(this instanceof Buffer);
+
   offset = offset >>> 0;
   if (!noAssert)
     checkOffset(offset, 4, this.length);
@@ -795,6 +817,8 @@ Buffer.prototype.readFloatBE = function readFloatBE(offset, noAssert) {
 
 
 Buffer.prototype.readDoubleLE = function readDoubleLE(offset, noAssert) {
+  assert(this instanceof Buffer);
+
   offset = offset >>> 0;
   if (!noAssert)
     checkOffset(offset, 8, this.length);
@@ -803,6 +827,8 @@ Buffer.prototype.readDoubleLE = function readDoubleLE(offset, noAssert) {
 
 
 Buffer.prototype.readDoubleBE = function readDoubleBE(offset, noAssert) {
+  assert(this instanceof Buffer);
+
   offset = offset >>> 0;
   if (!noAssert)
     checkOffset(offset, 8, this.length);
@@ -1025,6 +1051,8 @@ function checkFloat(buffer, value, offset, ext) {
 
 
 Buffer.prototype.writeFloatLE = function writeFloatLE(val, offset, noAssert) {
+  assert(this instanceof Buffer);
+
   val = +val;
   offset = offset >>> 0;
   if (!noAssert)
@@ -1035,6 +1063,8 @@ Buffer.prototype.writeFloatLE = function writeFloatLE(val, offset, noAssert) {
 
 
 Buffer.prototype.writeFloatBE = function writeFloatBE(val, offset, noAssert) {
+  assert(this instanceof Buffer);
+
   val = +val;
   offset = offset >>> 0;
   if (!noAssert)
@@ -1045,6 +1075,8 @@ Buffer.prototype.writeFloatBE = function writeFloatBE(val, offset, noAssert) {
 
 
 Buffer.prototype.writeDoubleLE = function writeDoubleLE(val, offset, noAssert) {
+  assert(this instanceof Buffer);
+
   val = +val;
   offset = offset >>> 0;
   if (!noAssert)
@@ -1055,6 +1087,8 @@ Buffer.prototype.writeDoubleLE = function writeDoubleLE(val, offset, noAssert) {
 
 
 Buffer.prototype.writeDoubleBE = function writeDoubleBE(val, offset, noAssert) {
+  assert(this instanceof Buffer);
+
   val = +val;
   offset = offset >>> 0;
   if (!noAssert)

src/node_buffer.cc

@@ -303,22 +303,19 @@ void Base64Slice(const FunctionCallbackInfo<Value>& args) {
 void Copy(const FunctionCallbackInfo<Value> &args) {
   Environment* env = Environment::GetCurrent(args);
 
-  if (!HasInstance(args[0]))
-    return env->ThrowTypeError("first arg should be a Buffer");
+  Local<Object> target = args[1]->ToObject(env->isolate());
 
-  Local<Object> target = args[0]->ToObject(env->isolate());
-
-  ARGS_THIS(args.This())
+  ARGS_THIS(args[0].As<Object>());
   size_t target_length = target->GetIndexedPropertiesExternalArrayDataLength();
   char* target_data = static_cast<char*>(
       target->GetIndexedPropertiesExternalArrayData());
   size_t target_start;
   size_t source_start;
   size_t source_end;
 
-  CHECK_NOT_OOB(ParseArrayIndex(args[1], 0, &target_start));
-  CHECK_NOT_OOB(ParseArrayIndex(args[2], 0, &source_start));
-  CHECK_NOT_OOB(ParseArrayIndex(args[3], obj_length, &source_end));
+  CHECK_NOT_OOB(ParseArrayIndex(args[2], 0, &target_start));
+  CHECK_NOT_OOB(ParseArrayIndex(args[3], 0, &source_start));
+  CHECK_NOT_OOB(ParseArrayIndex(args[4], obj_length, &source_end));
 
   // Copy 0 bytes; we're done
   if (target_start >= target_length || source_start >= source_end)
@@ -723,8 +720,6 @@ void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
   env->SetMethod(proto, "ucs2Write", Ucs2Write);
   env->SetMethod(proto, "utf8Write", Utf8Write);
 
-  env->SetMethod(proto, "copy", Copy);
-
   // for backwards compatibility
   proto->ForceSet(env->offset_string(),
                   Uint32::New(env->isolate(), 0),
@@ -742,6 +737,7 @@ void Initialize(Handle<Object> target,
   env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8);
   env->SetMethod(target, "compare", Compare);
   env->SetMethod(target, "fill", Fill);
+  env->SetMethod(target, "copy", Copy);
   env->SetMethod(target, "indexOfBuffer", IndexOfBuffer);
   env->SetMethod(target, "indexOfNumber", IndexOfNumber);
   env->SetMethod(target, "indexOfString", IndexOfString);

test/parallel/test-buffer.js

@@ -1172,3 +1172,45 @@ Buffer.poolSize = ps;
 assert.throws(function() {
   Buffer(10).copy();
 });
+
+
+// https://github.com/nodejs/io.js/issues/1485
+// C++ bindings fail assertions (die) when called on non-buffers.
+assert.equal(Buffer.prototype.toString(), '[object Object]');
+
+[null, undefined, {}, 0].forEach(function(v) {
+  assert.throws(function() {
+    Buffer.prototype.write.call(v);
+  });
+
+  assert.throws(function() {
+    Buffer.prototype.copy.call(v, new Buffer(0));
+  });
+  assert.throws(function() {
+    (new Buffer(0)).copy(v);
+  });
+
+  assert.throws(function() {
+    Buffer.prototype.equals.call(v, new Buffer('hi'));
+  });
+
+  assert.throws(function() {
+    Buffer.prototype.compare.call(v, new Buffer('hi'));
+  });
+
+  assert.throws(function() {
+    Buffer.prototype.fill.call(v);
+  });
+
+  assert.throws(function() {
+    Buffer.prototype.indexOf.call(v, 0);
+  });
+
+  assert.throws(function() {
+    Buffer.prototype.readDoubleLE.call(v, 0, false);
+  });
+
+  assert.throws(function() {
+    Buffer.prototype.writeDoubleLE.call(v, 0, 0, false);
+  });
+});