buffer: remove noAssert argument

src/node_buffer.cc

@@ -718,11 +718,16 @@ static inline void Swizzle(char* start, unsigned int len) {
 
 template <typename T, enum Endianness endianness>
 void ReadFloatGeneric(const FunctionCallbackInfo<Value>& args) {
-  THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[0]);
+  auto env = Environment::GetCurrent(args);
+  THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
   SPREAD_BUFFER_ARG(args[0], ts_obj);
 
-  uint32_t offset = args[1]->Uint32Value();
-  CHECK_LE(offset + sizeof(T), ts_obj_length);
+  uint32_t offset;
+  if (!args[1]->Uint32Value(env->context()).To(&offset))
+    return;  // Exception pending.
+
+  if (ts_obj_length < Add32Clamp(offset, sizeof(T)))
+    return env->ThrowRangeError("Index out of range");
 
   union NoAlias {
     T val;
@@ -762,13 +767,10 @@ void ReadDoubleBE(const FunctionCallbackInfo<Value>& args) {
 template <typename T, enum Endianness endianness>
 void WriteFloatGeneric(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
+  THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
 
   bool should_assert = args.Length() < 4;
 
-  if (should_assert) {
-    THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
-  }
-
   Local<ArrayBufferView> ts_obj = args[0].As<ArrayBufferView>();
   ArrayBuffer::Contents ts_obj_c = ts_obj->Buffer()->GetContents();
   const size_t ts_obj_offset = ts_obj->ByteOffset();

src/util-inl.h

@@ -341,6 +341,14 @@ bool StringEqualNoCaseN(const char* a, const char* b, size_t length) {
   return true;
 }
 
+inline uint32_t Add32Clamp(uint32_t x, uint32_t y) {
+  return x + y >= x ? x + y : static_cast<uint32_t>(-1);
+}
+
+inline uint64_t Add64Clamp(uint64_t x, uint64_t y) {
+  return x + y >= x ? x + y : static_cast<uint64_t>(-1);
+}
+
 inline size_t MultiplyWithOverflowCheck(size_t a, size_t b) {
   size_t ret = a * b;
   if (a != 0)

src/util.h

@@ -269,6 +269,10 @@ inline bool StringEqualNoCase(const char* a, const char* b);
 // strncasecmp() is locale-sensitive.  Use StringEqualNoCaseN() instead.
 inline bool StringEqualNoCaseN(const char* a, const char* b, size_t length);
 
+// Saturating addition.
+inline uint32_t Add32Clamp(uint32_t x, uint32_t y);
+inline uint64_t Add64Clamp(uint64_t x, uint64_t y);
+
 // Allocates an array of member type T. For up to kStackStorageSize items,
 // the stack is used, otherwise malloc().
 template <typename T, size_t kStackStorageSize = 1024>

test/parallel/test-buffer-read.js

@@ -140,3 +140,75 @@ assert.throws(() => Buffer.allocUnsafe(8).readFloatLE(-1), RangeError);
   assert.strictEqual(buf.readIntLE(0, 6), 0x060504030201);
   assert.strictEqual(buf.readIntBE(0, 6), 0x010203040506);
 }
+
+// https://github.com/nodejs/node/issues/8724 - specific to methods dealing
+// with floating point types because they call out to C++ code.
+{
+  // ERR_INDEX_OUT_OF_RANGE is optional, exceptions from the binding layer
+  // don't have it.
+  const re = /^RangeError( \[ERR_INDEX_OUT_OF_RANGE\])?: Index out of range$/;
+  const buf = Buffer.alloc(0);
+  assert.throws(() => buf.readFloatBE(0), re);
+  assert.throws(() => buf.readFloatLE(0), re);
+  assert.throws(() => buf.readDoubleBE(0), re);
+  assert.throws(() => buf.readDoubleLE(0), re);
+  for (const noAssert of [true, false]) {
+    assert.throws(() => buf.readFloatBE(0, noAssert), re);
+    assert.throws(() => buf.readFloatLE(0, noAssert), re);
+    assert.throws(() => buf.readDoubleBE(0, noAssert), re);
+    assert.throws(() => buf.readDoubleLE(0, noAssert), re);
+  }
+}
+
+{
+  const { readFloatBE, readFloatLE, readDoubleBE, readDoubleLE } =
+      Buffer.prototype;
+  const re = /^TypeError: argument should be a Buffer$/;
+  assert.throws(() => readFloatBE(0, true), re);
+  assert.throws(() => readFloatLE(0, true), re);
+  assert.throws(() => readDoubleBE(0, true), re);
+  assert.throws(() => readDoubleLE(0, true), re);
+}
+
+{
+  const { readFloatBE, readFloatLE, readDoubleBE, readDoubleLE } =
+      Buffer.prototype;
+  const re = /^TypeError: Cannot read property 'length' of undefined$/;
+  assert.throws(() => readFloatBE(0), re);
+  assert.throws(() => readFloatLE(0), re);
+  assert.throws(() => readDoubleBE(0), re);
+  assert.throws(() => readDoubleLE(0), re);
+  assert.throws(() => readFloatBE(0, false), re);
+  assert.throws(() => readFloatLE(0, false), re);
+  assert.throws(() => readDoubleBE(0, false), re);
+  assert.throws(() => readDoubleLE(0, false), re);
+}
+
+{
+  const re =
+      new RegExp('^TypeError: Method get TypedArray\\.prototype\\.length ' +
+                 'called on incompatible receiver \\[object Object\\]$');
+  assert.throws(() => Buffer.prototype.readFloatBE(0), re);
+  assert.throws(() => Buffer.prototype.readFloatLE(0), re);
+  assert.throws(() => Buffer.prototype.readDoubleBE(0), re);
+  assert.throws(() => Buffer.prototype.readDoubleLE(0), re);
+  assert.throws(() => Buffer.prototype.readFloatBE(0, false), re);
+  assert.throws(() => Buffer.prototype.readFloatLE(0, false), re);
+  assert.throws(() => Buffer.prototype.readDoubleBE(0, false), re);
+  assert.throws(() => Buffer.prototype.readDoubleLE(0, false), re);
+}
+
+for (const noAssert of [true, false]) {
+  const re = /^TypeError: argument should be a Buffer$/;
+  // Wrong receiver.
+  assert.throws(() => Buffer.prototype.readFloatBE.call({}, 0, noAssert), re);
+  assert.throws(() => Buffer.prototype.readFloatLE.call({}, 0, noAssert), re);
+  assert.throws(() => Buffer.prototype.readDoubleBE.call({}, 0, noAssert), re);
+  assert.throws(() => Buffer.prototype.readDoubleLE.call({}, 0, noAssert), re);
+  if (noAssert === false) continue;  // noAssert=false is tested above.
+  // Non-method call.
+  assert.throws(() => Buffer.prototype.readFloatBE(0, noAssert), re);
+  assert.throws(() => Buffer.prototype.readFloatLE(0, noAssert), re);
+  assert.throws(() => Buffer.prototype.readDoubleBE(0, noAssert), re);
+  assert.throws(() => Buffer.prototype.readDoubleLE(0, noAssert), re);
+}

test/parallel/test-buffer-write-noassert.js

@@ -100,3 +100,26 @@ writePartial('writeFloatBE', [1, 6], 10,
              Buffer.from([0, 0, 0, 0, 0, 0, 63, 128, 0]));
 writePartial('writeFloatLE', [1, 6], 10,
              Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 128]));
+
+// https://github.com/nodejs/node/issues/8724 - specific to methods dealing
+// with floating point types because they call out to C++ code.
+for (const noAssert of [true, false]) {
+  const re = /^TypeError: argument should be a Buffer$/;
+  const proto = Buffer.prototype;
+  const { writeFloatBE, writeFloatLE, writeDoubleBE, writeDoubleLE } = proto;
+  // Non-method call.
+  assert.throws(() => writeFloatBE(0, 0, noAssert), re);
+  assert.throws(() => writeFloatLE(0, 0, noAssert), re);
+  assert.throws(() => writeDoubleBE(0, 0, noAssert), re);
+  assert.throws(() => writeDoubleLE(0, 0, noAssert), re);
+  // Wrong receiver.
+  assert.throws(() => proto.writeFloatBE(0, 0, noAssert), re);
+  assert.throws(() => proto.writeFloatLE(0, 0, noAssert), re);
+  assert.throws(() => proto.writeDoubleBE(0, 0, noAssert), re);
+  assert.throws(() => proto.writeDoubleLE(0, 0, noAssert), re);
+  // Wrong receiver.
+  assert.throws(() => proto.writeFloatBE.call({}, 0, 0, noAssert), re);
+  assert.throws(() => proto.writeFloatLE.call({}, 0, 0, noAssert), re);
+  assert.throws(() => proto.writeDoubleBE.call({}, 0, 0, noAssert), re);
+  assert.throws(() => proto.writeDoubleLE.call({}, 0, 0, noAssert), re);
+}