buffer: slice on zero length buffer

trevnorris · trevnorris · commit 35e0d60d0cc6 · 2013-07-22T17:27:59.000-07:00
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.
diff --git 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
@@ -124,15 +124,16 @@ void SliceOnto(const FunctionCallbackInfo<Value>& 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
@@ -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
