From ff4226a3cd20ef6cfb155ca206c745785b6e098f Mon Sep 17 00:00:00 2001
From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com>
Date: Fri, 16 Aug 2024 09:48:57 -0700
Subject: fix(node/fs): Use correct offset and length in node:fs.read and write
 (#25049)

My fix in #25030 was buggy, I forgot to pass the `byteOffset` and
`byteLength`. Whoops.

I also discovered that fs.read was not respecting the `offset` argument,
and we were constructing a new `Buffer` for the callback instead of just
passing the original one (which is what node does, and the @types/node
definitions also indicate the callback should get the same type).
Fixes #25028.
---
 tests/unit_node/_fs/_fs_read_test.ts | 100 ++++++++++++++++++++++++++---------
 1 file changed, 75 insertions(+), 25 deletions(-)

(limited to 'tests/unit_node/_fs/_fs_read_test.ts')

diff --git a/tests/unit_node/_fs/_fs_read_test.ts b/tests/unit_node/_fs/_fs_read_test.ts
index 288e4a57c..867ec01c5 100644
--- a/tests/unit_node/_fs/_fs_read_test.ts
+++ b/tests/unit_node/_fs/_fs_read_test.ts
@@ -1,6 +1,7 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 /// <reference types="npm:@types/node" />
 import {
+  assert,
   assertEquals,
   assertFalse,
   assertMatch,
@@ -12,23 +13,23 @@ import { Buffer } from "node:buffer";
 import * as path from "@std/path";
 import { closeSync } from "node:fs";
 
-async function readTest(
+async function readTest<T extends NodeJS.ArrayBufferView>(
   testData: string,
-  buffer: NodeJS.ArrayBufferView,
+  buffer: T,
   offset: number,
   length: number,
   position: number | null = null,
   expected: (
     fd: number,
     bytesRead: number | null,
-    data: ArrayBufferView | undefined,
+    data: T | undefined,
   ) => void,
 ) {
   let fd1 = 0;
   await new Promise<{
     fd: number;
     bytesRead: number | null;
-    data: ArrayBufferView | undefined;
+    data: T | undefined;
   }>((resolve, reject) => {
     open(testData, "r", (err, fd) => {
       if (err) reject(err);
@@ -323,33 +324,82 @@ Deno.test({
 });
 
 Deno.test({
-  name: "accepts non Uint8Array buffer",
+  name: "read with offset TypedArray buffers",
   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");
+    const bufConstructors = [
+      Int8Array,
+      Uint8Array,
+    ];
+    const offsets = [0, 24, 48];
+
+    const resetBuffer = () => {
+      new Uint8Array(buffer).fill(0);
+    };
+    const decoder = new TextDecoder();
+
+    for (const constr of bufConstructors) {
+      // test combinations of buffers internally offset from their backing array buffer,
+      // and also offset in the read call
+      for (const innerOffset of offsets) {
+        for (const offset of offsets) {
+          // test read
+          resetBuffer();
+          const buf = new constr(
+            buffer,
+            innerOffset,
+          );
+          await readTest(
+            testData,
+            buf,
+            offset,
+            buf.byteLength - offset,
+            null,
+            (_fd, bytesRead, data) => {
+              assert(data);
+              assert(bytesRead);
+              assertStrictEquals(bytesRead, 11);
+              assertEquals(data == buf, true);
+              const got = decoder.decode(
+                data.subarray(
+                  offset,
+                  offset + bytesRead,
+                ),
+              );
+              const want = "hello world";
+              assertEquals(got.length, want.length);
+              assertEquals(
+                got,
+                want,
+              );
+            },
+          );
+
+          // test readSync
+          resetBuffer();
+          const fd = openSync(testData, "r");
+          try {
+            const bytesRead = readSync(
+              fd,
+              buf,
+              offset,
+              buf.byteLength - offset,
+              null,
+            );
 
-      assertEquals(buf.slice(0, nRead), new Int8Array(expected.buffer));
-    } finally {
-      closeSync(fd);
+            assertStrictEquals(bytesRead, 11);
+            assertEquals(
+              decoder.decode(buf.subarray(offset, offset + bytesRead)),
+              "hello world",
+            );
+          } finally {
+            closeSync(fd);
+          }
+        }
+      }
     }
   },
 });
-- 
cgit v1.2.3