authorgravatar for alex@alexrp.comAlex Rønne Petersen <alex@alexrp.com> 2025-08-03 11:19:10+02:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2025-08-03 11:19:10+02:00
loga5f891d0b30672e51f9f72c716953ed172603031
tree3ccd1cd7c2baee27301c1c164236737e48632e48
parentfa445d86a110f1171b75824fe5ec139089fa4733
parent0f4106356e298f894fe31704af350da6209019bc
signaturebadge-check Signed by PGP key B5690EEEBB952194

Merge pull request #24562 from h57624paen/fix-win-spawn-double-normalize

std.process.Child: fix double path normalization in spawnWindows

4 files changed, 71 insertions(+), 11 deletions(-)

lib/std/process/Child.zig-9
...@@ -901,11 +901,6 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void {...@@ -901,11 +901,6 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void {
901 if (dir_buf.items.len > 0) try dir_buf.append(self.allocator, fs.path.sep);901 if (dir_buf.items.len > 0) try dir_buf.append(self.allocator, fs.path.sep);
902 try dir_buf.appendSlice(self.allocator, app_dir);902 try dir_buf.appendSlice(self.allocator, app_dir);
903 }903 }
904 if (dir_buf.items.len > 0) {
905 // Need to normalize the path, openDirW can't handle things like double backslashes
906 const normalized_len = windows.normalizePath(u16, dir_buf.items) catch return error.BadPathName;
907 dir_buf.shrinkRetainingCapacity(normalized_len);
908 }
909904
910 windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, flags, &siStartInfo, &piProcInfo) catch |no_path_err| {905 windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, flags, &siStartInfo, &piProcInfo) catch |no_path_err| {
911 const original_err = switch (no_path_err) {906 const original_err = switch (no_path_err) {
...@@ -930,10 +925,6 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void {...@@ -930,10 +925,6 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void {
930 while (it.next()) |search_path| {925 while (it.next()) |search_path| {
931 dir_buf.clearRetainingCapacity();926 dir_buf.clearRetainingCapacity();
932 try dir_buf.appendSlice(self.allocator, search_path);927 try dir_buf.appendSlice(self.allocator, search_path);
933 // Need to normalize the path, some PATH values can contain things like double
934 // backslashes which openDirW can't handle
935 const normalized_len = windows.normalizePath(u16, dir_buf.items) catch continue;
936 dir_buf.shrinkRetainingCapacity(normalized_len);
937928
938 if (windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, flags, &siStartInfo, &piProcInfo)) {929 if (windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, flags, &siStartInfo, &piProcInfo)) {
939 break :run;930 break :run;
test/standalone/child_process/build.zig+11
...@@ -31,5 +31,16 @@ pub fn build(b: *std.Build) void {...@@ -31,5 +31,16 @@ pub fn build(b: *std.Build) void {
31 run.addArtifactArg(child);31 run.addArtifactArg(child);
32 run.expectExitCode(0);32 run.expectExitCode(0);
3333
34 // Use a temporary directory within the cache as the CWD to test
35 // spawning the child using a path that contains a leading `..` component.
36 const run_relative = b.addRunArtifact(main);
37 run_relative.addArtifactArg(child);
38 const write_tmp_dir = b.addWriteFiles();
39 const tmp_cwd = write_tmp_dir.getDirectory();
40 run_relative.addDirectoryArg(tmp_cwd);
41 run_relative.setCwd(tmp_cwd);
42 run_relative.expectExitCode(0);
43
34 test_step.dependOn(&run.step);44 test_step.dependOn(&run.step);
45 test_step.dependOn(&run_relative.step);
35}46}
test/standalone/child_process/main.zig+14-2
...@@ -11,7 +11,14 @@ pub fn main() !void {...@@ -11,7 +11,14 @@ pub fn main() !void {
11 var it = try std.process.argsWithAllocator(gpa);11 var it = try std.process.argsWithAllocator(gpa);
12 defer it.deinit();12 defer it.deinit();
13 _ = it.next() orelse unreachable; // skip binary name13 _ = it.next() orelse unreachable; // skip binary name
14 const child_path = it.next() orelse unreachable;14 const child_path, const needs_free = child_path: {
15 const child_path = it.next() orelse unreachable;
16 const cwd_path = it.next() orelse break :child_path .{ child_path, false };
17 // If there is a third argument, it is the current CWD somewhere within the cache directory.
18 // In that case, modify the child path in order to test spawning a path with a leading `..` component.
19 break :child_path .{ try std.fs.path.relative(gpa, cwd_path, child_path), true };
20 };
21 defer if (needs_free) gpa.free(child_path);
1522
16 var child = std.process.Child.init(&.{ child_path, "hello arg" }, gpa);23 var child = std.process.Child.init(&.{ child_path, "hello arg" }, gpa);
17 child.stdin_behavior = .Pipe;24 child.stdin_behavior = .Pipe;
...@@ -39,7 +46,12 @@ pub fn main() !void {...@@ -39,7 +46,12 @@ pub fn main() !void {
39 },46 },
40 else => |term| testError("abnormal child exit: {}", .{term}),47 else => |term| testError("abnormal child exit: {}", .{term}),
41 }48 }
42 return if (parent_test_error) error.ParentTestError else {};49 if (parent_test_error) return error.ParentTestError;
50
51 // Check that FileNotFound is consistent across platforms when trying to spawn an executable that doesn't exist
52 const missing_child_path = try std.mem.concat(gpa, u8, &.{ child_path, "_intentionally_missing" });
53 defer gpa.free(missing_child_path);
54 try std.testing.expectError(error.FileNotFound, std.process.Child.run(.{ .allocator = gpa, .argv = &.{missing_child_path} }));
43}55}
4456
45var parent_test_error = false;57var parent_test_error = false;
test/standalone/windows_spawn/main.zig+46
...@@ -1,4 +1,5 @@...@@ -1,4 +1,5 @@
1const std = @import("std");1const std = @import("std");
2
2const windows = std.os.windows;3const windows = std.os.windows;
3const utf16Literal = std.unicode.utf8ToUtf16LeStringLiteral;4const utf16Literal = std.unicode.utf8ToUtf16LeStringLiteral;
45
...@@ -39,6 +40,9 @@ pub fn main() anyerror!void {...@@ -39,6 +40,9 @@ pub fn main() anyerror!void {
39 // No PATH, so it should fail to find anything not in the cwd40 // No PATH, so it should fail to find anything not in the cwd
40 try testExecError(error.FileNotFound, allocator, "something_missing");41 try testExecError(error.FileNotFound, allocator, "something_missing");
4142
43 // make sure we don't get error.BadPath traversing out of cwd with a relative path
44 try testExecError(error.FileNotFound, allocator, "..\\.\\.\\.\\\\..\\more_missing");
45
42 std.debug.assert(windows.kernel32.SetEnvironmentVariableW(46 std.debug.assert(windows.kernel32.SetEnvironmentVariableW(
43 utf16Literal("PATH"),47 utf16Literal("PATH"),
44 tmp_absolute_path_w,48 tmp_absolute_path_w,
...@@ -149,6 +153,48 @@ pub fn main() anyerror!void {...@@ -149,6 +153,48 @@ pub fn main() anyerror!void {
149 // If we try to exec but provide a cwd that is an absolute path, the PATH153 // If we try to exec but provide a cwd that is an absolute path, the PATH
150 // should still be searched and the goodbye.exe in something should be found.154 // should still be searched and the goodbye.exe in something should be found.
151 try testExecWithCwd(allocator, "goodbye", tmp_absolute_path, "hello from exe\n");155 try testExecWithCwd(allocator, "goodbye", tmp_absolute_path, "hello from exe\n");
156
157 // introduce some extra path separators into the path which is dealt with inside the spawn call.
158 const denormed_something_subdir_size = std.mem.replacementSize(u16, something_subdir_abs_path, utf16Literal("\\"), utf16Literal("\\\\\\\\"));
159
160 const denormed_something_subdir_abs_path = try allocator.allocSentinel(u16, denormed_something_subdir_size, 0);
161 defer allocator.free(denormed_something_subdir_abs_path);
162
163 _ = std.mem.replace(u16, something_subdir_abs_path, utf16Literal("\\"), utf16Literal("\\\\\\\\"), denormed_something_subdir_abs_path);
164
165 const denormed_something_subdir_wtf8 = try std.unicode.wtf16LeToWtf8Alloc(allocator, denormed_something_subdir_abs_path);
166 defer allocator.free(denormed_something_subdir_wtf8);
167
168 // clear the path to ensure that the match comes from the cwd
169 std.debug.assert(windows.kernel32.SetEnvironmentVariableW(
170 utf16Literal("PATH"),
171 null,
172 ) == windows.TRUE);
173
174 try testExecWithCwd(allocator, "goodbye", denormed_something_subdir_wtf8, "hello from exe\n");
175
176 // normalization should also work if the non-normalized path is found in the PATH var.
177 std.debug.assert(windows.kernel32.SetEnvironmentVariableW(
178 utf16Literal("PATH"),
179 denormed_something_subdir_abs_path,
180 ) == windows.TRUE);
181 try testExec(allocator, "goodbye", "hello from exe\n");
182
183 // now make sure we can launch executables "outside" of the cwd
184 var subdir_cwd = try tmp.dir.openDir(denormed_something_subdir_wtf8, .{});
185 defer subdir_cwd.close();
186
187 try tmp.dir.rename("something/goodbye.exe", "hello.exe");
188 try subdir_cwd.setAsCwd();
189
190 // clear the PATH again
191 std.debug.assert(windows.kernel32.SetEnvironmentVariableW(
192 utf16Literal("PATH"),
193 null,
194 ) == windows.TRUE);
195
196 // while we're at it make sure non-windows separators work fine
197 try testExec(allocator, "../hello", "hello from exe\n");
152}198}
153199
154fn testExecError(err: anyerror, allocator: std.mem.Allocator, command: []const u8) !void {200fn testExecError(err: anyerror, allocator: std.mem.Allocator, command: []const u8) !void {