fs: allow int64 offset in fs.write/writeSync/fd.write

Ref #26563 PR-URL: #26572 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
nodejs · Aug 17, 2019 · 5e3b4d6 · 5e3b4d6
1 parent a3c0014
commit 5e3b4d6
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 27 deletions.
diff --git a/lib/fs.js b/lib/fs.js
@@ -541,8 +541,11 @@ function write(fd, buffer, offset, length, position, callback) {
 
   if (isArrayBufferView(buffer)) {
     callback = maybeCallback(callback || position || length || offset);
-    if (typeof offset !== 'number')
+    if (offset == null || typeof offset === 'function') {
       offset = 0;
+    } else {
+      validateSafeInteger(offset, 'offset');
+    }
     if (typeof length !== 'number')
       length = buffer.length - offset;
     if (typeof position !== 'number')
@@ -580,8 +583,11 @@ function writeSync(fd, buffer, offset, length, position) {
   if (isArrayBufferView(buffer)) {
     if (position === undefined)
       position = null;
-    if (typeof offset !== 'number')
+    if (offset == null) {
       offset = 0;
+    } else {
+      validateSafeInteger(offset, 'offset');
+    }
     if (typeof length !== 'number')
       length = buffer.byteLength - offset;
     validateOffsetLengthWrite(offset, length, buffer.byteLength);

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -240,8 +240,11 @@ async function write(handle, buffer, offset, length, position) {
     return { bytesWritten: 0, buffer };
 
   if (isUint8Array(buffer)) {
-    if (typeof offset !== 'number')
+    if (offset == null) {
       offset = 0;
+    } else {
+      validateSafeInteger(offset, 'offset');
+    }
     if (typeof length !== 'number')
       length = buffer.length - offset;
     if (typeof position !== 'number')

diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
@@ -2,7 +2,7 @@
 
 const { Object, Reflect } = primordials;
 
-const { Buffer, kMaxLength } = require('buffer');
+const { Buffer } = require('buffer');
 const {
   codes: {
     ERR_FS_INVALID_SYMLINK_TYPE,
@@ -476,9 +476,8 @@ const validateOffsetLengthWrite = hideStackFrames(
       throw new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset);
     }
 
-    const max = byteLength > kMaxLength ? kMaxLength : byteLength;
-    if (length > max - offset) {
-      throw new ERR_OUT_OF_RANGE('length', `<= ${max - offset}`, length);
+    if (length > byteLength - offset) {
+      throw new ERR_OUT_OF_RANGE('length', `<= ${byteLength - offset}`, length);
     }
   }
 );

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -88,7 +88,7 @@ constexpr char kPathSeparator = '/';
 const char* const kPathSeparator = "\\/";
 #endif
 
-#define GET_OFFSET(a) ((a)->IsNumber() ? (a).As<Integer>()->Value() : -1)
+#define GET_OFFSET(a) (IsSafeJsInt(a) ? (a).As<Integer>()->Value() : -1)
 #define TRACE_NAME(name) "fs.sync." #name
 #define GET_TRACE_ENABLED                                                  \
   (*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED                             \
@@ -1669,9 +1669,11 @@ static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
   char* buffer_data = Buffer::Data(buffer_obj);
   size_t buffer_length = Buffer::Length(buffer_obj);
 
-  CHECK(args[2]->IsInt32());
-  const size_t off = static_cast<size_t>(args[2].As<Int32>()->Value());
-  CHECK_LE(off, buffer_length);
+  CHECK(IsSafeJsInt(args[2]));
+  const int64_t off_64 = args[2].As<Integer>()->Value();
+  CHECK_GE(off_64, 0);
+  CHECK_LE(static_cast<uint64_t>(off_64), buffer_length);
+  const size_t off = static_cast<size_t>(off_64);
 
   CHECK(args[3]->IsInt32());
   const size_t len = static_cast<size_t>(args[3].As<Int32>()->Value());

diff --git a/test/parallel/test-fs-util-validateoffsetlengthwrite.js b/test/parallel/test-fs-util-validateoffsetlengthwrite.js
@@ -22,22 +22,6 @@ const { kMaxLength } = require('buffer');
   );
 }
 
-// RangeError when byteLength > kMaxLength, and length > kMaxLength - offset .
-{
-  const offset = kMaxLength;
-  const length = 100;
-  const byteLength = kMaxLength + 1;
-  common.expectsError(
-    () => validateOffsetLengthWrite(offset, length, byteLength),
-    {
-      code: 'ERR_OUT_OF_RANGE',
-      type: RangeError,
-      message: 'The value of "length" is out of range. ' +
-               `It must be <= ${kMaxLength - offset}. Received ${length}`
-    }
-  );
-}
-
 // RangeError when byteLength < kMaxLength, and length > byteLength - offset .
 {
   const offset = kMaxLength - 150;

diff --git a/test/parallel/test-fs-write-buffer.js b/test/parallel/test-fs-write-buffer.js
@@ -148,3 +148,27 @@ tmpdir.refresh();
     fs.write(fd, Uint8Array.from(expected), cb);
   }));
 }
+
+// fs.write with invalid offset type
+{
+  const filename = path.join(tmpdir.path, 'write7.txt');
+  fs.open(filename, 'w', 0o644, common.mustCall((err, fd) => {
+    assert.ifError(err);
+
+    assert.throws(() => {
+      fs.write(fd,
+               Buffer.from('abcd'),
+               NaN,
+               expected.length,
+               0,
+               common.mustNotCall());
+    }, {
+      code: 'ERR_OUT_OF_RANGE',
+      name: 'RangeError',
+      message: 'The value of "offset" is out of range. ' +
+               'It must be an integer. Received NaN'
+    });
+
+    fs.closeSync(fd);
+  }));
+}