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

Author: =

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;

test/standalone/child_process/main.zig

@@ -39,7 +39,9 @@ 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;
+
+    try testSpawnRelative(gpa, child_path);
 }
 
 var parent_test_error = false;
@@ -54,3 +56,29 @@ fn testError(comptime fmt: []const u8, args: anytype) void {
     }
     parent_test_error = true;
 }
+
+fn testSpawnRelative(allocator: std.mem.Allocator, child_path: []const u8) !void {
+    // regression test: ensure we can launch processes "above" the current working directory.
+    const deeper_dir_path = "./1/2/3/4/5";
+    const child_dir_path = std.fs.path.dirname(child_path) orelse return error.BadChildPath;
+    var child_dir = try std.fs.cwd().openDir(child_dir_path, .{});
+    defer child_dir.close();
+
+    try child_dir.setAsCwd();
+    try child_dir.makePath(deeper_dir_path);
+    var deeper_dir = try child_dir.openDir(deeper_dir_path, .{});
+    defer deeper_dir.close();
+    try deeper_dir.setAsCwd();
+    const rel_path = try std.fs.path.relative(allocator, deeper_dir_path, std.fs.path.basename(child_path));
+    defer allocator.free(rel_path);
+
+    const result = try std.process.Child.run(.{ .allocator = allocator, .argv = &.{rel_path} });
+
+    allocator.free(result.stdout);
+    allocator.free(result.stderr);
+
+    const res = std.process.Child.run(.{ .allocator = allocator, .argv = &.{"../../notafile"} });
+
+    // would expect this to be the same error on all platforms. pre-fix windows would return BadPathName instead.
+    try std.testing.expectError(error.FileNotFound, res);
+}

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");
 
+    // Regression test - 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,42 @@ 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);
+
+    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 {