Buffer.copy should copy through sourceEnd, as specified.

Improve test-buffer.js to cover all copy error cases.

Fix off by one error in string_decoder.

doc/api.markdown

@@ -177,7 +177,7 @@ into `buf2`, starting at the 8th byte in `buf2`.
     buf1.copy(buf2, 8, 16, 20);
     console.log(buf2.toString('ascii', 0, 25));
 
-    // !!!!!!!!qrst!!!!!!!!!!!!!
+    // !!!!!!!!qrstu!!!!!!!!!!!!
 
 
 ### buffer.slice(start, end)

lib/string_decoder.js

@@ -25,7 +25,7 @@ StringDecoder.prototype.write = function (buffer) {
       : buffer.length;
 
     // add the new bytes to the char buffer
-    buffer.copy(this.charBuffer, this.charReceived, 0, i);
+    buffer.copy(this.charBuffer, this.charReceived, 0, (i - 1));
     this.charReceived += i;
 
     if (this.charReceived < this.charLength) {
@@ -79,7 +79,7 @@ StringDecoder.prototype.write = function (buffer) {
   }
 
   // buffer the incomplete character bytes we got
-  buffer.copy(this.charBuffer, 0, buffer.length - i, buffer.length);
+  buffer.copy(this.charBuffer, 0, buffer.length - i, (buffer.length - 1));
   this.charReceived = i;
 
   if (buffer.length - i > 0) {

src/node_buffer.cc

@@ -298,34 +298,41 @@ Handle<Value> Buffer::Copy(const Arguments &args) {
   ssize_t target_start = args[1]->Int32Value();
   ssize_t source_start = args[2]->Int32Value();
   ssize_t source_end = args[3]->IsInt32() ? args[3]->Int32Value()
-                                          : source->length();
+                                          : (source->length() - 1);
 
   if (source_end < source_start) {
     return ThrowException(Exception::Error(String::New(
             "sourceEnd < sourceStart")));
   }
 
-  if (target_start < 0 || target_start > target->length()) {
+  if (target_start < 0 || target_start >= target->length()) {
     return ThrowException(Exception::Error(String::New(
             "targetStart out of bounds")));
   }
 
-  if (source_start < 0 || source_start > source->length()) {
+  if (source_start < 0 || source_start >= source->length()) {
     return ThrowException(Exception::Error(String::New(
             "sourceStart out of bounds")));
   }
 
-  if (source_end < 0 || source_end > source->length()) {
+  if (source_end < 0 || source_end >= source->length()) {
     return ThrowException(Exception::Error(String::New(
             "sourceEnd out of bounds")));
   }
 
-  ssize_t to_copy = MIN(source_end - source_start,
-                        target->length() - target_start);
+  ssize_t to_copy = MIN( (source_end - source_start + 1),
+                         (target->length() - target_start) );
 
-  memcpy((void*)(target->data() + target_start),
-         (const void*)(source->data() + source_start),
-         to_copy);
+  if (source->handle_->StrictEquals(target->handle_)) {
+    // need to use slightly slower memmove is the ranges might overlap
+    memmove((void*)(target->data() + target_start),
+      (const void*)(source->data() + source_start),
+      to_copy);
+  } else {
+    memcpy((void*)(target->data() + target_start),
+      (const void*)(source->data() + source_start),
+      to_copy);
+  }
 
   return scope.Close(Integer::New(to_copy));
 }

test/simple/test-buffer.js

@@ -6,7 +6,7 @@ var Buffer = require('buffer').Buffer;
 var b = new Buffer(1024);
 
 console.log("b.length == " + b.length);
-assert.equal(1024, b.length);
+assert.strictEqual(1024, b.length);
 
 for (var i = 0; i < 1024; i++) {
   assert.ok(b[i] >= 0);
@@ -19,12 +19,96 @@ for (var i = 0; i < 1024; i++) {
 
 var c = new Buffer(512);
 
-var copied = b.copy(c, 0, 0, 512);
-assert.equal(512, copied);
+// copy 512 bytes, from 0 to 511.
+var copied = b.copy(c, 0, 0, 511);
+console.log("copied " + copied + " bytes from b into c");
+assert.strictEqual(512, copied);
 for (var i = 0; i < c.length; i++) {
   print('.');
   assert.equal(i % 256, c[i]);
 }
+console.log("");
+
+// try to copy 513 bytes, and hope we don't overrun c, which is only 512 long
+var copied = b.copy(c, 0, 0, 512);
+console.log("copied " + copied + " bytes from b into c");
+assert.strictEqual(512, copied);
+for (var i = 0; i < c.length; i++) {
+  assert.equal(i % 256, c[i]);
+}
+
+// copy all of c back into b, without specifying sourceEnd
+var copied = c.copy(b, 0, 0);
+console.log("copied " + copied + " bytes from c back into b");
+assert.strictEqual(512, copied);
+for (var i = 0; i < b.length; i++) {
+  assert.equal(i % 256, b[i]);
+}
+
+// copy 768 bytes from b into b
+var copied = b.copy(b, 0, 256, 1023);
+console.log("copied " + copied + " bytes from b into c");
+assert.strictEqual(768, copied);
+for (var i = 0; i < c.length; i++) {
+  assert.equal(i % 256, c[i]);
+}
+
+var caught_error = null;
+
+// try to copy from before the beginning of b
+caught_error = null;
+try {
+    var copied = b.copy(c, 0, 100, 10);
+} catch (err) {
+    caught_error = err;
+}
+assert.strictEqual('sourceEnd < sourceStart', caught_error.message);
+
+// try to copy to before the beginning of c
+caught_error = null;
+try {
+    var copied = b.copy(c, -1, 0, 10);
+} catch (err) {
+    caught_error = err;
+}
+assert.strictEqual('targetStart out of bounds', caught_error.message);
+
+// try to copy to after the end of c
+caught_error = null;
+try {
+    var copied = b.copy(c, 512, 0, 10);
+} catch (err) {
+    caught_error = err;
+}
+assert.strictEqual('targetStart out of bounds', caught_error.message);
+
+// try to copy starting before the beginning of b
+caught_error = null;
+try {
+    var copied = b.copy(c, 0, -1, 1);
+} catch (err) {
+    caught_error = err;
+}
+assert.strictEqual('sourceStart out of bounds', caught_error.message);
+
+// try to copy starting after the end of b
+caught_error = null;
+try {
+    var copied = b.copy(c, 0, 1024, 1024);
+} catch (err) {
+    caught_error = err;
+}
+assert.strictEqual('sourceStart out of bounds', caught_error.message);
+
+// a too-low sourceEnd will get caught by earlier checks
+
+// try to copy ending after the end of b
+try {
+    var copied = b.copy(c, 0, 1023, 1024);
+} catch (err) { 
+    caught_error = err;
+}
+assert.strictEqual('sourceEnd out of bounds', caught_error.message);
 
 
 var asciiString = "hello world";