From 7cf03529329c0f8ddcd4a78b127b00939810dca6 Mon Sep 17 00:00:00 2001 From: Hrvoje Niksic Date: Sat, 25 Apr 2026 08:20:01 +0200 Subject: [PATCH 1/2] Fix env_remove to be case-insensitive on Windows --- src/exec.rs | 5 ++- src/spawn.rs | 120 ++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 107 insertions(+), 18 deletions(-) diff --git a/src/exec.rs b/src/exec.rs index 0526ac8..162a91b 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -23,7 +23,7 @@ use std::sync::Arc; use crate::job::Job; pub(crate) use crate::job::{ReadAdapter, ReadErrAdapter, WriteAdapter}; use crate::pipeline::Pipeline; -use crate::spawn::{Arg, OsOptions, SpawnResult, display_escape, spawn}; +use crate::spawn::{Arg, OsOptions, SpawnResult, display_escape, env_keys_cmp, spawn}; use os::*; @@ -299,7 +299,8 @@ impl Exec { /// Other environment variables are inherited by default. pub fn env_remove(mut self, key: impl Into) -> Exec { let key = key.into(); - self.ensure_env().retain(|(k, _v)| *k != key); + self.ensure_env() + .retain(|(k, _v)| env_keys_cmp(k, &key).is_ne()); self } diff --git a/src/spawn.rs b/src/spawn.rs index b3b0214..3f5fe77 100644 --- a/src/spawn.rs +++ b/src/spawn.rs @@ -11,6 +11,7 @@ use crate::process::ExtProcessState; use crate::process::Process; pub(crate) use os::OsOptions; +pub(crate) use os::env_keys_cmp; pub use os::make_pipe; /// A process argument, either regular (quoted on Windows) or raw @@ -291,6 +292,12 @@ pub(crate) mod os { pub const NULL_DEVICE: &str = "/dev/null"; + /// Compares two env var names under the platform's env semantics. Unix env + /// var names are case-sensitive, so this is byte ordering. + pub(crate) fn env_keys_cmp(a: &OsStr, b: &OsStr) -> std::cmp::Ordering { + a.cmp(b) + } + use crate::posix; use std::collections::HashSet; use std::ffi::OsString; @@ -523,7 +530,24 @@ pub(crate) mod os { pub const NULL_DEVICE: &str = "nul"; - use std::collections::HashSet; + /// Compares two env var names under the platform's env semantics. Windows env + /// var names are case-insensitive. The case-fold is currently ASCII-only, + /// which misses non-ASCII case mappings; the stdlib uses the OS's + /// `CompareStringOrdinal` (see `std::sys::process::windows::EnvKey`) for full + /// correctness. Switching to that means rewriting only this function. + pub(crate) fn env_keys_cmp(a: &OsStr, b: &OsStr) -> std::cmp::Ordering { + fn fold_unit(c: u16) -> u16 { + if c < 128 { + (c as u8).to_ascii_uppercase() as u16 + } else { + c + } + } + a.encode_wide() + .map(fold_unit) + .cmp(b.encode_wide().map(fold_unit)) + } + use std::env; use std::ffi::{OsStr, OsString}; use std::fs::File; @@ -574,24 +598,14 @@ pub(crate) mod os { } fn format_env_block(env: &[(OsString, OsString)]) -> Vec { - fn to_uppercase(s: &OsStr) -> OsString { - OsString::from_wide( - &s.encode_wide() - .map(|c| { - if c < 128 { - (c as u8).to_ascii_uppercase() as u16 - } else { - c - } - }) - .collect::>(), - ) - } + // Dedupe by env-var-name semantics, keeping the last occurrence of each + // key. Walk in reverse, retain entries whose key isn't already seen, then + // reverse back to restore original relative order. let mut pruned: Vec<_> = { - let mut seen = HashSet::::new(); + let mut seen = std::collections::BTreeSet::>::new(); env.iter() .rev() - .filter(|&(k, _)| seen.insert(to_uppercase(k))) + .filter(|(k, _)| seen.insert(EnvKey(k))) .collect() }; pruned.reverse(); @@ -606,6 +620,32 @@ pub(crate) mod os { block } + /// Wrapper used as a `BTreeSet` key for env-block dedup. All comparison + /// operators delegate to `env_keys_cmp`, so the dedup uses the platform's + /// env-name semantics without any case-fold logic leaking outside + /// `env_keys_cmp`. + struct EnvKey<'a>(&'a OsStr); + + impl Ord for EnvKey<'_> { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + env_keys_cmp(self.0, other.0) + } + } + + impl PartialOrd for EnvKey<'_> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } + } + + impl PartialEq for EnvKey<'_> { + fn eq(&self, other: &Self) -> bool { + self.cmp(other).is_eq() + } + } + + impl Eq for EnvKey<'_> {} + fn ensure_child_stream( stream: &mut Option>, which: StandardStream, @@ -725,4 +765,52 @@ pub(crate) mod os { } cmdline.push('"' as u16); } + + #[cfg(test)] + mod tests { + use super::*; + + // Parse a Windows env block (KEY=VALUE\0...KEY=VALUE\0\0) back to pairs. + fn parse_block(block: &[u16]) -> Vec<(OsString, OsString)> { + let mut entries = Vec::new(); + let mut start = 0; + for (i, &u) in block.iter().enumerate() { + if u == 0 { + if i == start { + break; + } + let chunk = &block[start..i]; + let eq = chunk.iter().position(|&u| u == b'=' as u16).unwrap(); + entries.push(( + OsString::from_wide(&chunk[..eq]), + OsString::from_wide(&chunk[eq + 1..]), + )); + start = i + 1; + } + } + entries + } + + fn pair(k: &str, v: &str) -> (OsString, OsString) { + (OsString::from(k), OsString::from(v)) + } + + #[test] + fn format_env_block_dedup_keeps_last_occurrence() { + let env = vec![pair("A", "1"), pair("B", "x"), pair("A", "2")]; + assert_eq!( + parse_block(&format_env_block(&env)), + vec![pair("B", "x"), pair("A", "2")] + ); + } + + #[test] + fn format_env_block_dedup_is_case_insensitive() { + let env = vec![pair("Path", "old"), pair("FOO", "y"), pair("PATH", "new")]; + assert_eq!( + parse_block(&format_env_block(&env)), + vec![pair("FOO", "y"), pair("PATH", "new")] + ); + } + } } From 9b4fe9f286a3bf07c047b8594d564de0e0dd6013 Mon Sep 17 00:00:00 2001 From: Hrvoje Niksic Date: Sat, 25 Apr 2026 09:14:41 +0200 Subject: [PATCH 2/2] Use CompareStringOrdinal for Windows env-key comparison --- Cargo.toml | 2 +- src/spawn.rs | 58 +++++++++++++++++++++++++++++----------------------- src/win32.rs | 25 ++++++++++++++++++++++ 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 304e33d..1f3232e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ edition = "2024" libc = "0.2.78" [target.'cfg(windows)'.dependencies] -winapi = { version = "0.3.8", features = ["std", "handleapi", "namedpipeapi", "processenv", "synchapi", "winerror", "processthreadsapi", "winbase", "fileapi", "minwinbase", "ioapiset"] } +winapi = { version = "0.3.8", features = ["std", "handleapi", "namedpipeapi", "processenv", "synchapi", "winerror", "processthreadsapi", "winbase", "fileapi", "minwinbase", "ioapiset", "stringapiset"] } [dev-dependencies] tempfile = "3.3.0" diff --git a/src/spawn.rs b/src/spawn.rs index 3f5fe77..fb5b807 100644 --- a/src/spawn.rs +++ b/src/spawn.rs @@ -531,21 +531,13 @@ pub(crate) mod os { pub const NULL_DEVICE: &str = "nul"; /// Compares two env var names under the platform's env semantics. Windows env - /// var names are case-insensitive. The case-fold is currently ASCII-only, - /// which misses non-ASCII case mappings; the stdlib uses the OS's - /// `CompareStringOrdinal` (see `std::sys::process::windows::EnvKey`) for full - /// correctness. Switching to that means rewriting only this function. + /// var names are case-insensitive; this delegates to the OS via + /// `CompareStringOrdinal(bIgnoreCase=TRUE)`, matching what the stdlib uses for + /// `std::sys::process::windows::EnvKey`. pub(crate) fn env_keys_cmp(a: &OsStr, b: &OsStr) -> std::cmp::Ordering { - fn fold_unit(c: u16) -> u16 { - if c < 128 { - (c as u8).to_ascii_uppercase() as u16 - } else { - c - } - } - a.encode_wide() - .map(fold_unit) - .cmp(b.encode_wide().map(fold_unit)) + let a: Vec = a.encode_wide().collect(); + let b: Vec = b.encode_wide().collect(); + win32::compare_string_ordinal(&a, &b, true) } use std::env; @@ -602,10 +594,10 @@ pub(crate) mod os { // key. Walk in reverse, retain entries whose key isn't already seen, then // reverse back to restore original relative order. let mut pruned: Vec<_> = { - let mut seen = std::collections::BTreeSet::>::new(); + let mut seen = std::collections::BTreeSet::::new(); env.iter() .rev() - .filter(|(k, _)| seen.insert(EnvKey(k))) + .filter(|(k, _)| seen.insert(EnvKey::new(k))) .collect() }; pruned.reverse(); @@ -620,31 +612,36 @@ pub(crate) mod os { block } - /// Wrapper used as a `BTreeSet` key for env-block dedup. All comparison - /// operators delegate to `env_keys_cmp`, so the dedup uses the platform's - /// env-name semantics without any case-fold logic leaking outside - /// `env_keys_cmp`. - struct EnvKey<'a>(&'a OsStr); + /// `BTreeSet` key for env-block dedup. Caches the UTF-16 encoding so each + /// compare in the set hits the OS API directly without re-encoding - + /// matches the approach in `std::sys::process::windows::EnvKey`. + struct EnvKey(Vec); - impl Ord for EnvKey<'_> { + impl EnvKey { + fn new(s: &OsStr) -> Self { + EnvKey(s.encode_wide().collect()) + } + } + + impl Ord for EnvKey { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - env_keys_cmp(self.0, other.0) + win32::compare_string_ordinal(&self.0, &other.0, true) } } - impl PartialOrd for EnvKey<'_> { + impl PartialOrd for EnvKey { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } - impl PartialEq for EnvKey<'_> { + impl PartialEq for EnvKey { fn eq(&self, other: &Self) -> bool { self.cmp(other).is_eq() } } - impl Eq for EnvKey<'_> {} + impl Eq for EnvKey {} fn ensure_child_stream( stream: &mut Option>, @@ -812,5 +809,14 @@ pub(crate) mod os { vec![pair("FOO", "y"), pair("PATH", "new")] ); } + + #[test] + fn format_env_block_dedup_folds_non_ascii_case() { + // Beyond ASCII: U+00C4 LATIN CAPITAL LETTER A WITH DIAERESIS vs U+00E4 + // (lowercase). Equal under Windows' case-insensitive ordinal compare; + // unequal under a naive ASCII-only fold. + let env = vec![pair("Ä", "old"), pair("ä", "new")]; + assert_eq!(parse_block(&format_env_block(&env)), vec![pair("ä", "new")]); + } } } diff --git a/src/win32.rs b/src/win32.rs index 9da733b..dba8e01 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -484,6 +484,31 @@ pub fn SetHandleInformation(handle: &File, dwMask: u32, dwFlags: u32) -> Result< Ok(()) } +/// Compare two UTF-16 strings using the OS's ordinal comparison. With +/// `ignore_case=true` the OS applies its language-independent uppercase mapping +/// (operates per UTF-16 code unit, surrogate-preserving), then ordinal-compares +/// the result. See https://learn.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-comparestringordinal +pub fn compare_string_ordinal(a: &[u16], b: &[u16], ignore_case: bool) -> std::cmp::Ordering { + const CSTR_LESS_THAN: i32 = 1; + const CSTR_EQUAL: i32 = 2; + const CSTR_GREATER_THAN: i32 = 3; + let r = unsafe { + winapi::um::stringapiset::CompareStringOrdinal( + a.as_ptr(), + a.len() as i32, + b.as_ptr(), + b.len() as i32, + if ignore_case { TRUE } else { FALSE }, + ) + }; + match r { + CSTR_LESS_THAN => std::cmp::Ordering::Less, + CSTR_EQUAL => std::cmp::Ordering::Equal, + CSTR_GREATER_THAN => std::cmp::Ordering::Greater, + _ => panic!("CompareStringOrdinal failed: {}", Error::last_os_error()), + } +} + #[allow(clippy::too_many_arguments)] pub fn CreateProcess( appname: Option<&OsStr>,