fix(node/fs): node:fs.read and write should accept typed arrays other than Uint8Array (#25030)

Part of #25028.

Our underlying read/write operations in `io` assume the buffer is a
Uint8Array, but we were passing in other typed arrays (in the case above
it was `Int8Array`).

ext/node/polyfills/_fs/_fs_common.ts

@@ -38,7 +38,7 @@ export type BinaryOptionsArgument =
 export type FileOptionsArgument = Encodings | FileOptions;
 
 export type ReadOptions = {
-  buffer: Buffer | Uint8Array;
+  buffer: Buffer | ArrayBufferView;
   offset: number;
   length: number;
   position: number | null;

ext/node/polyfills/_fs/_fs_read.ts

@@ -9,6 +9,7 @@ import * as io from "ext:deno_io/12_io.js";
 import * as fs from "ext:deno_fs/30_fs.js";
 import { ReadOptions } from "ext:deno_node/_fs/_fs_common.ts";
 import {
+  arrayBufferViewToUint8Array,
   validateOffsetLengthRead,
   validatePosition,
 } from "ext:deno_node/internal/fs/utils.mjs";
@@ -16,6 +17,7 @@ import {
   validateBuffer,
   validateInteger,
 } from "ext:deno_node/internal/validators.mjs";
+import { isArrayBufferView } from "ext:deno_node/internal/util/types.ts";
 
 type readSyncOptions = {
   offset: number;
@@ -38,7 +40,7 @@ export function read(
 ): void;
 export function read(
   fd: number,
-  buffer: Buffer | Uint8Array,
+  buffer: ArrayBufferView,
   offset: number,
   length: number,
   position: number | null,
@@ -46,7 +48,7 @@ export function read(
 ): void;
 export function read(
   fd: number,
-  optOrBufferOrCb?: Buffer | Uint8Array | ReadOptions | Callback,
+  optOrBufferOrCb?: ArrayBufferView | ReadOptions | Callback,
   offsetOrCallback?: number | Callback,
   length?: number,
   position?: number | null,
@@ -75,9 +77,9 @@ export function read(
   }
 
   if (
-    optOrBufferOrCb instanceof Buffer || optOrBufferOrCb instanceof Uint8Array
+    isArrayBufferView(optOrBufferOrCb)
   ) {
-    buffer = optOrBufferOrCb;
+    buffer = arrayBufferViewToUint8Array(optOrBufferOrCb);
   } else if (typeof optOrBufferOrCb === "function") {
     offset = 0;
     buffer = Buffer.alloc(16384);
@@ -86,7 +88,7 @@ export function read(
   } else {
     const opt = optOrBufferOrCb as ReadOptions;
     if (
-      !(opt.buffer instanceof Buffer) && !(opt.buffer instanceof Uint8Array)
+      !isArrayBufferView(opt.buffer)
     ) {
       throw new ERR_INVALID_ARG_TYPE("buffer", [
         "Buffer",
@@ -94,8 +96,12 @@ export function read(
         "DataView",
       ], optOrBufferOrCb);
     }
+    if (opt.buffer === undefined) {
+      buffer = Buffer.alloc(16384);
+    } else {
+      buffer = arrayBufferViewToUint8Array(opt.buffer);
+    }
     offset = opt.offset ?? 0;
-    buffer = opt.buffer ?? Buffer.alloc(16384);
     length = opt.length ?? buffer.byteLength;
     position = opt.position ?? null;
   }
@@ -131,19 +137,19 @@ export function read(
 
 export function readSync(
   fd: number,
-  buffer: Buffer | Uint8Array,
+  buffer: ArrayBufferView,
   offset: number,
   length: number,
   position: number | null,
 ): number;
 export function readSync(
   fd: number,
-  buffer: Buffer | Uint8Array,
+  buffer: ArrayBufferView,
   opt: readSyncOptions,
 ): number;
 export function readSync(
   fd: number,
-  buffer: Buffer | Uint8Array,
+  buffer: ArrayBufferView,
   offsetOrOpt?: number | readSyncOptions,
   length?: number,
   position?: number | null,
@@ -156,6 +162,8 @@ export function readSync(
 
   validateBuffer(buffer);
 
+  buffer = arrayBufferViewToUint8Array(buffer);
+
   if (length == null) {
     length = 0;
   }

ext/node/polyfills/_fs/_fs_write.mjs

@@ -12,6 +12,7 @@ import {
 import * as io from "ext:deno_io/12_io.js";
 import * as fs from "ext:deno_fs/30_fs.js";
 import {
+  arrayBufferViewToUint8Array,
   getValidatedFd,
   validateOffsetLengthWrite,
   validateStringAfterArrayBufferView,
@@ -23,9 +24,7 @@ export function writeSync(fd, buffer, offset, length, position) {
   fd = getValidatedFd(fd);
 
   const innerWriteSync = (fd, buffer, offset, length, position) => {
-    if (buffer instanceof DataView) {
-      buffer = new Uint8Array(buffer.buffer);
-    }
+    buffer = arrayBufferViewToUint8Array(buffer);
     if (typeof position === "number") {
       fs.seekSync(fd, position, io.SeekMode.Start);
     }
@@ -69,9 +68,7 @@ export function write(fd, buffer, offset, length, position, callback) {
   fd = getValidatedFd(fd);
 
   const innerWrite = async (fd, buffer, offset, length, position) => {
-    if (buffer instanceof DataView) {
-      buffer = new Uint8Array(buffer.buffer);
-    }
+    buffer = arrayBufferViewToUint8Array(buffer);
     if (typeof position === "number") {
       await fs.seek(fd, position, io.SeekMode.Start);
     }

ext/node/polyfills/internal/fs/utils.mjs

@@ -986,6 +986,16 @@ export const validatePosition = hideStackFrames((position) => {
   }
 });
 
+/** @type {(buffer: ArrayBufferView) => Uint8Array} */
+export const arrayBufferViewToUint8Array = hideStackFrames(
+  (buffer) => {
+    if (!(buffer instanceof Uint8Array)) {
+      return new Uint8Array(buffer.buffer);
+    }
+    return buffer;
+  },
+);
+
 export const realpathCacheKey = Symbol("realpathCacheKey");
 export const constants = {
   kIoMaxLength,

tests/unit_node/_fs/_fs_read_test.ts

@@ -1,4 +1,5 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+/// <reference types="npm:@types/node" />
 import {
   assertEquals,
   assertFalse,
@@ -13,21 +14,21 @@ import { closeSync } from "node:fs";
 
 async function readTest(
   testData: string,
-  buffer: Buffer,
+  buffer: NodeJS.ArrayBufferView,
   offset: number,
   length: number,
   position: number | null = null,
   expected: (
     fd: number,
     bytesRead: number | null,
-    data: Buffer | undefined,
+    data: ArrayBufferView | undefined,
   ) => void,
 ) {
   let fd1 = 0;
   await new Promise<{
     fd: number;
     bytesRead: number | null;
-    data: Buffer | undefined;
+    data: ArrayBufferView | undefined;
   }>((resolve, reject) => {
     open(testData, "r", (err, fd) => {
       if (err) reject(err);
@@ -320,3 +321,35 @@ Deno.test({
     closeSync(fd);
   },
 });
+
+Deno.test({
+  name: "accepts non Uint8Array buffer",
+  async fn() {
+    const moduleDir = path.dirname(path.fromFileUrl(import.meta.url));
+    const testData = path.resolve(moduleDir, "testdata", "hello.txt");
+    const buffer = new ArrayBuffer(1024);
+    const buf = new Int8Array(buffer);
+    await readTest(
+      testData,
+      buf,
+      buf.byteOffset,
+      buf.byteLength,
+      null,
+      (_fd, bytesRead, data) => {
+        assertStrictEquals(bytesRead, 11);
+        assertEquals(data instanceof Buffer, true);
+        assertMatch((data as Buffer).toString(), /hello world/);
+      },
+    );
+    const fd = openSync(testData, "r");
+
+    try {
+      const nRead = readSync(fd, buf);
+      const expected = new TextEncoder().encode("hello world");
+
+      assertEquals(buf.slice(0, nRead), new Int8Array(expected.buffer));
+    } finally {
+      closeSync(fd);
+    }
+  },
+});

tests/unit_node/_fs/_fs_write_test.ts

@@ -73,3 +73,42 @@ Deno.test({
     assertEquals(decoder.decode(data), "\x00\x00\x00\x00hello world");
   },
 });
+
+Deno.test({
+  name: "Accepts non Uint8Array buffer",
+  async fn() {
+    const tempFile: string = Deno.makeTempFileSync();
+    using file = Deno.openSync(tempFile, {
+      create: true,
+      write: true,
+      read: true,
+    });
+    const bytes = [0, 1, 2, 3, 4];
+    const buffer = new Int8Array(bytes);
+    for (let i = 0; i < buffer.length; i++) {
+      buffer[i] = i;
+    }
+    let nWritten = writeSync(file.rid, buffer);
+
+    const data = Deno.readFileSync(tempFile);
+
+    assertEquals(nWritten, 5);
+    assertEquals(data, new Uint8Array(bytes));
+
+    nWritten = await new Promise((resolve, reject) =>
+      write(
+        file.rid,
+        buffer,
+        0,
+        5,
+        (err: unknown, nwritten: number) => {
+          if (err) return reject(err);
+          resolve(nwritten);
+        },
+      )
+    );
+
+    assertEquals(nWritten, 5);
+    assertEquals(data, new Uint8Array(bytes));
+  },
+});