From f1151a84adae8b9fa4dac1f2631a1eeb680ed059 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 10 Jul 2024 19:49:36 -0700 Subject: [PATCH] On Windows, fix fs.writeFile(1, fs.writeFile(2, fs.writeFile(\\nul (#12410) Co-authored-by: Dylan Conway --- src/bun.js/node/node_fs.zig | 29 +++++++---------------- src/string_immutable.zig | 12 +++++++++- src/sys.zig | 11 +++++++++ test/js/node/fs/fs-writeFile-1-fixture.js | 15 ++++++++++++ test/js/node/fs/fs.test.ts | 12 ++++++++++ 5 files changed, 58 insertions(+), 21 deletions(-) create mode 100644 test/js/node/fs/fs-writeFile-1-fixture.js diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig index d9120b8d61..2c3929314c 100644 --- a/src/bun.js/node/node_fs.zig +++ b/src/bun.js/node/node_fs.zig @@ -5758,26 +5758,15 @@ pub const NodeFS = struct { } } - if (Environment.isWindows) { - if (args.flag == .a) { - return Maybe(Return.WriteFile).success; - } - - const rc = std.os.windows.kernel32.SetEndOfFile(fd.cast()); - if (rc == 0) { - return .{ - .err = Syscall.Error{ - .errno = @intFromEnum(std.os.windows.kernel32.GetLastError()), - .syscall = .SetEndOfFile, - .fd = fd, - }, - }; - } - } else { - // https://github.com/oven-sh/bun/issues/2931 - // https://github.com/oven-sh/bun/issues/10222 - // only truncate if we're not appending and writing to a path - if ((@intFromEnum(args.flag) & bun.O.APPEND) == 0 and args.file != .fd) { + // https://github.com/oven-sh/bun/issues/2931 + // https://github.com/oven-sh/bun/issues/10222 + // Only truncate if we're not appending and writing to a path + if ((@intFromEnum(args.flag) & bun.O.APPEND) == 0 and args.file != .fd) { + // If this errors, we silently ignore it. + // Not all files are seekable (and thus, not all files can be truncated). + if (Environment.isWindows) { + _ = std.os.windows.kernel32.SetEndOfFile(fd.cast()); + } else { _ = ftruncateSync(.{ .fd = fd, .len = @as(JSC.WebCore.Blob.SizeType, @truncate(written)) }); } } diff --git a/src/string_immutable.zig b/src/string_immutable.zig index 25f76f7c51..db2e54696d 100644 --- a/src/string_immutable.zig +++ b/src/string_immutable.zig @@ -744,6 +744,14 @@ pub fn eql(self: string, other: anytype) bool { return true; } +pub fn eqlComptimeT(comptime T: type, self: []const T, comptime alt: anytype) bool { + if (T == u16) { + return eqlComptimeUTF16(self, alt); + } + + return eqlComptime(self, alt); +} + pub fn eqlComptime(self: string, comptime alt: anytype) bool { return eqlComptimeCheckLenWithType(u8, self, alt, true); } @@ -1732,7 +1740,9 @@ pub fn toWDirPath(wbuf: []u16, utf8: []const u8) [:0]const u16 { pub fn assertIsValidWindowsPath(comptime T: type, path: []const T) void { if (Environment.allow_assert and Environment.isWindows) { if (bun.path.Platform.windows.isAbsoluteT(T, path) and - isWindowsAbsolutePathMissingDriveLetter(T, path)) + isWindowsAbsolutePathMissingDriveLetter(T, path) and + // is it a null device path? that's not an error. it's just a weird file path. + !eqlComptimeT(T, path, "\\\\.\\NUL") and !eqlComptimeT(T, path, "\\\\.\\nul") and !eqlComptimeT(T, path, "\\nul") and !eqlComptimeT(T, path, "\\NUL")) { std.debug.panic("Internal Error: Do not pass posix paths to Windows APIs, was given '{s}'" ++ if (Environment.isDebug) " (missing a root like 'C:\\', see PosixToWinNormalizer for why this is an assertion)" else ". Please open an issue on GitHub with a reproduction.", .{ if (T == u8) path else bun.fmt.utf16(path), diff --git a/src/sys.zig b/src/sys.zig index d135e8b33a..1e74deb903 100644 --- a/src/sys.zig +++ b/src/sys.zig @@ -776,6 +776,17 @@ pub fn normalizePathWindows( var path = if (T == u16) path_ else bun.strings.convertUTF8toUTF16InBuffer(&wbuf, path_); if (std.fs.path.isAbsoluteWindowsWTF16(path)) { + // handle the special "nul" device + // we technically should handle the other DOS devices too. + if (path_.len >= "\\nul".len and + (bun.strings.eqlComptimeT(T, path_[path_.len - "\\nul".len ..], "\\nul") or + bun.strings.eqlComptimeT(T, path_[path_.len - "\\NUL".len ..], "\\NUL"))) + { + @memcpy(buf[0..bun.strings.w("\\??\\NUL").len], bun.strings.w("\\??\\NUL")); + buf[bun.strings.w("\\??\\NUL").len] = 0; + return .{ .result = buf[0..bun.strings.w("\\??\\NUL").len :0] }; + } + const norm = bun.path.normalizeStringGenericTZ(u16, path, buf, .{ .add_nt_prefix = true, .zero_terminate = true }); return .{ .result = norm }; } diff --git a/test/js/node/fs/fs-writeFile-1-fixture.js b/test/js/node/fs/fs-writeFile-1-fixture.js new file mode 100644 index 0000000000..08078b2488 --- /dev/null +++ b/test/js/node/fs/fs-writeFile-1-fixture.js @@ -0,0 +1,15 @@ +import { writeFileSync, writeFile } from "node:fs"; + +process.exitCode = 1; +let input = process.argv[2]; + +if (input === "1") { + input = 1; +} else if (input === "2") { + input = 2; +} + +writeFileSync(input, "Hello World!\n"); +writeFile(input, "Hello World!\n", () => { + process.exitCode = 0; +}); diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 52c6944ca3..bbdb7ec2c8 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -62,6 +62,18 @@ function mkdirForce(path: string) { if (!existsSync(path)) mkdirSync(path, { recursive: true }); } +it("fs.writeFile(1, data) should work when its inherited", async () => { + expect([join(import.meta.dir, "fs-writeFile-1-fixture.js"), "1"]).toRun(); +}); + +it("fs.writeFile(2, data) should work when its inherited", async () => { + expect([join(import.meta.dir, "fs-writeFile-1-fixture.js"), "2"]).toRun(); +}); + +it("fs.writeFile(/dev/null, data) should work", async () => { + expect([join(import.meta.dir, "fs-writeFile-1-fixture.js"), require("os").devNull]).toRun(); +}); + it("fs.openAsBlob", async () => { expect((await openAsBlob(import.meta.path)).size).toBe(statSync(import.meta.path).size); });