Skip to content

Commit afa66f6

Browse files
committed
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
1 parent fc4b7c9 commit afa66f6

File tree

2 files changed

+46
-9
lines changed

2 files changed

+46
-9
lines changed

lib/std/process/Child.zig

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -901,11 +901,6 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void {
901901
if (dir_buf.items.len > 0) try dir_buf.append(self.allocator, fs.path.sep);
902902
try dir_buf.appendSlice(self.allocator, app_dir);
903903
}
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

910905
windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, flags, &siStartInfo, &piProcInfo) catch |no_path_err| {
911906
const original_err = switch (no_path_err) {
@@ -930,10 +925,6 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void {
930925
while (it.next()) |search_path| {
931926
dir_buf.clearRetainingCapacity();
932927
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

938929
if (windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, flags, &siStartInfo, &piProcInfo)) {
939930
break :run;

test/standalone/windows_spawn/main.zig

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const std = @import("std");
2+
23
const windows = std.os.windows;
34
const utf16Literal = std.unicode.utf8ToUtf16LeStringLiteral;
45

@@ -39,6 +40,9 @@ pub fn main() anyerror!void {
3940
// No PATH, so it should fail to find anything not in the cwd
4041
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+
4246
std.debug.assert(windows.kernel32.SetEnvironmentVariableW(
4347
utf16Literal("PATH"),
4448
tmp_absolute_path_w,
@@ -149,6 +153,48 @@ pub fn main() anyerror!void {
149153
// If we try to exec but provide a cwd that is an absolute path, the PATH
150154
// should still be searched and the goodbye.exe in something should be found.
151155
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");
152198
}
153199

154200
fn testExecError(err: anyerror, allocator: std.mem.Allocator, command: []const u8) !void {

0 commit comments

Comments
 (0)