fix(node/fs): Enable test-fs-read-zero-length.js and test-fs-read-type.js

node/_fs/_fs_read.ts

@@ -1,10 +1,8 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 import { Buffer } from "../buffer.ts";
-import { assert } from "../../testing/asserts.ts";
-import {
-  ERR_INVALID_ARG_TYPE,
-  ERR_INVALID_ARG_VALUE,
-} from "../internal/errors.ts";
+import { ERR_INVALID_ARG_TYPE, ERR_OUT_OF_RANGE } from "../internal/errors.ts";
+import { validateOffsetLengthRead, validatePosition } from "./_fs_util.ts";
+import { validateBuffer } from "../internal/validators.mjs";
 
 type readOptions = {
   buffer: Buffer | Uint8Array;
@@ -66,11 +64,12 @@ export function read(
     cb = optOrBufferOrCb;
   } else {
     offset = offsetOrCallback as number;
+    if (Number.isNaN(offset)) {
+      throw new ERR_OUT_OF_RANGE("offset", "an integer", offset);
+    }
     cb = callback;
   }
 
-  if (!cb) throw new ERR_INVALID_ARG_TYPE("cb", "Callback", cb);
-
   if (
     optOrBufferOrCb instanceof Buffer || optOrBufferOrCb instanceof Uint8Array
   ) {
@@ -82,28 +81,27 @@ export function read(
     position = null;
   } else {
     const opt = optOrBufferOrCb as readOptions;
-    offset = opt.offset ?? 0;
-    buffer = opt.buffer ?? Buffer.alloc(16384);
-    length = opt.length ?? buffer.byteLength;
-    position = opt.position ?? null;
+    if (opt.buffer instanceof Buffer || opt.buffer instanceof Uint8Array) {
+      offset = opt.offset ?? 0;
+      buffer = opt.buffer ?? Buffer.alloc(16384);
+      length = opt.length ?? buffer.byteLength;
+      position = opt.position ?? null;
+    } else {
+      throw new ERR_INVALID_ARG_TYPE("buffer", [
+        "Buffer",
+        "TypedArray",
+        "DataView",
+      ], optOrBufferOrCb);
+    }
   }
 
-  assert(offset >= 0, "offset should be greater or equal to 0");
-  assert(
-    offset + length <= buffer.byteLength,
-    `buffer doesn't have enough data: byteLength = ${buffer.byteLength}, offset + length = ${
-      offset + length
-    }`,
-  );
-
-  if (buffer.byteLength == 0) {
-    throw new ERR_INVALID_ARG_VALUE(
-      "buffer",
-      buffer,
-      "is empty and cannot be written",
-    );
+  if (position == null) {
+    position = -1;
   }
 
+  validatePosition(position);
+  validateOffsetLengthRead(offset, length, buffer.byteLength);
+
   let err: Error | null = null,
     numberOfBytesRead: number | null = null;
 
@@ -123,6 +121,8 @@ export function read(
     err = error instanceof Error ? error : new Error("[non-error thrown]");
   }
 
+  if (!cb) throw new ERR_INVALID_ARG_TYPE("cb", "Callback", cb);
+
   if (err) {
     (callback as (err: Error) => void)(err);
   } else {
@@ -154,34 +154,34 @@ export function readSync(
 ): number {
   let offset = 0;
 
-  if (length == null) {
-    length = 0;
+  if (typeof fd !== "number") {
+    throw new ERR_INVALID_ARG_TYPE("fd", "number", fd);
   }
 
-  if (buffer.byteLength == 0) {
-    throw new ERR_INVALID_ARG_VALUE(
-      "buffer",
-      buffer,
-      "is empty and cannot be written",
-    );
+  validateBuffer(buffer);
+
+  if (length == null) {
+    length = 0;
   }
 
   if (typeof offsetOrOpt === "number") {
     offset = offsetOrOpt;
+    if (Number.isNaN(offsetOrOpt)) {
+      throw new ERR_OUT_OF_RANGE("offset", "an integer", offset);
+    }
   } else {
     const opt = offsetOrOpt as readSyncOptions;
     offset = opt.offset ?? 0;
     length = opt.length ?? buffer.byteLength;
     position = opt.position ?? null;
   }
 
-  assert(offset >= 0, "offset should be greater or equal to 0");
-  assert(
-    offset + length <= buffer.byteLength,
-    `buffer doesn't have enough data: byteLength = ${buffer.byteLength}, offset + length = ${
-      offset + length
-    }`,
-  );
+  if (position == null) {
+    position = -1;
+  }
+
+  validatePosition(position);
+  validateOffsetLengthRead(offset, length, buffer.byteLength);
 
   let currentPosition = 0;
   if (typeof position === "number" && position >= 0) {

node/_fs/_fs_util.ts

@@ -0,0 +1,48 @@
+// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
+// Copyright Joyent and Node contributors. All rights reserved. MIT license.
+
+import {
+  ERR_INVALID_ARG_TYPE,
+  ERR_OUT_OF_RANGE,
+  hideStackFrames,
+} from "../internal/errors.ts";
+import { validateInteger } from "../internal/validators.mjs";
+
+export const validatePosition = hideStackFrames((position: unknown) => {
+  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);
+  }
+});
+
+export const validateOffsetLengthRead = hideStackFrames(
+  (offset: number, length: number, bufferLength: number) => {
+    if (offset < 0) {
+      throw new ERR_OUT_OF_RANGE("offset", ">= 0", offset);
+    }
+    if (length < 0) {
+      throw new ERR_OUT_OF_RANGE("length", ">= 0", length);
+    }
+    if (offset + length > bufferLength) {
+      throw new ERR_OUT_OF_RANGE(
+        "length",
+        `<= ${bufferLength - offset}`,
+        length,
+      );
+    }
+  },
+);
+
+export default {
+  validatePosition,
+  validateOffsetLengthRead,
+};

node/_tools/config.json

@@ -291,6 +291,8 @@
       "test-fs-read-stream-patch-open.js",
       "test-fs-read-stream-resume.js",
       "test-fs-read-stream-throw-type-error.js",
+      "test-fs-read-type.js",
+      "test-fs-read-zero-length.js",
       "test-fs-read.js",
       "test-fs-readdir-stack-overflow.js",
       "test-fs-readdir.js",