From afa66f6111c88cda6bf576367f980f2a84c33116 Mon Sep 17 00:00:00 2001 From: lumanetic <396151+h57624paen@users.noreply.github.com> Date: Sat, 26 Jul 2025 01:34:19 -0400 Subject: [PATCH 1/2] std.process.Child: fix double path normalization in spawnWindows besides simply being redundant work, the now removed normalize call would cause spawn to errantly fail (BadPath) when passing a relative path which traversed 'above' the current working directory. This case is already handled by leaving normalization to the windows.wToPrefixedFileW call in windowsCreateProcessPathExt --- lib/std/process/Child.zig | 9 ----- test/standalone/windows_spawn/main.zig | 46 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/lib/std/process/Child.zig b/lib/std/process/Child.zig index 21cc545f12..ce228176f4 100644 --- a/lib/std/process/Child.zig +++ b/lib/std/process/Child.zig @@ -901,11 +901,6 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void { if (dir_buf.items.len > 0) try dir_buf.append(self.allocator, fs.path.sep); try dir_buf.appendSlice(self.allocator, app_dir); } - if (dir_buf.items.len > 0) { - // Need to normalize the path, openDirW can't handle things like double backslashes - const normalized_len = windows.normalizePath(u16, dir_buf.items) catch return error.BadPathName; - dir_buf.shrinkRetainingCapacity(normalized_len); - } windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, flags, &siStartInfo, &piProcInfo) catch |no_path_err| { const original_err = switch (no_path_err) { @@ -930,10 +925,6 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void { while (it.next()) |search_path| { dir_buf.clearRetainingCapacity(); try dir_buf.appendSlice(self.allocator, search_path); - // Need to normalize the path, some PATH values can contain things like double - // backslashes which openDirW can't handle - const normalized_len = windows.normalizePath(u16, dir_buf.items) catch continue; - dir_buf.shrinkRetainingCapacity(normalized_len); if (windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, flags, &siStartInfo, &piProcInfo)) { break :run; diff --git a/test/standalone/windows_spawn/main.zig b/test/standalone/windows_spawn/main.zig index 3b0d0efe75..4cacf14c7c 100644 --- a/test/standalone/windows_spawn/main.zig +++ b/test/standalone/windows_spawn/main.zig @@ -1,4 +1,5 @@ const std = @import("std"); + const windows = std.os.windows; const utf16Literal = std.unicode.utf8ToUtf16LeStringLiteral; @@ -39,6 +40,9 @@ pub fn main() anyerror!void { // No PATH, so it should fail to find anything not in the cwd try testExecError(error.FileNotFound, allocator, "something_missing"); + // make sure we don't get error.BadPath traversing out of cwd with a relative path + try testExecError(error.FileNotFound, allocator, "..\\.\\.\\.\\\\..\\more_missing"); + std.debug.assert(windows.kernel32.SetEnvironmentVariableW( utf16Literal("PATH"), tmp_absolute_path_w, @@ -149,6 +153,48 @@ pub fn main() anyerror!void { // If we try to exec but provide a cwd that is an absolute path, the PATH // should still be searched and the goodbye.exe in something should be found. try testExecWithCwd(allocator, "goodbye", tmp_absolute_path, "hello from exe\n"); + + // introduce some extra path separators into the path which is dealt with inside the spawn call. + const denormed_something_subdir_size = std.mem.replacementSize(u16, something_subdir_abs_path, utf16Literal("\\"), utf16Literal("\\\\\\\\")); + + const denormed_something_subdir_abs_path = try allocator.allocSentinel(u16, denormed_something_subdir_size, 0); + defer allocator.free(denormed_something_subdir_abs_path); + + _ = std.mem.replace(u16, something_subdir_abs_path, utf16Literal("\\"), utf16Literal("\\\\\\\\"), denormed_something_subdir_abs_path); + + const denormed_something_subdir_wtf8 = try std.unicode.wtf16LeToWtf8Alloc(allocator, denormed_something_subdir_abs_path); + defer allocator.free(denormed_something_subdir_wtf8); + + // clear the path to ensure that the match comes from the cwd + std.debug.assert(windows.kernel32.SetEnvironmentVariableW( + utf16Literal("PATH"), + null, + ) == windows.TRUE); + + try testExecWithCwd(allocator, "goodbye", denormed_something_subdir_wtf8, "hello from exe\n"); + + // normalization should also work if the non-normalized path is found in the PATH var. + std.debug.assert(windows.kernel32.SetEnvironmentVariableW( + utf16Literal("PATH"), + denormed_something_subdir_abs_path, + ) == windows.TRUE); + try testExec(allocator, "goodbye", "hello from exe\n"); + + // now make sure we can launch executables "outside" of the cwd + var subdir_cwd = try tmp.dir.openDir(denormed_something_subdir_wtf8, .{}); + defer subdir_cwd.close(); + + try tmp.dir.rename("something/goodbye.exe", "hello.exe"); + try subdir_cwd.setAsCwd(); + + // clear the PATH again + std.debug.assert(windows.kernel32.SetEnvironmentVariableW( + utf16Literal("PATH"), + null, + ) == windows.TRUE); + + // while we're at it make sure non-windows separators work fine + try testExec(allocator, "../hello", "hello from exe\n"); } fn testExecError(err: anyerror, allocator: std.mem.Allocator, command: []const u8) !void { From 0f4106356e298f894fe31704af350da6209019bc Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Fri, 25 Jul 2025 00:24:09 -0700 Subject: [PATCH 2/2] child_process standalone test: Test spawning a path with leading .. Also check that FileNotFound is consistently returned when the path is missing. The new `run_relative` step will test spawning paths like: child_path: ../84385e7e669db0967d7a42765011dbe0/child missing_child_path: ../84385e7e669db0967d7a42765011dbe0/child_intentionally_missing --- test/standalone/child_process/build.zig | 11 +++++++++++ test/standalone/child_process/main.zig | 16 ++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/test/standalone/child_process/build.zig b/test/standalone/child_process/build.zig index 81b40835f3..9e96aaa608 100644 --- a/test/standalone/child_process/build.zig +++ b/test/standalone/child_process/build.zig @@ -31,5 +31,16 @@ pub fn build(b: *std.Build) void { run.addArtifactArg(child); run.expectExitCode(0); + // Use a temporary directory within the cache as the CWD to test + // spawning the child using a path that contains a leading `..` component. + const run_relative = b.addRunArtifact(main); + run_relative.addArtifactArg(child); + const write_tmp_dir = b.addWriteFiles(); + const tmp_cwd = write_tmp_dir.getDirectory(); + run_relative.addDirectoryArg(tmp_cwd); + run_relative.setCwd(tmp_cwd); + run_relative.expectExitCode(0); + test_step.dependOn(&run.step); + test_step.dependOn(&run_relative.step); } diff --git a/test/standalone/child_process/main.zig b/test/standalone/child_process/main.zig index 6537f90acf..9ded383d96 100644 --- a/test/standalone/child_process/main.zig +++ b/test/standalone/child_process/main.zig @@ -11,7 +11,14 @@ pub fn main() !void { var it = try std.process.argsWithAllocator(gpa); defer it.deinit(); _ = it.next() orelse unreachable; // skip binary name - const child_path = it.next() orelse unreachable; + const child_path, const needs_free = child_path: { + const child_path = it.next() orelse unreachable; + const cwd_path = it.next() orelse break :child_path .{ child_path, false }; + // If there is a third argument, it is the current CWD somewhere within the cache directory. + // In that case, modify the child path in order to test spawning a path with a leading `..` component. + break :child_path .{ try std.fs.path.relative(gpa, cwd_path, child_path), true }; + }; + defer if (needs_free) gpa.free(child_path); var child = std.process.Child.init(&.{ child_path, "hello arg" }, gpa); child.stdin_behavior = .Pipe; @@ -39,7 +46,12 @@ pub fn main() !void { }, else => |term| testError("abnormal child exit: {}", .{term}), } - return if (parent_test_error) error.ParentTestError else {}; + if (parent_test_error) return error.ParentTestError; + + // Check that FileNotFound is consistent across platforms when trying to spawn an executable that doesn't exist + const missing_child_path = try std.mem.concat(gpa, u8, &.{ child_path, "_intentionally_missing" }); + defer gpa.free(missing_child_path); + try std.testing.expectError(error.FileNotFound, std.process.Child.run(.{ .allocator = gpa, .argv = &.{missing_child_path} })); } var parent_test_error = false;