fs: add read(buffer, options) signatures

This adds the following: `fs.read(fd, buffer[, options], callback)` `filehandle.read(buffer[, options])`
nodejs · Apr 17, 2022 · c69abaa · c69abaa
1 parent c3a581c
commit c69abaa
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 30 deletions.
diff --git a/lib/fs.js b/lib/fs.js
@@ -605,21 +605,30 @@ function openSync(path, flags, mode) {
  *   ) => any} callback
  * @returns {void}
  */
-function read(fd, buffer, offset, length, position, callback) {
+function read(fd, buffer, offsetOrOptions, length, position, callback) {
   fd = getValidatedFd(fd);
 
-  if (arguments.length <= 3) {
-    // Assume fs.read(fd, options, callback)
-    let options = ObjectCreate(null);
+  let offset = offsetOrOptions;
+  if (arguments.length === 4 && typeof offsetOrOptions === 'object') {
+    // This is fs.read(fd, buffer, options, callback)
+    callback = length;
+    ({
+      offset = 0,
+      length = buffer.byteLength - offset,
+      position = null
+    } = offsetOrOptions ?? ObjectCreate(null));
+  } else if (arguments.length <= 3) {
+    // Assume fs.read(fd, params, callback)
+    let params = ObjectCreate(null);
     if (arguments.length < 3) {
       // This is fs.read(fd, callback)
       // buffer will be the callback
       callback = buffer;
     } else {
       // This is fs.read(fd, {}, callback)
-      // buffer will be the options object
+      // buffer will be the params object
       // offset is the callback
-      options = buffer;
+      params = buffer;
       callback = offset;
     }
 
@@ -628,7 +637,7 @@ function read(fd, buffer, offset, length, position, callback) {
       offset = 0,
       length = buffer.byteLength - offset,
       position = null
-    } = options);
+    } = params);
   }
 
   validateBuffer(buffer);
@@ -686,23 +695,21 @@ ObjectDefineProperty(read, internalUtil.customPromisifyArgs,
  *   }} [offset]
  * @returns {number}
  */
-function readSync(fd, buffer, offset, length, position) {
+function readSync(fd, buffer, offsetOrOptions, length, position) {
   fd = getValidatedFd(fd);
 
   validateBuffer(buffer);
 
-  if (arguments.length <= 3) {
-    // Assume fs.readSync(fd, buffer, options)
-    const options = offset || ObjectCreate(null);
-
+  let offset = offsetOrOptions;
+  if (typeof offset === 'object') {
     ({
       offset = 0,
       length = buffer.byteLength - offset,
       position = null
-    } = options);
+    } = offsetOrOptions ?? ObjectCreate(null));
   }
 
-  if (offset == null) {
+  if (offset === false || offset === undefined) {
     offset = 0;
   } else {
     validateInteger(offset, 'offset', 0);

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -508,21 +508,31 @@ async function open(path, flags, mode) {
                                  flagsNumber, mode, kUsePromises));
 }
 
-async function read(handle, bufferOrOptions, offset, length, position) {
-  let buffer = bufferOrOptions;
+async function read(handle, bufferOrParams, offset, length, position) {
+  let buffer = bufferOrParams;
   if (!isArrayBufferView(buffer)) {
-    bufferOrOptions ??= ObjectCreate(null);
+    // This is fh.read(params)
+    bufferOrParams ??= ObjectCreate(null);
     ({
       buffer = Buffer.alloc(16384),
       offset = 0,
       length = buffer.byteLength - offset,
       position = null
-    } = bufferOrOptions);
+    } = bufferOrParams);
 
     validateBuffer(buffer);
   }
 
-  if (offset == null) {
+  if (typeof offset === 'object') {
+    // This is fh.read(buffer, options)
+    ({
+      offset = 0,
+      length = buffer.byteLength - offset,
+      position = null
+    } = offset ?? ObjectCreate(null));
+  }
+
+  if (offset === false || offset === undefined) {
     offset = 0;
   } else {
     validateInteger(offset, 'offset', 0);

diff --git a/test/parallel/test-fs-readSync-optional-params.js b/test/parallel/test-fs-readSync-optional-params.js
@@ -31,26 +31,16 @@ for (const options of [
   { length: expected.length, position: 0 },
   { offset: 0, length: expected.length, position: 0 },
 
-  { offset: null },
+  { offset: false },
   { position: null },
   { position: -1 },
   { position: 0n },
 
   // Test default params
   {},
   null,
-  undefined,
-
-  // Test if bad params are interpreted as default (not mandatory)
-  false,
-  true,
-  Infinity,
-  42n,
-  Symbol(),
 
   // Test even more malicious corner cases
-  '4'.repeat(expected.length),
-  new String('4444'),
   [4, 4, 4, 4],
 ]) {
   runTest(Buffer.allocUnsafe(expected.length), options);