fs: allow position parameter to be a BigInt in read and readSync

Fixes: #36185 PR-URL: #36190 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Zeyu Yang <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Jan 21, 2021 · c4cdf1d · c4cdf1d
1 parent 02a8f52
commit c4cdf1d
Show file tree

Hide file tree

Showing 4 changed files with 127 additions and 10 deletions.
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -2921,7 +2921,7 @@ changes:
 * `buffer` {Buffer|TypedArray|DataView}
 * `offset` {integer}
 * `length` {integer}
-* `position` {integer}
+* `position` {integer|bigint}
 * `callback` {Function}
   * `err` {Error}
   * `bytesRead` {integer}
@@ -2966,7 +2966,7 @@ changes:
   * `buffer` {Buffer|TypedArray|DataView} **Default:** `Buffer.alloc(16384)`
   * `offset` {integer} **Default:** `0`
   * `length` {integer} **Default:** `buffer.length`
-  * `position` {integer} **Default:** `null`
+  * `position` {integer|bigint} **Default:** `null`
 * `callback` {Function}
   * `err` {Error}
   * `bytesRead` {integer}
@@ -3263,7 +3263,7 @@ changes:
 * `buffer` {Buffer|TypedArray|DataView}
 * `offset` {integer}
 * `length` {integer}
-* `position` {integer}
+* `position` {integer|bigint}
 * Returns: {number}
 
 Returns the number of `bytesRead`.
@@ -3290,7 +3290,7 @@ changes:
 * `options` {Object}
   * `offset` {integer} **Default:** `0`
   * `length` {integer} **Default:** `buffer.length`
-  * `position` {integer} **Default:** `null`
+  * `position` {integer|bigint} **Default:** `null`
 * Returns: {number}
 
 Returns the number of `bytesRead`.

diff --git a/lib/fs.js b/lib/fs.js
@@ -37,7 +37,6 @@ const {
   BigIntPrototypeToString,
   MathMax,
   Number,
-  NumberIsSafeInteger,
   ObjectCreate,
   ObjectDefineProperties,
   ObjectDefineProperty,
@@ -75,7 +74,8 @@ const {
     ERR_FS_FILE_TOO_LARGE,
     ERR_INVALID_ARG_VALUE,
     ERR_INVALID_ARG_TYPE,
-    ERR_FEATURE_UNAVAILABLE_ON_PLATFORM
+    ERR_FEATURE_UNAVAILABLE_ON_PLATFORM,
+    ERR_OUT_OF_RANGE,
   },
   hideStackFrames,
   uvErrmapGet,
@@ -545,9 +545,23 @@ function read(fd, buffer, offset, length, position, callback) {
 
   validateOffsetLengthRead(offset, length, buffer.byteLength);
 
-  if (!NumberIsSafeInteger(position))
+  if (position == null)
     position = -1;
 
+  if (typeof position === 'number') {
+    validateInteger(position, 'position');
+  } else if (typeof position === 'bigint') {
+    if (!(position >= -(2n ** 63n) && position <= 2n ** 63n - 1n)) {
+      throw new ERR_OUT_OF_RANGE('position',
+                                 `>= ${-(2n ** 63n)} && <= ${2n ** 63n - 1n}`,
+                                 position);
+    }
+  } else {
+    throw new ERR_INVALID_ARG_TYPE('position',
+                                   ['integer', 'bigint'],
+                                   position);
+  }
+
   function wrapper(err, bytesRead) {
     // Retain a reference to buffer so that it can't be GC'ed too soon.
     callback(err, bytesRead || 0, buffer);
@@ -597,9 +611,23 @@ function readSync(fd, buffer, offset, length, position) {
 
   validateOffsetLengthRead(offset, length, buffer.byteLength);
 
-  if (!NumberIsSafeInteger(position))
+  if (position == null)
     position = -1;
 
+  if (typeof position === 'number') {
+    validateInteger(position, 'position');
+  } else if (typeof position === 'bigint') {
+    if (!(position >= -(2n ** 63n) && position <= 2n ** 63n - 1n)) {
+      throw new ERR_OUT_OF_RANGE('position',
+                                 `>= ${-(2n ** 63n)} && <= ${2n ** 63n - 1n}`,
+                                 position);
+    }
+  } else {
+    throw new ERR_INVALID_ARG_TYPE('position',
+                                   ['integer', 'bigint'],
+                                   position);
+  }
+
   const ctx = {};
   const result = binding.read(fd, buffer, offset, length, position,
                               undefined, ctx);

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -51,6 +51,7 @@ namespace node {
 namespace fs {
 
 using v8::Array;
+using v8::BigInt;
 using v8::Boolean;
 using v8::Context;
 using v8::EscapableHandleScope;
@@ -2038,8 +2039,10 @@ static void Read(const FunctionCallbackInfo<Value>& args) {
   const size_t len = static_cast<size_t>(args[3].As<Int32>()->Value());
   CHECK(Buffer::IsWithinBounds(off, len, buffer_length));
 
-  CHECK(IsSafeJsInt(args[4]));
-  const int64_t pos = args[4].As<Integer>()->Value();
+  CHECK(IsSafeJsInt(args[4]) || args[4]->IsBigInt());
+  const int64_t pos = args[4]->IsNumber() ?
+                      args[4].As<Integer>()->Value() :
+                      args[4].As<BigInt>()->Int64Value();
 
   char* buf = buffer_data + off;
   uv_buf_t uvbuf = uv_buf_init(buf, len);

diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js
@@ -76,6 +76,47 @@ assert.throws(() => {
            'It must be >= 0. Received -1'
 });
 
+[true, () => {}, {}, ''].forEach((value) => {
+  assert.throws(() => {
+    fs.read(fd,
+            Buffer.allocUnsafe(expected.length),
+            0,
+            expected.length,
+            value,
+            common.mustNotCall());
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError'
+  });
+});
+
+[0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
+  assert.throws(() => {
+    fs.read(fd,
+            Buffer.allocUnsafe(expected.length),
+            0,
+            expected.length,
+            value,
+            common.mustNotCall());
+  }, {
+    code: 'ERR_OUT_OF_RANGE',
+    name: 'RangeError'
+  });
+});
+
+fs.read(fd,
+        Buffer.allocUnsafe(expected.length),
+        0,
+        expected.length,
+        0n,
+        common.mustCall());
+
+fs.read(fd,
+        Buffer.allocUnsafe(expected.length),
+        0,
+        expected.length,
+        2n ** 53n - 1n,
+        common.mustCall());
 
 assert.throws(
   () => fs.readSync(fd, expected.length, 0, 'utf-8'),
@@ -151,3 +192,48 @@ assert.throws(() => {
   message: 'The value of "length" is out of range. ' +
            'It must be <= 4. Received 5'
 });
+
+[true, () => {}, {}, ''].forEach((value) => {
+  assert.throws(() => {
+    fs.readSync(fd,
+                Buffer.allocUnsafe(expected.length),
+                0,
+                expected.length,
+                value);
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError'
+  });
+});
+
+[0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
+  assert.throws(() => {
+    fs.readSync(fd,
+                Buffer.allocUnsafe(expected.length),
+                0,
+                expected.length,
+                value);
+  }, {
+    code: 'ERR_OUT_OF_RANGE',
+    name: 'RangeError'
+  });
+});
+
+fs.readSync(fd,
+            Buffer.allocUnsafe(expected.length),
+            0,
+            expected.length,
+            0n);
+
+try {
+  fs.readSync(fd,
+              Buffer.allocUnsafe(expected.length),
+              0,
+              expected.length,
+              2n ** 53n - 1n);
+} catch (err) {
+  // On systems where max file size is below 2^53-1, we'd expect a EFBIG error.
+  // This is not using `assert.throws` because the above call should not raise
+  // any error on systems that allows file of that size.
+  if (err.code !== 'EFBIG') throw err;
+}