buffer: accept negative indices in Buffer#slice()

bnoordhuis · isaacs · commit 3f7e88a852af · 2013-02-12T17:09:19.000-08:00
A negative start or end parameter now indexes from the end of the
buffer. More in line with String#slice() and ArrayBuffer#slice().
diff --git a/doc/api/buffer.markdown b/doc/api/buffer.markdown
@@ -48,8 +48,8 @@ NOTE: Node.js v0.8 simply retained a reference to the buffer in `array.buffer`
 instead of cloning it.
 
 While more efficient, it introduces subtle incompatibilities with the typed
-arrays specification.  `ArrayBuffer#slice()` and `Buffer#slice()` behave
-differently when passed negative indices, for example.
+arrays specification.  `ArrayBuffer#slice()` makes a copy of the slice while
+`Buffer#slice()` creates a view.
 
 ## Class: Buffer
 
@@ -260,7 +260,7 @@ into `buf2`, starting at the 8th byte in `buf2`.
 
 Returns a new buffer which references the same memory as the old, but offset
 and cropped by the `start` (defaults to `0`) and `end` (defaults to
-`buffer.length`) indexes.
+`buffer.length`) indexes.  Negative indexes start from the end of the buffer.
 
 **Modifying the new buffer slice will modify memory in the original buffer!**
 
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -29,6 +29,17 @@ exports.INSPECT_MAX_BYTES = 50;
 SlowBuffer.prototype.__proto__ = Buffer.prototype;
 
 
+function clamp(index, len, defaultValue) {
+  if (typeof index === 'undefined') return defaultValue;
+  index = ~~index;  // Coerce to integer.
+  if (index >= len) return len;
+  if (index >= 0) return index;
+  index += len;
+  if (index >= 0) return index;
+  return 0;
+}
+
+
 function toHex(n) {
   if (n < 16) return '0' + n.toString(16);
   return n.toString(16);
@@ -132,16 +143,10 @@ SlowBuffer.prototype.write = function(string, offset, length, encoding) {
 
 // slice(start, end)
 SlowBuffer.prototype.slice = function(start, end) {
-  if (end === undefined) end = this.length;
-
-  if (end > this.length) {
-    throw new RangeError('end > this.length');
-  }
-  if (start > end) {
-    throw new RangeError('start > end');
-  }
-
-  return new Buffer(this, end - start, +start);
+  var len = this.length;
+  start = clamp(start, len, 0);
+  end = clamp(end, len, len);
+  return new Buffer(this, end - start, start);
 };
 
 
@@ -528,14 +533,10 @@ Buffer.prototype.copy = function(target, target_start, start, end) {
 
 // slice(start, end)
 Buffer.prototype.slice = function(start, end) {
-  if (end === undefined) end = this.length;
-  if (end > this.length)
-    throw new RangeError('end > this.length');
-  if (start > end)
-    throw new RangeError('start > end');
-  if (start < 0)
-    throw new RangeError('start < 0');
-  return new Buffer(this.parent, end - start, +start + this.offset);
+  var len = this.length;
+  start = clamp(start, len, 0);
+  end = clamp(end, len, len);
+  return new Buffer(this.parent, end - start, start + this.offset);
 };
 
 
diff --git a/test/simple/test-buffer.js b/test/simple/test-buffer.js
@@ -925,30 +925,24 @@ assert.throws(function() {
   for (var i = 0; i < len; ++i) buf[i] = 0x42;     // Try to force segfault.
 }, RangeError);
 
-assert.throws(function() {
-  var len = 0xfffff;
-  var sbuf = new SlowBuffer(len);
-  sbuf = sbuf.slice(-len);                          // Should throw.
-  for (var i = 0; i < len; ++i) sbuf[i] = 0x42;     // Try to force segfault.
-}, RangeError);
-
 assert.throws(function() {
   var sbuf = new SlowBuffer(1);
   var buf = new Buffer(sbuf, 1, 0);
   buf.length = 0xffffffff;
   buf.slice(0xffffff0, 0xffffffe);                  // Should throw.
 }, Error);
 
-assert.throws(function() {
-  var sbuf = new SlowBuffer(8);
-  var buf = new Buffer(sbuf, 8, 0);
-  buf.slice(-8);  // Should throw. Throws Error instead of RangeError
-                  // for the sake of v0.8 compatibility.
-}, Error);
-
-assert.throws(function() {
-  var sbuf = new SlowBuffer(16);
-  var buf = new Buffer(sbuf, 8, 8);
-  buf.slice(-8);  // Should throw. Throws Error instead of RangeError
-                  // for the sake of v0.8 compatibility.
-}, Error);
+(function() {
+  var buf = new Buffer('0123456789');
+  assert.equal(buf.slice(-10, 10), '0123456789');
+  assert.equal(buf.slice(-20, 10), '0123456789');
+  assert.equal(buf.slice(-20, -10), '');
+  assert.equal(buf.slice(0, -1), '012345678');
+  assert.equal(buf.slice(2, -2), '234567');
+  assert.equal(buf.slice(0, 65536), '0123456789');
+  assert.equal(buf.slice(65536, 0), '');
+  for (var i = 0, s = buf.toString(); i < buf.length; ++i) {
+    assert.equal(buf.slice(-i), s.slice(-i));
+    assert.equal(buf.slice(0, -i), s.slice(0, -i));
+  }
+})();
