From 35e0d60d0cc654d88f05de6229ef7913bf2ba5e1 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Mon, 22 Jul 2013 17:04:17 -0700 Subject: [PATCH] buffer: slice on zero length buffer SlowBuffer(0) passes NULL instead of doing malloc(0). So when someone attempted to SlowBuffer(0).slice(0, 1) an assert would fail in smalloc::SliceOnto. It's important that the check go where it is because the resulting Buffer needs to have external array data allocated. In the case a user tries to slice a zero length Buffer it will also have NULL passed as the data argument. Also fixed where the .parent attribute was set for zero length Buffers. There is no need to track the source of slice if the slice isn't actually occurring. --- lib/buffer.js | 13 ++++++++----- src/smalloc.cc | 5 +++-- test/simple/test-buffer.js | 7 +++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index bab4a12c1ed..2aac14caa73 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -83,10 +83,12 @@ function Buffer(subject, encoding) { if (this.length < Buffer.poolSize / 2 && this.length > 0) { if (this.length > poolSize - poolOffset) createPool(); - this.parent = sliceOnto(allocPool, - this, - poolOffset, - poolOffset + this.length); + var parent = sliceOnto(allocPool, + this, + poolOffset, + poolOffset + this.length); + if (this.length > 0) + this.parent = parent; poolOffset += this.length; } else { alloc(this, this.length); @@ -373,8 +375,9 @@ Buffer.prototype.slice = function(start, end) { var buf = new Buffer(); sliceOnto(this, buf, start, end); - buf.parent = this.parent === undefined ? this : this.parent; buf.length = end - start; + if (buf.length > 0) + buf.parent = this.parent === undefined ? this : this.parent; return buf; }; diff --git a/src/smalloc.cc b/src/smalloc.cc index 4b33c756f93..a48fd7c88a1 100644 --- a/src/smalloc.cc +++ b/src/smalloc.cc @@ -124,15 +124,16 @@ void SliceOnto(const FunctionCallbackInfo& args) { size_t source_len = source->GetIndexedPropertiesExternalArrayDataLength(); size_t start = args[2]->Uint32Value(); size_t end = args[3]->Uint32Value(); + size_t length = end - start; assert(!dest->HasIndexedPropertiesInExternalArrayData()); - assert(source_data != NULL); + assert(source_data != NULL || length == 0); assert(end <= source_len); assert(start <= end); dest->SetIndexedPropertiesToExternalArrayData(source_data + start, kExternalUnsignedByteArray, - end - start); + length); args.GetReturnValue().Set(source); } diff --git a/test/simple/test-buffer.js b/test/simple/test-buffer.js index 40b8e7bebb3..91b38422f51 100644 --- a/test/simple/test-buffer.js +++ b/test/simple/test-buffer.js @@ -947,6 +947,13 @@ assert.throws(function() { buf.readInt8(0); }, RangeError); assert.equal(buf.slice(-i), s.slice(-i)); assert.equal(buf.slice(0, -i), s.slice(0, -i)); } + // try to slice a zero length Buffer + // see https://github.com/joyent/node/issues/5881 + SlowBuffer(0).slice(0, 1); + // make sure a zero length slice doesn't set the .parent attribute + assert.equal(Buffer(5).slice(0,0).parent, undefined); + // and make sure a proper slice does have a parent + assert.ok(typeof Buffer(5).slice(0, 5).parent === 'object'); })(); // Make sure byteLength properly checks for base64 padding