Permalink
Browse files

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.
  • Loading branch information...
1 parent aa49151 commit a2f70da4c9bf14a6e0c4a17cac11a5ab555ce90f @mranney mranney committed with ry Jun 24, 2010
Showing with 106 additions and 15 deletions.
  1. +1 −1 doc/api.markdown
  2. +2 −2 lib/string_decoder.js
  3. +16 −9 src/node_buffer.cc
  4. +87 −3 test/simple/test-buffer.js
View
@@ -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)
View
@@ -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) {
View
@@ -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));
}
View
@@ -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";

0 comments on commit a2f70da

Please sign in to comment.