fast buffer bounds checking in copy()

ry · ry · commit d5bdda74c160 · 2010-09-09T11:03:48.000-07:00
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -17,29 +17,29 @@ SlowBuffer.prototype.inspect = function () {
 };
 
 
-SlowBuffer.prototype.toString = function (encoding, start, stop) {
+SlowBuffer.prototype.toString = function (encoding, start, end) {
   encoding = String(encoding || 'utf8').toLowerCase();
   start = +start || 0;
-  if (typeof stop == "undefined") stop = this.length;
+  if (typeof end == "undefined") end = this.length;
 
   // Fastpath empty strings
-  if (+stop == start) {
+  if (+end == start) {
     return '';
   }
 
   switch (encoding) {
     case 'utf8':
     case 'utf-8':
-      return this.utf8Slice(start, stop);
+      return this.utf8Slice(start, end);
 
     case 'ascii':
-      return this.asciiSlice(start, stop);
+      return this.asciiSlice(start, end);
 
     case 'binary':
-      return this.binarySlice(start, stop);
+      return this.binarySlice(start, end);
 
     case 'base64':
-      return this.base64Slice(start, stop);
+      return this.base64Slice(start, end);
 
     default:
       throw new Error('Unknown encoding');
@@ -212,14 +212,19 @@ Buffer.prototype.write = function write (string, offset, encoding) {
 
 
 // toString(encoding, start=0, end=buffer.length)
-Buffer.prototype.toString = function toString (encoding, start, end) {
-  encoding || (encoding = 'utf8');
-  start    || (start    = 0);
-  end      || (end      = this.length);
+Buffer.prototype.toString = function (encoding, start, end) {
+  if (typeof encoding == 'undefined') encoding = 'utf8';
 
-  // Make sure we aren't oob
-  if (end > this.length) {
+  if (typeof start == 'undefined' || start < 0) {
+    start = 0;
+  } else if (start > this.length) {
+    start = this.length;
+  }
+
+  if (typeof end == "undefined" || end > this.length) {
     end = this.length;
+  } else if (end < 0) {
+    end = 0;
   }
 
   return this.parent.toString(encoding, start + this.offset, end + this.offset);
@@ -232,14 +237,37 @@ Buffer.byteLength = SlowBuffer.byteLength;
 
 // copy(targetBuffer, targetStart, sourceStart, sourceEnd=buffer.length)
 Buffer.prototype.copy = function copy (target, target_start, start, end) {
+  var source = this;
   start || (start = 0);
   end   || (end   = this.length);
 
+  if (end < start) throw new Error("sourceEnd < sourceStart");
+
+  // Copy 0 bytes; we're done
+  if (end === start) return 0;
+  if (target.length == 0 || source.length == 0) return 0;
+
+  if (target_start < 0 || target_start >= target.length) {
+    throw new Error("targetStart out of bounds");
+  }
+
+  if (start < 0 || start >= source.length) {
+    throw new Error("sourceStart out of bounds");
+  }
+
+  if (end < 0 || end > source.length) {
+    throw new Error("sourceEnd out of bounds");
+  }
+
   // Are we oob?
   if (end > this.length) {
     end = this.length;
   }
 
+  if (target.length - target_start < end - start) {
+    end = target.length - target_start + start;
+  }
+
   return this.parent.copy(target.parent,
                           target_start + target.offset,
                           start + this.offset,
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -485,8 +485,10 @@ Handle<Value> Buffer::Copy(const Arguments &args) {
             "sourceEnd out of bounds")));
   }
 
-  ssize_t to_copy = MIN(source_end - source_start,
-                        target->length() - target_start);
+  ssize_t to_copy = MIN(MIN(source_end - source_start,
+                            target->length() - target_start), 
+                            source->length() - source_start);
+  
 
   if (target->blob_ == source->blob_) {
     // need to use slightly slower memmove is the ranges might overlap
diff --git a/test/simple/test-buffer.js b/test/simple/test-buffer.js
@@ -19,11 +19,13 @@ for (var i = 0; i < 1024; i++) {
 }
 
 var c = new Buffer(512);
+console.log("c.length == %d", c.length);
+assert.strictEqual(512, c.length);
 
 // copy 512 bytes, from 0 to 512.
 var copied = b.copy(c, 0, 0, 512);
 console.log("copied " + copied + " bytes from b into c");
-assert.strictEqual(512, copied);
+assert.equal(512, copied);
 for (var i = 0; i < c.length; i++) {
   common.print('.');
   assert.equal(i % 256, c[i]);
