Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ext/node/fs): position argument not applied #24009

Merged
merged 1 commit into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix(ext/node/fs): position argument not applied
  • Loading branch information
marvinhagemeister committed May 28, 2024
commit 5f7192a037651d70da49f085d7f97e6560e1b9f5
14 changes: 7 additions & 7 deletions ext/node/polyfills/internal/fs/handle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,27 @@ export class FileHandle extends EventEmitter {
#rid: number;
constructor(rid: number) {
super();
this.rid = rid;
this.#rid = rid;
}

get fd() {
return this.rid;
return this.#rid;
}

read(
buffer: Buffer,
buffer: Uint8Array,
offset?: number,
length?: number,
position?: number | null,
): Promise<ReadResult>;
read(options?: ReadOptions): Promise<ReadResult>;
read(
bufferOrOpt: Buffer | ReadOptions,
bufferOrOpt: Uint8Array | ReadOptions,
offset?: number,
length?: number,
position?: number | null,
): Promise<ReadResult> {
if (bufferOrOpt instanceof Buffer) {
if (bufferOrOpt instanceof Uint8Array) {
return new Promise((resolve, reject) => {
read(
this.fd,
Expand Down Expand Up @@ -90,12 +90,12 @@ export class FileHandle extends EventEmitter {
encoding: string,
): Promise<WriteResult>;
write(
bufferOrStr: Buffer | string,
bufferOrStr: Uint8Array | string,
offsetOrPosition: number,
lengthOrEncoding: number | string,
position?: number,
): Promise<WriteResult> {
if (bufferOrStr instanceof Buffer) {
if (bufferOrStr instanceof Uint8Array) {
const buffer = bufferOrStr;
const offset = offsetOrPosition;
const length = lengthOrEncoding;
Expand Down
24 changes: 24 additions & 0 deletions tests/unit_node/_fs/_fs_write_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,27 @@ Deno.test({
assertEquals(decoder.decode(data), "hello");
},
});

Deno.test({
name: "Data is padded if position > length",
async fn() {
const tempFile: string = Deno.makeTempFileSync();

using file = await Deno.open(tempFile, {
create: true,
write: true,
read: true,
});

const str = "hello world";
const buffer = Buffer.from(str);
const bytesWritten = writeSync(file.rid, buffer, 0, str.length, 4);

const data = Deno.readFileSync(tempFile);
Deno.removeSync(tempFile);

assertEquals(bytesWritten, str.length);
// Check if result is padded
assertEquals(decoder.decode(data), "\x00\x00\x00\x00hello world");
},
});
38 changes: 37 additions & 1 deletion tests/unit_node/fs_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ import {
statSync,
writeFileSync,
} from "node:fs";
import { constants as fsPromiseConstants, cp } from "node:fs/promises";
import {
constants as fsPromiseConstants,
cp,
FileHandle,
open,
writeFile,
} from "node:fs/promises";
import process from "node:process";
import { pathToAbsoluteFileUrl } from "../unit/test_util.ts";

Expand Down Expand Up @@ -165,3 +171,33 @@ Deno.test(
assertEquals(result, undefined);
},
);

// Test for https://github.com/denoland/deno/issues/23707
Deno.test(
"[node/fs/promises read] respect position argument",
async () => {
const file = mkdtempSync(join(tmpdir(), "foo-")) + "/test.bin";
await writeFile(file, "");

const res: number[] = [];
let fd: FileHandle | undefined;
try {
fd = await open(file, "r+");

for (let i = 0; i <= 5; i++) {
const buffer = new Uint8Array([i]);
await fd.write(buffer, 0, 1, i + 10);
}

for (let i = 10; i <= 15; i++) {
const buffer = new Uint8Array(1);
await fd.read(buffer, 0, 1, i);
res.push(Number(buffer.toString()));
}
} finally {
await fd?.close();
}

assertEquals(res, [0, 1, 2, 3, 4, 5]);
},
);