From 423761bb6de68bec9482dbc3b3c21b2b5c74a2b9 Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Mon, 17 Mar 2025 17:53:12 -0700 Subject: [PATCH 1/5] createWindowsEnvBlock: Reduce NUL terminator count to only what's required This code previously added 4 NUL code units, but that was likely due to a misinterpretation of this part of the CreateProcess documentation: > A Unicode environment block is terminated by four zero bytes: two for the last string, two more to terminate the block. (four zero *bytes* means *two* zero code units) Additionally, the second zero code unit is only actually needed when the environment is empty due to a quirk of the CreateProcess implementation. In the case of a non-empty environment, there always ends up being two trailing NUL code units since one will come after the last environment variable in the block. --- lib/std/process.zig | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/std/process.zig b/lib/std/process.zig index b734276444b5..4efb58c3dff5 100644 --- a/lib/std/process.zig +++ b/lib/std/process.zig @@ -2036,7 +2036,8 @@ test createNullDelimitedEnvMap { pub fn createWindowsEnvBlock(allocator: mem.Allocator, env_map: *const EnvMap) ![]u16 { // count bytes needed const max_chars_needed = x: { - var max_chars_needed: usize = 4; // 4 for the final 4 null bytes + // Only need 2 trailing NUL code units for an empty environment + var max_chars_needed: usize = if (env_map.count() == 0) 2 else 1; var it = env_map.iterator(); while (it.next()) |pair| { // +1 for '=' @@ -2060,12 +2061,14 @@ pub fn createWindowsEnvBlock(allocator: mem.Allocator, env_map: *const EnvMap) ! } result[i] = 0; i += 1; - result[i] = 0; - i += 1; - result[i] = 0; - i += 1; - result[i] = 0; - i += 1; + // An empty environment is a special case that requires a redundant + // NUL terminator. CreateProcess will read the second code unit even + // though theoretically the first should be enough to recognize that the + // environment is empty (see https://nullprogram.com/blog/2023/08/23/) + if (env_map.count() == 0) { + result[i] = 0; + i += 1; + } return try allocator.realloc(result, i); } From b2cc408a3e5ae0f8c2550c8828f6d388c6969b5a Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Mon, 17 Mar 2025 17:24:00 -0700 Subject: [PATCH 2/5] std.process: Allow WTF-8 in env var functions with comptime-known keys --- lib/std/process.zig | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/std/process.zig b/lib/std/process.zig index 4efb58c3dff5..26b25b25fe48 100644 --- a/lib/std/process.zig +++ b/lib/std/process.zig @@ -419,10 +419,10 @@ pub fn getEnvVarOwned(allocator: Allocator, key: []const u8) GetEnvVarOwnedError } } -/// On Windows, `key` must be valid UTF-8. +/// On Windows, `key` must be valid WTF-8. pub fn hasEnvVarConstant(comptime key: []const u8) bool { if (native_os == .windows) { - const key_w = comptime unicode.utf8ToUtf16LeStringLiteral(key); + const key_w = comptime unicode.wtf8ToWtf16LeStringLiteral(key); return getenvW(key_w) != null; } else if (native_os == .wasi and !builtin.link_libc) { @compileError("hasEnvVarConstant is not supported for WASI without libc"); @@ -431,10 +431,10 @@ pub fn hasEnvVarConstant(comptime key: []const u8) bool { } } -/// On Windows, `key` must be valid UTF-8. +/// On Windows, `key` must be valid WTF-8. pub fn hasNonEmptyEnvVarConstant(comptime key: []const u8) bool { if (native_os == .windows) { - const key_w = comptime unicode.utf8ToUtf16LeStringLiteral(key); + const key_w = comptime unicode.wtf8ToWtf16LeStringLiteral(key); const value = getenvW(key_w) orelse return false; return value.len != 0; } else if (native_os == .wasi and !builtin.link_libc) { @@ -451,10 +451,10 @@ pub const ParseEnvVarIntError = std.fmt.ParseIntError || error{EnvironmentVariab /// /// Since the key is comptime-known, no allocation is needed. /// -/// On Windows, `key` must be valid UTF-8. +/// On Windows, `key` must be valid WTF-8. pub fn parseEnvVarInt(comptime key: []const u8, comptime I: type, base: u8) ParseEnvVarIntError!I { if (native_os == .windows) { - const key_w = comptime std.unicode.utf8ToUtf16LeStringLiteral(key); + const key_w = comptime std.unicode.wtf8ToWtf16LeStringLiteral(key); const text = getenvW(key_w) orelse return error.EnvironmentVariableNotFound; return std.fmt.parseIntWithGenericCharacter(I, u16, text, base); } else if (native_os == .wasi and !builtin.link_libc) { From 752e7c0fd06d96450898913f6207bbe245ca12fc Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Sun, 16 Mar 2025 17:27:41 -0700 Subject: [PATCH 3/5] Add standalone test for environment variables Tests all environment variable APIs in std.process --- test/standalone/build.zig.zon | 3 + test/standalone/env_vars/build.zig | 33 ++++++ test/standalone/env_vars/main.zig | 173 +++++++++++++++++++++++++++++ 3 files changed, 209 insertions(+) create mode 100644 test/standalone/env_vars/build.zig create mode 100644 test/standalone/env_vars/main.zig diff --git a/test/standalone/build.zig.zon b/test/standalone/build.zig.zon index db1c7125a774..58aa0813be8d 100644 --- a/test/standalone/build.zig.zon +++ b/test/standalone/build.zig.zon @@ -96,6 +96,9 @@ .empty_env = .{ .path = "empty_env", }, + .env_vars = .{ + .path = "env_vars", + }, .issue_11595 = .{ .path = "issue_11595", }, diff --git a/test/standalone/env_vars/build.zig b/test/standalone/env_vars/build.zig new file mode 100644 index 000000000000..1dff97d214b1 --- /dev/null +++ b/test/standalone/env_vars/build.zig @@ -0,0 +1,33 @@ +const std = @import("std"); +const builtin = @import("builtin"); + +pub fn build(b: *std.Build) void { + const test_step = b.step("test", "Test it"); + b.default_step = test_step; + + const optimize: std.builtin.OptimizeMode = .Debug; + + const main = b.addExecutable(.{ + .name = "main", + .root_module = b.createModule(.{ + .root_source_file = b.path("main.zig"), + .target = b.graph.host, + .optimize = optimize, + }), + }); + + const run = b.addRunArtifact(main); + run.clearEnvironment(); + run.setEnvironmentVariable("FOO", "123"); + run.setEnvironmentVariable("EQUALS", "ABC=123"); + run.setEnvironmentVariable("NO_VALUE", ""); + run.setEnvironmentVariable("КИРиллИЦА", "non-ascii አማርኛ \u{10FFFF}"); + if (b.graph.host.result.os.tag == .windows) { + run.setEnvironmentVariable("=Hidden", "hi"); + // \xed\xa0\x80 is a WTF-8 encoded unpaired surrogate code point + run.setEnvironmentVariable("INVALID_UTF16_\xed\xa0\x80", "\xed\xa0\x80"); + } + run.disable_zig_progress = true; + + test_step.dependOn(&run.step); +} diff --git a/test/standalone/env_vars/main.zig b/test/standalone/env_vars/main.zig new file mode 100644 index 000000000000..12b911404a76 --- /dev/null +++ b/test/standalone/env_vars/main.zig @@ -0,0 +1,173 @@ +const std = @import("std"); +const builtin = @import("builtin"); + +// Note: the environment variables under test are set by the build.zig +pub fn main() !void { + var gpa: std.heap.GeneralPurposeAllocator(.{}) = .init; + defer _ = gpa.deinit(); + const allocator = gpa.allocator(); + + var arena_state = std.heap.ArenaAllocator.init(allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + // hasNonEmptyEnvVar + { + try std.testing.expect(try std.process.hasNonEmptyEnvVar(allocator, "FOO")); + try std.testing.expect(!(try std.process.hasNonEmptyEnvVar(allocator, "FOO="))); + try std.testing.expect(!(try std.process.hasNonEmptyEnvVar(allocator, "FO"))); + try std.testing.expect(!(try std.process.hasNonEmptyEnvVar(allocator, "FOOO"))); + if (builtin.os.tag == .windows) { + try std.testing.expect(try std.process.hasNonEmptyEnvVar(allocator, "foo")); + } + try std.testing.expect(try std.process.hasNonEmptyEnvVar(allocator, "EQUALS")); + try std.testing.expect(!(try std.process.hasNonEmptyEnvVar(allocator, "EQUALS=ABC"))); + try std.testing.expect(try std.process.hasNonEmptyEnvVar(allocator, "КИРиллИЦА")); + if (builtin.os.tag == .windows) { + try std.testing.expect(try std.process.hasNonEmptyEnvVar(allocator, "кирИЛЛица")); + } + try std.testing.expect(!(try std.process.hasNonEmptyEnvVar(allocator, "NO_VALUE"))); + try std.testing.expect(!(try std.process.hasNonEmptyEnvVar(allocator, "NOT_SET"))); + if (builtin.os.tag == .windows) { + try std.testing.expect(try std.process.hasNonEmptyEnvVar(allocator, "=HIDDEN")); + try std.testing.expect(try std.process.hasNonEmptyEnvVar(allocator, "INVALID_UTF16_\xed\xa0\x80")); + } + } + + // hasNonEmptyEnvVarContstant + { + try std.testing.expect(std.process.hasNonEmptyEnvVarConstant("FOO")); + try std.testing.expect(!std.process.hasNonEmptyEnvVarConstant("FOO=")); + try std.testing.expect(!std.process.hasNonEmptyEnvVarConstant("FO")); + try std.testing.expect(!std.process.hasNonEmptyEnvVarConstant("FOOO")); + if (builtin.os.tag == .windows) { + try std.testing.expect(std.process.hasNonEmptyEnvVarConstant("foo")); + } + try std.testing.expect(std.process.hasNonEmptyEnvVarConstant("EQUALS")); + try std.testing.expect(!std.process.hasNonEmptyEnvVarConstant("EQUALS=ABC")); + try std.testing.expect(std.process.hasNonEmptyEnvVarConstant("КИРиллИЦА")); + if (builtin.os.tag == .windows) { + try std.testing.expect(std.process.hasNonEmptyEnvVarConstant("кирИЛЛица")); + } + try std.testing.expect(!(std.process.hasNonEmptyEnvVarConstant("NO_VALUE"))); + try std.testing.expect(!(std.process.hasNonEmptyEnvVarConstant("NOT_SET"))); + if (builtin.os.tag == .windows) { + try std.testing.expect(std.process.hasNonEmptyEnvVarConstant("=HIDDEN")); + try std.testing.expect(std.process.hasNonEmptyEnvVarConstant("INVALID_UTF16_\xed\xa0\x80")); + } + } + + // hasEnvVar + { + try std.testing.expect(try std.process.hasEnvVar(allocator, "FOO")); + try std.testing.expect(!(try std.process.hasEnvVar(allocator, "FOO="))); + try std.testing.expect(!(try std.process.hasEnvVar(allocator, "FO"))); + try std.testing.expect(!(try std.process.hasEnvVar(allocator, "FOOO"))); + if (builtin.os.tag == .windows) { + try std.testing.expect(try std.process.hasEnvVar(allocator, "foo")); + } + try std.testing.expect(try std.process.hasEnvVar(allocator, "EQUALS")); + try std.testing.expect(!(try std.process.hasEnvVar(allocator, "EQUALS=ABC"))); + try std.testing.expect(try std.process.hasEnvVar(allocator, "КИРиллИЦА")); + if (builtin.os.tag == .windows) { + try std.testing.expect(try std.process.hasEnvVar(allocator, "кирИЛЛица")); + } + try std.testing.expect(try std.process.hasEnvVar(allocator, "NO_VALUE")); + try std.testing.expect(!(try std.process.hasEnvVar(allocator, "NOT_SET"))); + if (builtin.os.tag == .windows) { + try std.testing.expect(try std.process.hasEnvVar(allocator, "=HIDDEN")); + try std.testing.expect(try std.process.hasEnvVar(allocator, "INVALID_UTF16_\xed\xa0\x80")); + } + } + + // hasEnvVarConstant + { + try std.testing.expect(std.process.hasEnvVarConstant("FOO")); + try std.testing.expect(!std.process.hasEnvVarConstant("FOO=")); + try std.testing.expect(!std.process.hasEnvVarConstant("FO")); + try std.testing.expect(!std.process.hasEnvVarConstant("FOOO")); + if (builtin.os.tag == .windows) { + try std.testing.expect(std.process.hasEnvVarConstant("foo")); + } + try std.testing.expect(std.process.hasEnvVarConstant("EQUALS")); + try std.testing.expect(!std.process.hasEnvVarConstant("EQUALS=ABC")); + try std.testing.expect(std.process.hasEnvVarConstant("КИРиллИЦА")); + if (builtin.os.tag == .windows) { + try std.testing.expect(std.process.hasEnvVarConstant("кирИЛЛица")); + } + try std.testing.expect(std.process.hasEnvVarConstant("NO_VALUE")); + try std.testing.expect(!(std.process.hasEnvVarConstant("NOT_SET"))); + if (builtin.os.tag == .windows) { + try std.testing.expect(std.process.hasEnvVarConstant("=HIDDEN")); + try std.testing.expect(std.process.hasEnvVarConstant("INVALID_UTF16_\xed\xa0\x80")); + } + } + + // getEnvVarOwned + { + try std.testing.expectEqualSlices(u8, "123", try std.process.getEnvVarOwned(arena, "FOO")); + try std.testing.expectError(error.EnvironmentVariableNotFound, std.process.getEnvVarOwned(arena, "FOO=")); + try std.testing.expectError(error.EnvironmentVariableNotFound, std.process.getEnvVarOwned(arena, "FO")); + try std.testing.expectError(error.EnvironmentVariableNotFound, std.process.getEnvVarOwned(arena, "FOOO")); + if (builtin.os.tag == .windows) { + try std.testing.expectEqualSlices(u8, "123", try std.process.getEnvVarOwned(arena, "foo")); + } + try std.testing.expectEqualSlices(u8, "ABC=123", try std.process.getEnvVarOwned(arena, "EQUALS")); + try std.testing.expectError(error.EnvironmentVariableNotFound, std.process.getEnvVarOwned(arena, "EQUALS=ABC")); + try std.testing.expectEqualSlices(u8, "non-ascii አማርኛ \u{10FFFF}", try std.process.getEnvVarOwned(arena, "КИРиллИЦА")); + if (builtin.os.tag == .windows) { + try std.testing.expectEqualSlices(u8, "non-ascii አማርኛ \u{10FFFF}", try std.process.getEnvVarOwned(arena, "кирИЛЛица")); + } + try std.testing.expectEqualSlices(u8, "", try std.process.getEnvVarOwned(arena, "NO_VALUE")); + try std.testing.expectError(error.EnvironmentVariableNotFound, std.process.getEnvVarOwned(arena, "NOT_SET")); + if (builtin.os.tag == .windows) { + try std.testing.expectEqualSlices(u8, "hi", try std.process.getEnvVarOwned(arena, "=HIDDEN")); + try std.testing.expectEqualSlices(u8, "\xed\xa0\x80", try std.process.getEnvVarOwned(arena, "INVALID_UTF16_\xed\xa0\x80")); + } + } + + // parseEnvVarInt + { + try std.testing.expectEqual(123, try std.process.parseEnvVarInt("FOO", u32, 10)); + try std.testing.expectError(error.EnvironmentVariableNotFound, std.process.parseEnvVarInt("FO", u32, 10)); + try std.testing.expectError(error.EnvironmentVariableNotFound, std.process.parseEnvVarInt("FOOO", u32, 10)); + try std.testing.expectEqual(0x123, try std.process.parseEnvVarInt("FOO", u32, 16)); + if (builtin.os.tag == .windows) { + try std.testing.expectEqual(123, try std.process.parseEnvVarInt("foo", u32, 10)); + } + try std.testing.expectError(error.InvalidCharacter, std.process.parseEnvVarInt("EQUALS", u32, 10)); + try std.testing.expectError(error.EnvironmentVariableNotFound, std.process.parseEnvVarInt("EQUALS=ABC", u32, 10)); + try std.testing.expectError(error.InvalidCharacter, std.process.parseEnvVarInt("КИРиллИЦА", u32, 10)); + try std.testing.expectError(error.InvalidCharacter, std.process.parseEnvVarInt("NO_VALUE", u32, 10)); + try std.testing.expectError(error.EnvironmentVariableNotFound, std.process.parseEnvVarInt("NOT_SET", u32, 10)); + if (builtin.os.tag == .windows) { + try std.testing.expectError(error.InvalidCharacter, std.process.parseEnvVarInt("=HIDDEN", u32, 10)); + try std.testing.expectError(error.InvalidCharacter, std.process.parseEnvVarInt("INVALID_UTF16_\xed\xa0\x80", u32, 10)); + } + } + + // EnvMap + { + var env_map = try std.process.getEnvMap(allocator); + defer env_map.deinit(); + + try std.testing.expectEqualSlices(u8, "123", env_map.get("FOO").?); + try std.testing.expectEqual(null, env_map.get("FO")); + try std.testing.expectEqual(null, env_map.get("FOOO")); + if (builtin.os.tag == .windows) { + try std.testing.expectEqualSlices(u8, "123", env_map.get("foo").?); + } + try std.testing.expectEqualSlices(u8, "ABC=123", env_map.get("EQUALS").?); + try std.testing.expectEqual(null, env_map.get("EQUALS=ABC")); + try std.testing.expectEqualSlices(u8, "non-ascii አማርኛ \u{10FFFF}", env_map.get("КИРиллИЦА").?); + if (builtin.os.tag == .windows) { + try std.testing.expectEqualSlices(u8, "non-ascii አማርኛ \u{10FFFF}", env_map.get("кирИЛЛица").?); + } + try std.testing.expectEqualSlices(u8, "", env_map.get("NO_VALUE").?); + try std.testing.expectEqual(null, env_map.get("NOT_SET")); + if (builtin.os.tag == .windows) { + try std.testing.expectEqualSlices(u8, "hi", env_map.get("=HIDDEN").?); + try std.testing.expectEqualSlices(u8, "\xed\xa0\x80", env_map.get("INVALID_UTF16_\xed\xa0\x80").?); + } + } +} From 78ecf3bb3a8022eaaf2c59b590fc165e5491c73c Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Sun, 16 Mar 2025 17:28:26 -0700 Subject: [PATCH 4/5] windows: Document Environment pointer --- lib/std/os/windows.zig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index c29eb0f05a14..54a98c174fa6 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -4855,6 +4855,10 @@ pub const RTL_USER_PROCESS_PARAMETERS = extern struct { DllPath: UNICODE_STRING, ImagePathName: UNICODE_STRING, CommandLine: UNICODE_STRING, + /// Points to a NUL-terminated sequence of NUL-terminated + /// WTF-16 LE encoded `name=value` sequences. + /// Example using string literal syntax: + /// `"NAME=value\x00foo=bar\x00\x00"` Environment: [*:0]WCHAR, dwX: ULONG, dwY: ULONG, From 66dcebcc76d41b125e9c6de9fecd61a8b3a272da Mon Sep 17 00:00:00 2001 From: Ryan Liptak Date: Sun, 16 Mar 2025 17:37:31 -0700 Subject: [PATCH 5/5] getenvW: Take advantage of sliceTo/indexOfScalarPos optimizations Both sliceTo and indexOfScalarPos use SIMD when available to speed up the search. On my x86_64 machine, this leads to getenvW being around 2-3x faster overall. Additionally, any future improvements to sliceTo/indexOfScalarPos will benefit getenvW. --- lib/std/process.zig | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/std/process.zig b/lib/std/process.zig index 26b25b25fe48..fb8256c316af 100644 --- a/lib/std/process.zig +++ b/lib/std/process.zig @@ -527,31 +527,33 @@ pub fn getenvW(key: [*:0]const u16) ?[:0]const u16 { @compileError("Windows-only"); } const key_slice = mem.sliceTo(key, 0); + // '=' anywhere but the start makes this an invalid environment variable name + if (key_slice.len > 0 and std.mem.indexOfScalar(u16, key_slice[1..], '=') != null) { + return null; + } const ptr = windows.peb().ProcessParameters.Environment; var i: usize = 0; while (ptr[i] != 0) { - const key_start = i; + const key_value = mem.sliceTo(ptr[i..], 0); // There are some special environment variables that start with =, // so we need a special case to not treat = as a key/value separator // if it's the first character. // https://devblogs.microsoft.com/oldnewthing/20100506-00/?p=14133 - if (ptr[key_start] == '=') i += 1; - - while (ptr[i] != 0 and ptr[i] != '=') : (i += 1) {} - const this_key = ptr[key_start..i]; - - if (ptr[i] == '=') i += 1; - - const value_start = i; - while (ptr[i] != 0) : (i += 1) {} - const this_value = ptr[value_start..i :0]; + const equal_search_start: usize = if (key_value[0] == '=') 1 else 0; + const equal_index = std.mem.indexOfScalarPos(u16, key_value, equal_search_start, '=') orelse { + // This is enforced by CreateProcess. + // If violated, CreateProcess will fail with INVALID_PARAMETER. + unreachable; // must contain a = + }; + const this_key = key_value[0..equal_index]; if (windows.eqlIgnoreCaseWTF16(key_slice, this_key)) { - return this_value; + return key_value[equal_index + 1 ..]; } - i += 1; // skip over null byte + // skip past the NUL terminator + i += key_value.len + 1; } return null; }