Skip to content

Commit d761a71

Browse files
authored
fix(ext/node): read byob views at offset 0 in FileHandle.readableWebStream (#19)
The byob pull passed the view's byteOffset as the read offset. That offset is relative to the view, so a view that starts after byte 0 of its ArrayBuffer failed with ERR_OUT_OF_RANGE ("It must be <= 0"). Pass 0, as Node does since nodejs/node#58842 (v26.10.0). Add a unit test that reads with a byob reader into 100-byte DataViews at increasing offsets of one ArrayBuffer. It fails before this change.
1 parent fba9b50 commit d761a71

2 files changed

Lines changed: 43 additions & 2 deletions

File tree

‎ext/node/polyfills/internal/fs/handle.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ const {
9595
Symbol,
9696
SymbolAsyncDispose,
9797
TypedArrayPrototypeGetByteLength,
98-
TypedArrayPrototypeGetByteOffset,
9998
Uint8ArrayPrototype,
10099
} = primordials;
101100

@@ -429,9 +428,10 @@ export class FileHandle extends EventEmitter {
429428

430429
pull: async (controller) => {
431430
const view = controller.byobRequest!.view! as Uint8Array;
431+
// The read offset is relative to the view, so it is always 0.
432432
const { bytesRead } = await this.read(
433433
view,
434-
TypedArrayPrototypeGetByteOffset(view),
434+
0,
435435
TypedArrayPrototypeGetByteLength(view),
436436
);
437437

‎tests/unit_node/_fs/_fs_handle_test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,47 @@ Deno.test(
441441
},
442442
);
443443

444+
Deno.test(
445+
"[node/fs filehandle.readableWebStream] byob reads into views at a byteOffset of one ArrayBuffer",
446+
async function () {
447+
// Use this file so that the read needs more than one view.
448+
const source = path.fromFileUrl(import.meta.url);
449+
const expected = await Deno.readFile(source);
450+
assert(expected.byteLength > 300);
451+
452+
const fileHandle = await fs.open(source);
453+
const reader = fileHandle.readableWebStream().getReader({ mode: "byob" });
454+
455+
let buffer = new ArrayBuffer(expected.byteLength);
456+
let offset = 0;
457+
let reads = 0;
458+
let result;
459+
do {
460+
result = await reader.read(
461+
new DataView(
462+
buffer,
463+
offset,
464+
Math.min(100, buffer.byteLength - offset),
465+
),
466+
);
467+
if (result.value !== undefined) {
468+
assertEquals(result.value.byteOffset, offset);
469+
assert(result.value.byteLength <= 100);
470+
buffer = result.value.buffer as ArrayBuffer;
471+
offset += result.value.byteLength;
472+
reads++;
473+
}
474+
} while (!result.done && offset < buffer.byteLength);
475+
476+
assert(reads > 1);
477+
assertEquals(offset, expected.byteLength);
478+
assertEquals(new Uint8Array(buffer), expected);
479+
480+
reader.releaseLock();
481+
await fileHandle.close();
482+
},
483+
);
484+
444485
Deno.test(
445486
"[node/fs filehandle.readableWebStream] With autoClose option",
446487
async function () {

0 commit comments

Comments
 (0)