fix(ext/node): fs.read/fs.readSync and fs.write/fs.writeSync compatibility

ext/node/polyfills/_fs/_fs_common.ts

@@ -15,7 +15,6 @@ import {
   notImplemented,
   TextEncodings,
 } from "ext:deno_node/_utils.ts";
-import { type Buffer } from "node:buffer";
 import { assertEncoding } from "ext:deno_node/internal/fs/utils.mjs";
 
 export type CallbackWithError = (err: ErrnoException | null) => void;
@@ -34,13 +33,6 @@ export type BinaryOptionsArgument =
   | ({ encoding: BinaryEncodings } & FileOptions);
 export type FileOptionsArgument = Encodings | FileOptions;
 
-export type ReadOptions = {
-  buffer: Buffer | ArrayBufferView;
-  offset: number;
-  length: number;
-  position: number | null;
-};
-
 export interface WriteFileOptions extends FileOptions {
   mode?: number;
 }

ext/node/polyfills/_fs/_fs_read.ts

@@ -4,34 +4,33 @@
 // deno-lint-ignore-file prefer-primordials
 
 import { Buffer } from "node:buffer";
-import {
-  ERR_INVALID_ARG_TYPE,
-  ERR_INVALID_ARG_VALUE,
-} from "ext:deno_node/internal/errors.ts";
+import { ERR_INVALID_ARG_VALUE } from "ext:deno_node/internal/errors.ts";
 import * as io from "ext:deno_io/12_io.js";
-import { ReadOptions } from "ext:deno_node/_fs/_fs_common.ts";
 import {
   arrayBufferViewToUint8Array,
+  getValidatedFd,
   validateOffsetLengthRead,
   validatePosition,
 } from "ext:deno_node/internal/fs/utils.mjs";
 import {
   validateBuffer,
+  validateFunction,
   validateInteger,
+  validateObject,
 } from "ext:deno_node/internal/validators.mjs";
 import { isArrayBufferView } from "ext:deno_node/internal/util/types.ts";
 import { op_fs_seek_async, op_fs_seek_sync } from "ext:core/ops";
 import { primordials } from "ext:core/mod.js";
-import { customPromisifyArgs } from "ext:deno_node/internal/util.mjs";
+import {
+  customPromisifyArgs,
+  kEmptyObject,
+} from "ext:deno_node/internal/util.mjs";
 import * as process from "node:process";
+import type { ReadAsyncOptions, ReadSyncOptions } from "node:fs";
 
 const { ObjectDefineProperty } = primordials;
 
-type readSyncOptions = {
-  offset: number;
-  length: number;
-  position: number | null;
-};
+const validateOptionArgs = { __proto__: null, nullable: true };
 
 type BinaryCallback = (
   err: Error | null,
@@ -43,7 +42,13 @@ type Callback = BinaryCallback;
 export function read(fd: number, callback: Callback): void;
 export function read(
   fd: number,
-  options: ReadOptions,
+  options: ReadAsyncOptions<NodeJS.ArrayBufferView>,
+  callback: Callback,
+): void;
+export function read(
+  fd: number,
+  buffer: ArrayBufferView,
+  options: ReadSyncOptions,
   callback: Callback,
 ): void;
 export function read(
@@ -56,67 +61,68 @@ export function read(
 ): void;
 export function read(
   fd: number,
-  optOrBufferOrCb?: ArrayBufferView | ReadOptions | Callback,
-  offsetOrCallback?: number | Callback,
-  length?: number,
+  optOrBufferOrCb?:
+    | ArrayBufferView
+    | ReadAsyncOptions<NodeJS.ArrayBufferView>
+    | Callback,
+  offsetOrOpt?:
+    | number
+    | ReadAsyncOptions<NodeJS.ArrayBufferView>
+    | Callback,
+  lengthOrCb?: number | Callback,
   position?: number | null,
   callback?: Callback,
 ) {
-  let cb: Callback | undefined;
-  let offset = 0,
-    buffer: ArrayBufferView;
+  fd = getValidatedFd(fd);
 
-  if (typeof fd !== "number") {
-    throw new ERR_INVALID_ARG_TYPE("fd", "number", fd);
-  }
+  let offset = offsetOrOpt;
+  let buffer = optOrBufferOrCb;
+  let length = lengthOrCb;
+  let params = null;
+  if (arguments.length <= 4) {
+    if (arguments.length === 4) {
+      // This is fs.read(fd, buffer, options, callback)
+      validateObject(offsetOrOpt, "options", validateOptionArgs);
+      callback = length as Callback;
+      params = offsetOrOpt;
+    } else if (arguments.length === 3) {
+      // This is fs.read(fd, bufferOrParams, callback)
+      if (!isArrayBufferView(buffer)) {
+        // This is fs.read(fd, params, callback)
+        params = buffer;
+        ({ buffer = Buffer.alloc(16384) } = params ?? kEmptyObject);
+      }
+      callback = offsetOrOpt as Callback;
+    } else {
+      // This is fs.read(fd, callback)
+      callback = buffer as Callback;
+      buffer = Buffer.alloc(16384);
+    }
 
-  if (length == null) {
-    length = 0;
+    if (params !== undefined) {
+      validateObject(params, "options", validateOptionArgs);
+    }
+    ({
+      offset = 0,
+      length = buffer?.byteLength - (offset as number),
+      position = null,
+    } = params ?? kEmptyObject);
   }
 
-  if (typeof offsetOrCallback === "function") {
-    cb = offsetOrCallback;
-  } else if (typeof optOrBufferOrCb === "function") {
-    cb = optOrBufferOrCb;
-  } else {
-    offset = offsetOrCallback as number;
-    validateInteger(offset, "offset", 0);
-    cb = callback;
-  }
+  validateBuffer(buffer);
+  validateFunction(callback, "cb");
 
-  if (
-    isArrayBufferView(optOrBufferOrCb)
-  ) {
-    buffer = optOrBufferOrCb;
-  } else if (typeof optOrBufferOrCb === "function") {
+  if (offset == null) {
     offset = 0;
-    buffer = Buffer.alloc(16384);
-    length = buffer.byteLength;
-    position = null;
   } else {
-    const opt = optOrBufferOrCb as ReadOptions;
-    if (
-      !isArrayBufferView(opt.buffer)
-    ) {
-      throw new ERR_INVALID_ARG_TYPE("buffer", [
-        "Buffer",
-        "TypedArray",
-        "DataView",
-      ], optOrBufferOrCb);
-    }
-    if (opt.buffer === undefined) {
-      buffer = Buffer.alloc(16384);
-    } else {
-      buffer = opt.buffer;
-    }
-    offset = opt.offset ?? 0;
-    length = opt.length ?? buffer.byteLength - offset;
-    position = opt.position ?? null;
+    validateInteger(offset, "offset", 0);
   }
 
+  (length as number) |= 0;
+
   if (length === 0) {
     return process.nextTick(function tick() {
-      cb(null, 0, buffer);
+      callback!(null, 0, buffer);
     });
   }
 
@@ -128,15 +134,14 @@ export function read(
     );
   }
 
+  validateOffsetLengthRead(offset, length, buffer.byteLength);
+
   if (position == null) {
     position = -1;
+  } else {
+    validatePosition(position, "position", length as number);
   }
 
-  validatePosition(position);
-  validateOffsetLengthRead(offset, length, buffer.byteLength);
-
-  if (!cb) throw new ERR_INVALID_ARG_TYPE("cb", "Callback", cb);
-
   (async () => {
     try {
       let nread: number | null;
@@ -151,18 +156,24 @@ export function read(
         op_fs_seek_sync(fd, position, io.SeekMode.Start);
         nread = io.readSync(
           fd,
-          arrayBufferViewToUint8Array(buffer).subarray(offset, offset + length),
+          arrayBufferViewToUint8Array(buffer).subarray(
+            offset,
+            offset + (length as number),
+          ),
         );
         op_fs_seek_sync(fd, currentPosition, io.SeekMode.Start);
       } else {
         nread = await io.read(
           fd,
-          arrayBufferViewToUint8Array(buffer).subarray(offset, offset + length),
+          arrayBufferViewToUint8Array(buffer).subarray(
+            offset,
+            offset + (length as number),
+          ),
         );
       }
-      cb(null, nread ?? 0, buffer);
+      callback!(null, nread ?? 0, buffer);
     } catch (error) {
-      cb(error as Error, null);
+      callback!(error as Error, null);
     }
   })();
 }
@@ -183,37 +194,40 @@ export function readSync(
 export function readSync(
   fd: number,
   buffer: ArrayBufferView,
-  opt: readSyncOptions,
+  opt: ReadSyncOptions,
 ): number;
 export function readSync(
   fd: number,
   buffer: ArrayBufferView,
-  offsetOrOpt?: number | readSyncOptions,
+  offsetOrOpt?: number | ReadSyncOptions,
   length?: number,
   position?: number | null,
 ): number {
-  let offset = 0;
-
-  if (typeof fd !== "number") {
-    throw new ERR_INVALID_ARG_TYPE("fd", "number", fd);
-  }
+  fd = getValidatedFd(fd);
 
   validateBuffer(buffer);
 
-  if (length == null) {
-    length = buffer.byteLength;
+  let offset = offsetOrOpt;
+  if (arguments.length <= 3 || typeof offsetOrOpt === "object") {
+    if (offsetOrOpt !== undefined) {
+      validateObject(offsetOrOpt, "options", validateOptionArgs);
+    }
+
+    ({
+      offset = 0,
+      length = buffer.byteLength - (offset as number),
+      position = null,
+    } = offsetOrOpt ?? kEmptyObject);
   }
 
-  if (typeof offsetOrOpt === "number") {
-    offset = offsetOrOpt;
+  if (offset === undefined) {
+    offset = 0;
+  } else {
     validateInteger(offset, "offset", 0);
-  } else if (offsetOrOpt !== undefined) {
-    const opt = offsetOrOpt as readSyncOptions;
-    offset = opt.offset ?? 0;
-    length = opt.length ?? buffer.byteLength - offset;
-    position = opt.position ?? null;
   }
 
+  length! |= 0;
+
   if (length === 0) {
     return 0;
   }
@@ -226,13 +240,14 @@ export function readSync(
     );
   }
 
+  validateOffsetLengthRead(offset, length, buffer.byteLength);
+
   if (position == null) {
     position = -1;
+  } else {
+    validatePosition(position, "position", length);
   }
 
-  validatePosition(position);
-  validateOffsetLengthRead(offset, length, buffer.byteLength);
-
   let currentPosition = 0;
   if (typeof position === "number" && position >= 0) {
     currentPosition = op_fs_seek_sync(fd, 0, io.SeekMode.Current);
@@ -241,7 +256,7 @@ export function readSync(
 
   const numberOfBytesRead = io.readSync(
     fd,
-    arrayBufferViewToUint8Array(buffer).subarray(offset, offset + length),
+    arrayBufferViewToUint8Array(buffer).subarray(offset, offset + length!),
   );
 
   if (typeof position === "number" && position >= 0) {

ext/node/polyfills/_fs/_fs_write.ts

@@ -57,7 +57,7 @@ export function writeSync(
     position: number | null | undefined,
   ) => {
     buffer = arrayBufferViewToUint8Array(buffer);
-    if (typeof position === "number") {
+    if (typeof position === "number" && position >= 0) {
       op_fs_seek_sync(fd, position, io.SeekMode.Start);
     }
     let currentOffset = offset;
@@ -122,7 +122,7 @@ export function write(
     position: number | null | undefined,
   ) => {
     buffer = arrayBufferViewToUint8Array(buffer);
-    if (typeof position === "number") {
+    if (typeof position === "number" && position >= 0) {
       await op_fs_seek_async(fd, position, io.SeekMode.Start);
     }
     let currentOffset = offset;