diff --git a/src/js/node/fs.ts b/src/js/node/fs.ts index 3e1569eb188c..25b0bbb21e34 100644 --- a/src/js/node/fs.ts +++ b/src/js/node/fs.ts @@ -304,6 +304,9 @@ var access = function access(path, mode, callback) { length = buffer.byteLength - offsetOrOptions, position = null, } = offsetOrOptions ?? {}); + } else if (typeof offsetOrOptions === "function") { + // fs.write(fd, buffer, callback) + offsetOrOptions = 0; } fs.write(fd, buffer, offsetOrOptions, length, position).then(wrapper, callback); @@ -517,8 +520,10 @@ var access = function access(path, mode, callback) { try { if (types.isArrayBufferView(buffer)) { let offset = offsetOrOptions; - if (typeof offset === "object" && offset !== null) { - ({ offset = 0, length = buffer.byteLength - offset, position = null } = offsetOrOptions); + // `null` takes the options path, discarding `length` and `position`: + // https://github.com/nodejs/node/blob/v26.3.0/lib/fs.js#L884-L890 + if (typeof offset === "object") { + ({ offset = 0, length = buffer.byteLength - offset, position = null } = offsetOrOptions ?? {}); return fs.writeSync(fd, buffer, offset, length, position); } return arguments.length <= 2 ? fs.writeSync(fd, buffer) : fs.writeSync(fd, buffer, offset, length, position); diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 14060146f073..9a800a091e38 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -3693,47 +3693,42 @@ pub mod args { ..Default::default() }; arguments.eat(); - 'parse: { - let Some(mut current) = arguments.next() else { - break 'parse; - }; - match &args.buffer { - // fs.write(fd, buffer[, offset[, length[, position]]], callback) - StringOrBuffer::Buffer(_) => { - if current.is_undefined_or_null() || current.is_function() { - break 'parse; - } + match &args.buffer { + // fs.write(fd, buffer[, offset[, length[, position]]], callback) + // + // The slots are positional: one holding `undefined`, `null` or a + // non-number takes its default and the following slots are still + // read, as in Node's `fs.writeSync`: + // https://github.com/nodejs/node/blob/v26.3.0/lib/fs.js#L893-L900 + StringOrBuffer::Buffer(buffer) => { + let buf_len = buffer.slice().len() as u64; + let offset = arguments.next_eat().unwrap_or(JSValue::UNDEFINED); + if !offset.is_undefined_or_null() { args.offset = u64::try_from(validators::validate_integer( ctx, - current, + offset, "offset", Some(0), - Some(9007199254740991), + Some(bun_jsc::MAX_SAFE_INTEGER), )?) .expect("infallible: validated range"); - arguments.eat(); - let Some(next) = arguments.next() else { - break 'parse; - }; - current = next; - if !(current.is_number() || current.is_big_int()) { - break 'parse; - } - let length = current.to_int64(); - let buf_len = args.buffer.buffer().map(|b| b.slice().len()).unwrap_or(0); - let max_offset = buf_len as i64; - if args.offset as i64 > max_offset { - return Err(ctx.throw_range_error( - args.offset as f64, - bun_jsc::RangeErrorOptions { - field_name: b"offset", - max: max_offset, - ..Default::default() - }, - )); - } - let max_len = ((buf_len as u64 - args.offset) as i64).min(i32::MAX as i64); - if length > max_len || length < 0 { + } + if args.offset > buf_len { + return Err(ctx.throw_range_error( + args.offset as f64, + bun_jsc::RangeErrorOptions { + field_name: b"offset", + max: buf_len as i64, + ..Default::default() + }, + )); + } + // `args.length` defaults to the rest of the buffer. + let length = arguments.next_eat().unwrap_or(JSValue::UNDEFINED); + if length.is_number() { + let length = length.to_int64(); + let max_len = (buf_len - args.offset).min(i32::MAX as u64) as i64; + if length < 0 || length > max_len { return Err(ctx.throw_range_error( length as f64, bun_jsc::RangeErrorOptions { @@ -3744,45 +3739,34 @@ pub mod args { }, )); } - args.length = u64::try_from(length).expect("int cast"); - arguments.eat(); - let Some(next) = arguments.next() else { - break 'parse; - }; - current = next; - if !(current.is_number() || current.is_big_int()) { - break 'parse; - } - if let Some(position @ 0..) = i52::offset_from_js(current) { - args.position = Some(position); - } - arguments.eat(); + args.length = length as u64; } - // fs.write(fd, string[, position[, encoding]], callback) - _ => { - if let Some(position @ 0..) = i52::offset_from_js(current) { - args.position = Some(position); - } - // Node consumes the position slot whatever its type - // (null, undefined, a non-number); the encoding is - // strictly the next argument. - arguments.eat(); - let Some(next) = arguments.next() else { - break 'parse; - }; - current = next; - if current.is_string() { - args.encoding = Encoding::assert(current, ctx, args.encoding)?; - arguments.eat(); - // `bv` was converted to UTF-8 before the encoding - // argument was parsed; re-encode it now. Node - // treats the "buffer" encoding name as UTF-8 here. - if !matches!(args.encoding, Encoding::Utf8 | Encoding::Buffer) { - if let Some(encoded) = - StringOrBuffer::from_js_with_encoding(ctx, bv, args.encoding)? - { - args.buffer = encoded; - } + if let Some(position @ 0..) = arguments.next_eat().and_then(i52::offset_from_js) + { + args.position = Some(position); + } + } + // fs.write(fd, string[, position[, encoding]], callback) + // + // Node consumes the position slot whatever its type (null, + // undefined, a non-number); the encoding is strictly the next + // argument. + _ => { + if let Some(position @ 0..) = arguments.next_eat().and_then(i52::offset_from_js) + { + args.position = Some(position); + } + let encoding = arguments.next_eat().unwrap_or(JSValue::UNDEFINED); + if encoding.is_string() { + args.encoding = Encoding::assert(encoding, ctx, args.encoding)?; + // `bv` was converted to UTF-8 before the encoding + // argument was parsed; re-encode it now. Node + // treats the "buffer" encoding name as UTF-8 here. + if !matches!(args.encoding, Encoding::Utf8 | Encoding::Buffer) { + if let Some(encoded) = + StringOrBuffer::from_js_with_encoding(ctx, bv, args.encoding)? + { + args.buffer = encoded; } } } diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 8fbe948017e2..a7607797f59c 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -2065,7 +2065,103 @@ describe.concurrent("writev/readv with more than IOV_MAX buffers", () => { }); }); +// write(fd, buffer[, offset[, length[, position]]]) in all three forms (writeSync, +// callback write, filehandle.write). Each case runs against a file that already +// holds `positionalWriteHead` with the file offset at its end, so a write at +// position 0 and a write at the current position are distinguishable. +// Expectations are Node v26's. +const positionalWriteHead = "PPPPPPPPPPPP"; +const positionalWriteBuffer = Buffer.from("0123456789abcdef"); +const positionalWriteCases: [args: unknown[], written: number, file: string][] = [ + // A nullish offset is 0; the length and position after it still apply. + [[undefined, 8, 0], 8, "01234567PPPP"], + [[undefined, undefined, 0], 16, "0123456789abcdef"], + [[undefined, 8], 8, positionalWriteHead + "01234567"], + // A non-number length is the rest of the buffer; the position still applies. + [[undefined, "8", 0], 16, "0123456789abcdef"], + [[undefined, 8n, 0], 16, "0123456789abcdef"], + [[4, undefined, 0], 12, "456789abcdef"], + [[4, null, 0], 12, "456789abcdef"], + // A non-numeric position writes at the current file offset. + [[4, 8, undefined], 8, positionalWriteHead + "456789ab"], + [[4, 8, null], 8, positionalWriteHead + "456789ab"], + [[4, 8, "2"], 8, positionalWriteHead + "456789ab"], + [[4, 8, 2], 8, "PP456789abPP"], + // An offset equal to byteLength is allowed and writes nothing. + [[16], 0, positionalWriteHead], + // `null` and objects take the options form, which discards the trailing + // positional arguments. + [[null, 8, 0], 16, positionalWriteHead + "0123456789abcdef"], + [[{ offset: 4, length: 8, position: 0 }], 8, "456789abPPPP"], +]; +// The offset and length are validated against the buffer even when the +// arguments after them are omitted. +const positionalWriteErrors: [args: unknown[], error: unknown][] = [ + [[17], outOfRange("offset", "<= 16", 17)], + // Node words the length bound as "<= 16" or ">= 0" depending on which side + // was violated; Bun has always reported both bounds. + [[undefined, 17], outOfRange("length", ">= 0 and <= 16", 17)], + [[undefined, -1], outOfRange("length", ">= 0 and <= 16", -1)], + [[4, 13], outOfRange("length", ">= 0 and <= 12", 13)], +]; +function outOfRange(argument: string, bound: string, received: number) { + return { + name: "RangeError", + code: "ERR_OUT_OF_RANGE", + message: `The value of "${argument}" is out of range. It must be ${bound}. Received ${received}`, + }; +} +/** The return value of `fn`, or the shape of what it threw. */ +function outcomeOf(fn: () => unknown): unknown { + try { + return fn(); + } catch (error: any) { + return { name: error.name, code: error.code, message: error.message }; + } +} + describe("writeSync", () => { + it("reads offset, length and position positionally", () => { + using dir = tempDir("fs-writeSync-positional", {}); + const dest = join(String(dir), "out.bin"); + for (const [args, written, file] of positionalWriteCases) { + const fd = openSync(dest, "w+"); + let outcome: unknown; + try { + writeSync(fd, positionalWriteHead); + outcome = outcomeOf(() => (writeSync as Function)(fd, positionalWriteBuffer, ...args)); + } finally { + closeSync(fd); + } + expect({ args, outcome, file: readFileSync(dest, "latin1") }).toEqual({ args, outcome: written, file }); + } + }); + + it("validates offset and length against the buffer even when the later arguments are omitted", () => { + using dir = tempDir("fs-writeSync-positional-errors", {}); + const dest = join(String(dir), "out.bin"); + const errors: [args: unknown[], error: unknown][] = [ + ...positionalWriteErrors, + // Unlike fs.write, writeSync takes no callback, so a function is not an offset. + [[() => {}, 8, 0], expect.objectContaining({ name: "TypeError", code: "ERR_INVALID_ARG_TYPE" })], + ]; + for (const [args, error] of errors) { + const fd = openSync(dest, "w+"); + let outcome: unknown; + try { + writeSync(fd, positionalWriteHead); + outcome = outcomeOf(() => (writeSync as Function)(fd, positionalWriteBuffer, ...args)); + } finally { + closeSync(fd); + } + expect({ args, outcome, file: readFileSync(dest, "latin1") }).toEqual({ + args, + outcome: error, + file: positionalWriteHead, + }); + } + }); + it("treats a bigint position as the current offset", () => { // Node's fs.write does `if (typeof position !== 'number') position = null` // for the buffer overload; GetOffset then returns -1. fs.read accepts @@ -4804,6 +4900,66 @@ describe("fs.write", () => { }); }); + it("reads offset, length and position positionally", async () => { + using dir = tempDir("fs-write-positional", {}); + const dest = join(String(dir), "out.bin"); + const cases: typeof positionalWriteCases = [ + ...positionalWriteCases, + // fs.write(fd, buffer, callback): the callback sits in the offset slot. + [[], 16, positionalWriteHead + "0123456789abcdef"], + ]; + for (const [args, written, file] of cases) { + const fd = openSync(dest, "w+"); + let outcome: unknown; + try { + writeSync(fd, positionalWriteHead); + const { promise, resolve, reject } = Promise.withResolvers(); + (fs.write as Function)(fd, positionalWriteBuffer, ...args, (err: unknown, bytesWritten: number) => + err ? reject(err) : resolve(bytesWritten), + ); + outcome = await promise; + } finally { + closeSync(fd); + } + expect({ args, outcome, file: readFileSync(dest, "latin1") }).toEqual({ args, outcome: written, file }); + } + + for (const [args, written, file] of positionalWriteCases) { + const handle = await promises.open(dest, "w+"); + let outcome: unknown; + try { + await handle.write(positionalWriteHead); + outcome = (await (handle.write as Function)(positionalWriteBuffer, ...args)).bytesWritten; + } finally { + await handle.close(); + } + expect({ args, outcome, file: readFileSync(dest, "latin1") }).toEqual({ args, outcome: written, file }); + } + }); + + it("throws synchronously for an offset or length outside the buffer", () => { + using dir = tempDir("fs-write-positional-errors", {}); + const dest = join(String(dir), "out.bin"); + function mustNotCall() { + throw new Error("fs.write must throw instead of calling back"); + } + for (const [args, error] of positionalWriteErrors) { + const fd = openSync(dest, "w+"); + let outcome: unknown; + try { + writeSync(fd, positionalWriteHead); + outcome = outcomeOf(() => (fs.write as Function)(fd, positionalWriteBuffer, ...args, mustNotCall)); + } finally { + closeSync(fd); + } + expect({ args, outcome, file: readFileSync(dest, "latin1") }).toEqual({ + args, + outcome: error, + file: positionalWriteHead, + }); + } + }); + it("should work with (fd, string, position, encoding, callback)", done => { const path = `${tmpdir()}/bun-fs-write-3-${Date.now()}.txt`; const fd = fs.openSync(path, "w");