buffer: throw range error before truncating write

The check to determine whether `noAssert` was set to true and thus whether RangeErrors should be thrown was happening after the write was truncated to the available size of the buffer. These checks now occur in the correct order. Fixes: #5587 PR-URL: #5605 Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Mar 22, 2016 · 72fb796 · 72fb796
1 parent 341b3d0
commit 72fb796
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 3 deletions.
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -804,14 +804,14 @@ void WriteFloatGeneric(const FunctionCallbackInfo<Value>& args) {
   size_t offset = args[2]->IntegerValue(env->context()).FromMaybe(0);
 
   size_t memcpy_num = sizeof(T);
-  if (offset + sizeof(T) > ts_obj_length)
-    memcpy_num = ts_obj_length - offset;
 
   if (should_assert) {
     CHECK_NOT_OOB(offset + memcpy_num >= memcpy_num);
     CHECK_NOT_OOB(offset + memcpy_num <= ts_obj_length);
   }
-  CHECK_LE(offset + memcpy_num, ts_obj_length);
+
+  if (offset + memcpy_num > ts_obj_length)
+    memcpy_num = ts_obj_length - offset;
 
   union NoAlias {
     T val;

diff --git a/test/parallel/test-buffer.js b/test/parallel/test-buffer.js
@@ -1038,6 +1038,16 @@ assert.throws(function() {
   new Buffer(0xFFFFFFFFF);
 }, RangeError);
 
+// issue GH-5587
+assert.throws(function() {
+  var buf = new Buffer(8);
+  buf.writeFloatLE(0, 5);
+}, RangeError);
+assert.throws(function() {
+  var buf = new Buffer(16);
+  buf.writeDoubleLE(0, 9);
+}, RangeError);
+
 
 // attempt to overflow buffers, similar to previous bug in array buffers
 assert.throws(function() {