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

…type.js` (#2692)
denoland · Sep 25, 2022 · 6913e73 · 6913e73
1 parent b7b7d76
commit 6913e73
Show file tree

Hide file tree

Showing 5 changed files with 330 additions and 51 deletions.
diff --git a/node/_fs/_fs_read.ts b/node/_fs/_fs_read.ts
@@ -1,10 +1,11 @@
 // 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 } from "../internal/errors.ts";
 import {
-  ERR_INVALID_ARG_TYPE,
-  ERR_INVALID_ARG_VALUE,
-} from "../internal/errors.ts";
+  validateOffsetLengthRead,
+  validatePosition,
+} from "../internal/fs/utils.mjs";
+import { validateBuffer, validateInteger } from "../internal/validators.mjs";
 
 type readOptions = {
   buffer: Buffer | Uint8Array;
@@ -66,11 +67,10 @@ export function read(
     cb = optOrBufferOrCb;
   } else {
     offset = offsetOrCallback as number;
+    validateInteger(offset, "offset", 0);
     cb = callback;
   }
 
-  if (!cb) throw new ERR_INVALID_ARG_TYPE("cb", "Callback", cb);
-
   if (
     optOrBufferOrCb instanceof Buffer || optOrBufferOrCb instanceof Uint8Array
   ) {
@@ -82,28 +82,32 @@ export function read(
     position = null;
   } else {
     const opt = optOrBufferOrCb as readOptions;
+    if (
+      !(opt.buffer instanceof Buffer) && !(opt.buffer instanceof Uint8Array)
+    ) {
+      if (opt.buffer === null) {
+        // @ts-ignore: Intentionally create TypeError for passing test-fs-read.js#L87
+        length = opt.buffer.byteLength;
+      }
+      throw new ERR_INVALID_ARG_TYPE("buffer", [
+        "Buffer",
+        "TypedArray",
+        "DataView",
+      ], optOrBufferOrCb);
+    }
     offset = opt.offset ?? 0;
     buffer = opt.buffer ?? Buffer.alloc(16384);
     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 (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 +127,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 +160,32 @@ 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;
+    validateInteger(offset, "offset", 0);
   } 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) {

diff --git a/node/_tools/config.json b/node/_tools/config.json
@@ -46,7 +46,6 @@
       "test-fs-append-file.js",
       "test-fs-chmod-mask.js",
       "test-fs-chmod.js",
-      "test-fs-read.js",
       "test-fs-rmdir-recursive.js",
       "test-fs-write-file.js",
       "test-fs-write.js",
@@ -309,6 +308,8 @@
       "test-fs-read-stream-resume.js",
       "test-fs-read-stream-throw-type-error.js",
       "test-fs-read-stream.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",