diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index 44e8d040fd7b..57ee3e6ba4b3 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -527,7 +527,7 @@ function writeAll(data, size, pos, cb, retries = 0) { retries = bytesWritten ? 0 : retries + 1; size -= bytesWritten; - pos += bytesWritten; + if (pos !== undefined) pos += bytesWritten; // Try writing non-zero number of bytes up to 5 times. if (retries > 5) { @@ -542,7 +542,7 @@ function writeAll(data, size, pos, cb, retries = 0) { } function writevAll(chunks, size, pos, cb, retries = 0) { - this[kFs].writev(this.fd, chunks, this.pos, (er, bytesWritten, buffers) => { + this[kFs].writev(this.fd, chunks, pos, (er, bytesWritten, buffers) => { // No data currently available and operation should be retried later. if (er?.code === "EAGAIN") { er = null; @@ -557,7 +557,7 @@ function writevAll(chunks, size, pos, cb, retries = 0) { retries = bytesWritten ? 0 : retries + 1; size -= bytesWritten; - pos += bytesWritten; + if (pos !== undefined) pos += bytesWritten; // Try writing non-zero number of bytes up to 5 times. if (retries > 5) { diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 32d9c8856a42..e005264052bd 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -3852,19 +3852,13 @@ pub mod args { if !(current.is_number() || current.is_big_int()) { break 'parse; } - let position = i52::from_js(current); - if position >= 0 { - args.position = Some(position); - } + args.position = write_position_from_js(current); arguments.eat(); } // fs.write(fd, string[, position[, encoding]], callback) _ => { if current.is_number() { - let position = i52::from_js(current); - if position >= 0 { - args.position = Some(position); - } + args.position = write_position_from_js(current); } // Node consumes the position slot whatever its type // (null, undefined, a non-number); the encoding is @@ -10223,3 +10217,22 @@ impl i52 { (v.to_int64() << 12) >> 12 } } + +/// Node's `GetOffset` for `fs.write` / `fs.writeSync` position: only a +/// non-negative safe integer selects `pwrite`; NaN, ±Infinity, non-integers +/// and negatives fall back to the current file offset (None). +#[inline] +fn write_position_from_js(v: JSValue) -> Option { + if let Some(num) = v.get_number() { + if num.is_finite() + && num.trunc() == num + && num >= 0.0 + && num <= bun_jsc::MAX_SAFE_INTEGER as f64 + { + return Some(num as i64); + } + return None; + } + let position = i52::from_js(v); + if position >= 0 { Some(position) } else { None } +} diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 16396f0dd211..4cac0ec0f01b 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -2024,6 +2024,64 @@ describe("writeSync", () => { closeSync(fd); }); + // Node's binding maps any non-safe-integer position to -1 (current file + // offset). NaN previously coerced to 0, overwriting the start of the file. + it.each([ + ["NaN", NaN], + ["Infinity", Infinity], + ["-Infinity", -Infinity], + ["-1", -1], + ["1.5", 1.5], + ])("treats position %s as the current file offset", async (_label, position) => { + using dir = tempDir("write-position-current", {}); + const p = join(String(dir), "f"); + writeFileSync(p, Buffer.alloc(10, "A")); + const seed = Buffer.alloc(5); + const expected = "AAAAAXXAAA"; + + // writeSync(fd, buffer, offset, length, position) + { + const fd = openSync(p, "r+"); + try { + readSync(fd, seed, 0, 5, null); + expect(writeSync(fd, Buffer.from("XX"), 0, 2, position as number)).toBe(2); + } finally { + closeSync(fd); + } + expect(readFileSync(p, "utf8")).toBe(expected); + } + + // fs.write(fd, buffer, offset, length, position, callback) + writeFileSync(p, Buffer.alloc(10, "A")); + { + const fd = openSync(p, "r+"); + try { + readSync(fd, seed, 0, 5, null); + const { promise, resolve, reject } = Promise.withResolvers(); + fs.write(fd, Buffer.from("XX"), 0, 2, position as number, (err, written) => + err ? reject(err) : resolve(written), + ); + expect(await promise).toBe(2); + } finally { + closeSync(fd); + } + expect(readFileSync(p, "utf8")).toBe(expected); + } + + // writeSync(fd, string, position) + writeFileSync(p, Buffer.alloc(10, "A")); + { + const fd = openSync(p, "r+"); + try { + readSync(fd, seed, 0, 5, null); + expect(writeSync(fd, "XX", position as number)).toBe(2); + } finally { + closeSync(fd); + } + expect(readFileSync(p, "utf8")).toBe(expected); + } + }); + // writeSync(fd, string[, position[, encoding]]): the encoding used to be // parsed but never applied, so utf16le/hex/base64/latin1 all wrote raw UTF-8. it("honors the encoding argument for strings", () => { @@ -3671,6 +3729,92 @@ describe("createWriteStream", () => { } }); }); + + // With no `start` the stream passes `pos = undefined` to writeAll/writevAll. + // A partial write must retry at the current file offset (pos stays + // undefined), not at `undefined + bytesWritten` (NaN), which the native + // layer used to coerce to pwrite offset 0 and stamp the tail over the head. + it.each([ + ["write", undefined], + ["write", 0], + ["writev", undefined], + ["writev", 0], + ] as const)("retries a partial %s at the correct offset with start %p", async (method, start) => { + using dir = tempDir("write-stream-partial", {}); + const p = join(String(dir), "out"); + const payload = Buffer.from("ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + const positions: unknown[] = []; + let first = true; + + const customFs: any = { + open: fs.open, + close: fs.close, + write(fd, buf, offset, length, position, cb) { + positions.push(position); + if (first) { + first = false; + const half = Math.floor(length / 2); + fs.write(fd, buf, offset, half, position, (err, written) => cb(err, written, buf)); + return; + } + fs.write(fd, buf, offset, length, position, cb); + }, + writev(fd, chunks, position, cb) { + positions.push(position); + if (first) { + first = false; + fs.writev(fd, [chunks[0]], position, (err, written) => cb(err, written, chunks)); + return; + } + fs.writev(fd, chunks, position, cb); + }, + }; + + const stream = createWriteStream(p, { fs: customFs, start } as any); + const { promise, resolve, reject } = Promise.withResolvers(); + stream.on("error", reject); + stream.on("finish", resolve); + if (method === "writev") { + stream.cork(); + stream.write(payload.subarray(0, 10)); + stream.write(payload.subarray(10)); + stream.uncork(); + stream.end(); + } else { + stream.end(payload); + } + await promise; + + expect(readFileSync(p)).toEqual(payload); + expect(positions.some(v => typeof v === "number" && Number.isNaN(v))).toBe(false); + if (start === 0) { + expect(positions).toEqual(method === "writev" ? [0, 10] : [0, 13]); + } else { + expect(positions).toEqual([undefined, undefined]); + } + }); + + // End-to-end: a kernel-enforced short write (RLIMIT_FSIZE) must surface as + // an EFBIG 'error' and leave the file a byte-exact prefix of the source. + it.skipIf(!isLinux)("surfaces EFBIG from a short write instead of overwriting the file head", async () => { + using dir = tempDir("write-stream-fsize", {}); + const out = join(String(dir), "out"); + const fixture = join(import.meta.dir, "write-stream-fsize-fixture.js"); + await using proc = Bun.spawn({ + cmd: ["/bin/sh", "-c", `ulimit -f 1024 && exec "$0" "$1" "$2"`, bunExe(), fixture, out], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const result = JSON.parse(stdout.trim()); + expect({ head: result.head, isPrefix: result.isPrefix }).toEqual({ head: "AAAAAAAA", isPrefix: true }); + expect(result.events).toContain("error:EFBIG"); + expect(result.events).not.toContain("finish"); + expect(result.fileSize).toBeLessThanOrEqual(1 << 20); + expect(exitCode).toBe(0); + }); }); describe("fs/promises", () => { diff --git a/test/js/node/fs/write-stream-fsize-fixture.js b/test/js/node/fs/write-stream-fsize-fixture.js new file mode 100644 index 000000000000..998952f9f84b --- /dev/null +++ b/test/js/node/fs/write-stream-fsize-fixture.js @@ -0,0 +1,35 @@ +// Run under `ulimit -f 1024` (RLIMIT_FSIZE = 1 MiB). Writes a single 4 MiB +// chunk (1 MiB each of A/B/C/D) through createWriteStream and reports whether +// the on-disk bytes are a byte-exact prefix of the source. A short write that +// retries at offset 0 would stamp the tail block over the head. +const fs = require("fs"); +const path = process.argv[2]; + +const MiB = 1 << 20; +const src = Buffer.concat([ + Buffer.alloc(MiB, "A"), + Buffer.alloc(MiB, "B"), + Buffer.alloc(MiB, "C"), + Buffer.alloc(MiB, "D"), +]); + +const events = []; +const stream = fs.createWriteStream(path); +stream.on("error", e => events.push("error:" + (e && e.code))); +stream.on("finish", () => events.push("finish")); +stream.on("close", () => { + const out = fs.existsSync(path) ? fs.readFileSync(path) : Buffer.alloc(0); + const head = out.subarray(0, 8).toString(); + const isPrefix = src.subarray(0, out.length).equals(out); + process.stdout.write( + JSON.stringify({ + events, + bytesWritten: stream.bytesWritten, + fileSize: out.length, + head, + isPrefix, + }) + "\n", + ); +}); +stream.write(src); +stream.end();