buffer: fix writeInt{B,L}E for some negative values

The algorithm used to convert negative values to hex generates incorrect values when the low byte(s) of the value are zero because a carried subtraction is applied prematurely. Fixes nodejs#3992 Signed-off-by: Peter A. Bigot <[email protected]>
pabigot · Nov 23, 2015 · 26e8f0f · 26e8f0f
1 parent 8bc8038
commit 26e8f0f
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 4 deletions.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -938,10 +938,13 @@ Buffer.prototype.writeIntLE = function(value, offset, byteLength, noAssert) {
 
   var i = 0;
   var mul = 1;
-  var sub = value < 0 ? 1 : 0;
+  var sub = 0;
   this[offset] = value;
-  while (++i < byteLength && (mul *= 0x100))
+  while (++i < byteLength && (mul *= 0x100)) {
+    if (value < 0 && sub === 0 && this[offset + i - 1] !== 0)
+      sub = 1;
     this[offset + i] = ((value / mul) >> 0) - sub;
+  }
 
   return offset + byteLength;
 };
@@ -961,10 +964,13 @@ Buffer.prototype.writeIntBE = function(value, offset, byteLength, noAssert) {
 
   var i = byteLength - 1;
   var mul = 1;
-  var sub = value < 0 ? 1 : 0;
+  var sub = 0;
   this[offset + i] = value;
-  while (--i >= 0 && (mul *= 0x100))
+  while (--i >= 0 && (mul *= 0x100)) {
+    if (value < 0 && sub === 0 && this[offset + i + 1] !== 0)
+      sub = 1;
     this[offset + i] = ((value / mul) >> 0) - sub;
+  }
 
   return offset + byteLength;
 };

diff --git a/test/parallel/test-buffer.js b/test/parallel/test-buffer.js
@@ -1027,6 +1027,26 @@ assert.equal(buf.readInt8(0), -1);
   assert.deepEqual(buf.toJSON().data, [0xed, 0xcb, 0xaa]);
   assert.equal(buf.readIntBE(0, 3), -0x123456);
 
+  buf = new Buffer(3);
+  buf.writeIntLE(-0x123400, 0, 3);
+  assert.deepEqual(buf.toJSON().data, [0x00, 0xcc, 0xed]);
+  assert.equal(buf.readIntLE(0, 3), -0x123400);
+
+  buf = new Buffer(3);
+  buf.writeIntBE(-0x123400, 0, 3);
+  assert.deepEqual(buf.toJSON().data, [0xed, 0xcc, 0x00]);
+  assert.equal(buf.readIntBE(0, 3), -0x123400);
+
+  buf = new Buffer(3);
+  buf.writeIntLE(-0x120000, 0, 3);
+  assert.deepEqual(buf.toJSON().data, [0x00, 0x00, 0xee]);
+  assert.equal(buf.readIntLE(0, 3), -0x120000);
+
+  buf = new Buffer(3);
+  buf.writeIntBE(-0x120000, 0, 3);
+  assert.deepEqual(buf.toJSON().data, [0xee, 0x00, 0x00]);
+  assert.equal(buf.readIntBE(0, 3), -0x120000);
+
   buf = new Buffer(5);
   buf.writeUIntLE(0x1234567890, 0, 5);
   assert.deepEqual(buf.toJSON().data, [0x90, 0x78, 0x56, 0x34, 0x12]);
@@ -1056,6 +1076,17 @@ assert.equal(buf.readInt8(0), -1);
   buf.writeIntBE(-0x1234567890, 0, 5);
   assert.deepEqual(buf.toJSON().data, [0xed, 0xcb, 0xa9, 0x87, 0x70]);
   assert.equal(buf.readIntBE(0, 5), -0x1234567890);
+
+  buf = new Buffer(5);
+  buf.writeIntLE(-0x0012000000, 0, 5);
+  assert.deepEqual(buf.toJSON().data, [0x00, 0x00, 0x00, 0xee, 0xff]);
+  assert.equal(buf.readIntLE(0, 5), -0x0012000000);
+
+  buf = new Buffer(5);
+  buf.writeIntBE(-0x0012000000, 0, 5);
+  assert.deepEqual(buf.toJSON().data, [0xff, 0xee, 0x00, 0x00, 0x00]);
+  assert.equal(buf.readIntBE(0, 5), -0x0012000000);
+
 })();
 
 // test Buffer slice