diff --git a/ext/node/polyfills/_fs/_fs_read.ts b/ext/node/polyfills/_fs/_fs_read.ts index 7b3d9fccdb..90255195fb 100644 --- a/ext/node/polyfills/_fs/_fs_read.ts +++ b/ext/node/polyfills/_fs/_fs_read.ts @@ -28,7 +28,7 @@ type readSyncOptions = { type BinaryCallback = ( err: Error | null, bytesRead: number | null, - data?: Buffer, + data?: ArrayBufferView, ) => void; type Callback = BinaryCallback; @@ -56,7 +56,7 @@ export function read( ) { let cb: Callback | undefined; let offset = 0, - buffer: Buffer | Uint8Array; + buffer: ArrayBufferView; if (typeof fd !== "number") { throw new ERR_INVALID_ARG_TYPE("fd", "number", fd); @@ -79,7 +79,7 @@ export function read( if ( isArrayBufferView(optOrBufferOrCb) ) { - buffer = arrayBufferViewToUint8Array(optOrBufferOrCb); + buffer = optOrBufferOrCb; } else if (typeof optOrBufferOrCb === "function") { offset = 0; buffer = Buffer.alloc(16384); @@ -99,10 +99,10 @@ export function read( if (opt.buffer === undefined) { buffer = Buffer.alloc(16384); } else { - buffer = arrayBufferViewToUint8Array(opt.buffer); + buffer = opt.buffer; } offset = opt.offset ?? 0; - length = opt.length ?? buffer.byteLength; + length = opt.length ?? buffer.byteLength - offset; position = opt.position ?? null; } @@ -123,12 +123,18 @@ export function read( // We use sync calls below to avoid being affected by others during // these calls. fs.seekSync(fd, position, io.SeekMode.Start); - nread = io.readSync(fd, buffer); + nread = io.readSync( + fd, + arrayBufferViewToUint8Array(buffer).subarray(offset, offset + length), + ); fs.seekSync(fd, currentPosition, io.SeekMode.Start); } else { - nread = await io.read(fd, buffer); + nread = await io.read( + fd, + arrayBufferViewToUint8Array(buffer).subarray(offset, offset + length), + ); } - cb(null, nread ?? 0, Buffer.from(buffer.buffer, offset, length)); + cb(null, nread ?? 0, buffer); } catch (error) { cb(error as Error, null); } @@ -162,8 +168,6 @@ export function readSync( validateBuffer(buffer); - buffer = arrayBufferViewToUint8Array(buffer); - if (length == null) { length = 0; } @@ -174,7 +178,7 @@ export function readSync( } else if (offsetOrOpt !== undefined) { const opt = offsetOrOpt as readSyncOptions; offset = opt.offset ?? 0; - length = opt.length ?? buffer.byteLength; + length = opt.length ?? buffer.byteLength - offset; position = opt.position ?? null; } @@ -191,7 +195,10 @@ export function readSync( fs.seekSync(fd, position, io.SeekMode.Start); } - const numberOfBytesRead = io.readSync(fd, buffer); + const numberOfBytesRead = io.readSync( + fd, + arrayBufferViewToUint8Array(buffer).subarray(offset, offset + length), + ); if (typeof position === "number" && position >= 0) { fs.seekSync(fd, currentPosition, io.SeekMode.Start); diff --git a/ext/node/polyfills/internal/fs/utils.mjs b/ext/node/polyfills/internal/fs/utils.mjs index 53184b9c30..b0ef34873e 100644 --- a/ext/node/polyfills/internal/fs/utils.mjs +++ b/ext/node/polyfills/internal/fs/utils.mjs @@ -990,7 +990,11 @@ export const validatePosition = hideStackFrames((position) => { export const arrayBufferViewToUint8Array = hideStackFrames( (buffer) => { if (!(buffer instanceof Uint8Array)) { - return new Uint8Array(buffer.buffer); + return new Uint8Array( + buffer.buffer, + buffer.byteOffset, + buffer.byteLength, + ); } return buffer; }, diff --git a/tests/unit_node/_fs/_fs_handle_test.ts b/tests/unit_node/_fs/_fs_handle_test.ts index ea6af9c299..755e091fd7 100644 --- a/tests/unit_node/_fs/_fs_handle_test.ts +++ b/tests/unit_node/_fs/_fs_handle_test.ts @@ -38,11 +38,15 @@ Deno.test("read specify opt", async function () { buffer: new Buffer(byteLength), offset: 6, length: 5, + position: 6, }; let res = await fileHandle.read(opt); - assertEquals(res.bytesRead, byteLength); - assertEquals(new TextDecoder().decode(res.buffer as Uint8Array), "world"); + assertEquals(res.bytesRead, 5); + assertEquals( + new TextDecoder().decode(res.buffer.subarray(6) as Uint8Array), + "world", + ); const opt2 = { buffer: new Buffer(byteLength), @@ -51,8 +55,11 @@ Deno.test("read specify opt", async function () { }; res = await fileHandle.read(opt2); - assertEquals(res.bytesRead, byteLength); - assertEquals(decoder.decode(res.buffer as Uint8Array), "hello"); + assertEquals(res.bytesRead, 5); + assertEquals( + decoder.decode(res.buffer.subarray(0, 5) as Uint8Array), + "hello", + ); await fileHandle.close(); }); diff --git a/tests/unit_node/_fs/_fs_read_test.ts b/tests/unit_node/_fs/_fs_read_test.ts index 288e4a57c5..867ec01c5c 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. /// 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( 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]; - assertEquals(buf.slice(0, nRead), new Int8Array(expected.buffer)); - } finally { - closeSync(fd); + 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, + ); + + assertStrictEquals(bytesRead, 11); + assertEquals( + decoder.decode(buf.subarray(offset, offset + bytesRead)), + "hello world", + ); + } finally { + closeSync(fd); + } + } + } } }, }); diff --git a/tests/unit_node/_fs/_fs_write_test.ts b/tests/unit_node/_fs/_fs_write_test.ts index 3ce030bc60..a140548e12 100644 --- a/tests/unit_node/_fs/_fs_write_test.ts +++ b/tests/unit_node/_fs/_fs_write_test.ts @@ -75,7 +75,7 @@ Deno.test({ }); Deno.test({ - name: "Accepts non Uint8Array buffer", + name: "write with offset TypedArray buffers", async fn() { const tempFile: string = Deno.makeTempFileSync(); using file = Deno.openSync(tempFile, { @@ -83,32 +83,56 @@ Deno.test({ write: true, read: true, }); + const arrayBuffer = new ArrayBuffer(128); + const resetBuffer = () => { + new Uint8Array(arrayBuffer).fill(0); + }; + const bufConstructors = [ + Int8Array, + Uint8Array, + ]; + const offsets = [0, 24, 48]; const bytes = [0, 1, 2, 3, 4]; - const buffer = new Int8Array(bytes); - for (let i = 0; i < buffer.length; i++) { - buffer[i] = i; + for (const constr of bufConstructors) { + // test combinations of buffers internally offset from their backing array buffer, + // and also offset in the write call + for (const innerOffset of offsets) { + for (const offset of offsets) { + resetBuffer(); + const buffer = new constr( + arrayBuffer, + innerOffset, + offset + bytes.length, + ); + for (let i = 0; i < bytes.length; i++) { + buffer[offset + i] = i; + } + let nWritten = writeSync(file.rid, buffer, offset, bytes.length, 0); + + let data = Deno.readFileSync(tempFile); + + assertEquals(nWritten, bytes.length); + console.log(constr, innerOffset, offset); + assertEquals(data, new Uint8Array(bytes)); + nWritten = await new Promise((resolve, reject) => + write( + file.rid, + buffer, + offset, + bytes.length, + 0, + (err: unknown, nwritten: number) => { + if (err) return reject(err); + resolve(nwritten); + }, + ) + ); + + data = Deno.readFileSync(tempFile); + assertEquals(nWritten, 5); + assertEquals(data, new Uint8Array(bytes)); + } + } } - 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)); }, });