From b17e64b8e9b0fcb59f534fcadc053cb6f12ca989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Tue, 25 Aug 2026 14:35:56 +1200 Subject: [PATCH 1/3] feat(pty): encode Windows command lines Port the exact Win32 application-name and mutable command-line encoder from revisions bba7b2e51349, d7ba78b1a35e, 7acca84d7ff4, and 827b1483430e. Adapt it to consume the shared ordered CommandArg intent directly. Co-Authored-By: Claude --- src/tokio/pty.rs | 3 + src/tokio/pty/windows/command.rs | 361 +++++++++++++++++++++++++++++++ src/tokio/pty/windows/mod.rs | 7 + 3 files changed, 371 insertions(+) create mode 100644 src/tokio/pty/windows/command.rs create mode 100644 src/tokio/pty/windows/mod.rs diff --git a/src/tokio/pty.rs b/src/tokio/pty.rs index f6b4400..b2f75ed 100644 --- a/src/tokio/pty.rs +++ b/src/tokio/pty.rs @@ -76,6 +76,9 @@ mod unix; target_os = "solaris" )))] mod unsupported; +#[cfg(windows)] +#[allow(dead_code)] +mod windows; #[cfg(any( target_os = "android", target_os = "dragonfly", diff --git a/src/tokio/pty/windows/command.rs b/src/tokio/pty/windows/command.rs new file mode 100644 index 0000000..da33365 --- /dev/null +++ b/src/tokio/pty/windows/command.rs @@ -0,0 +1,361 @@ +//! Win32 application-name and mutable command-line encoding. +//! +//! Program and argument data stays in WTF-16. Regular arguments follow the Microsoft C runtime's +//! backslash-and-quote rules, while raw fragments are appended unchanged and in registration order. + +use std::{ffi::OsStr, io, os::windows::ffi::OsStrExt}; + +use crate::CommandArg; + +const BACKSLASH: u16 = b'\\' as u16; +const DOUBLE_QUOTE: u16 = b'"' as u16; +const SPACE: u16 = b' ' as u16; +const TAB: u16 = b'\t' as u16; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(super) struct WideCString(Vec); + +impl WideCString { + fn from_units(mut units: Vec) -> Self { + debug_assert!(!units.contains(&0)); + units.push(0); + Self(units) + } + + pub(super) fn from_os(value: &OsStr, field: &'static str) -> io::Result { + Ok(Self::from_units(encode(value, field)?)) + } + + pub(super) fn as_ptr(&self) -> *const u16 { + self.0.as_ptr() + } + + pub(super) fn as_mut_ptr(&mut self) -> *mut u16 { + self.0.as_mut_ptr() + } + + pub(super) fn as_units(&self) -> &[u16] { + &self.0 + } +} + +#[derive(Debug, Eq, PartialEq)] +pub(super) struct PreparedCommandLine { + pub(super) application_name: WideCString, + pub(super) command_line: WideCString, +} + +pub(super) fn prepare_command_line( + program: &OsStr, + args: &[CommandArg], +) -> io::Result { + let program = encode(program, "PTY program")?; + let application_name = WideCString::from_units(program.clone()); + let mut command_line = Vec::new(); + append_argv0(&mut command_line, &program)?; + + for arg in args { + command_line.push(SPACE); + match arg { + CommandArg::Regular(arg) => { + let arg = encode(arg, "PTY argument")?; + append_regular(&mut command_line, &arg, false); + } + CommandArg::Raw(arg) => { + command_line.extend(encode(arg, "PTY raw argument")?); + } + } + } + + Ok(PreparedCommandLine { + application_name, + command_line: WideCString::from_units(command_line), + }) +} + +pub(super) fn encode(value: &OsStr, field: &'static str) -> io::Result> { + let units = value.encode_wide().collect::>(); + if units.contains(&0) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("{field} contains an embedded NUL"), + )); + } + Ok(units) +} + +fn append_argv0(command_line: &mut Vec, program: &[u16]) -> io::Result<()> { + if program.contains(&DOUBLE_QUOTE) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "PTY program contains a double quote", + )); + } + command_line.push(DOUBLE_QUOTE); + command_line.extend(program); + command_line.push(DOUBLE_QUOTE); + Ok(()) +} + +fn append_regular(command_line: &mut Vec, arg: &[u16], force_quotes: bool) { + let quoted = + force_quotes || arg.is_empty() || arg.iter().any(|unit| matches!(*unit, SPACE | TAB)); + if quoted { + command_line.push(DOUBLE_QUOTE); + } + + let mut backslashes = 0; + for &unit in arg { + if unit == BACKSLASH { + backslashes += 1; + continue; + } + + append_backslashes(command_line, backslashes); + if unit == DOUBLE_QUOTE { + append_backslashes(command_line, backslashes); + command_line.push(BACKSLASH); + } + command_line.push(unit); + backslashes = 0; + } + + append_backslashes(command_line, backslashes); + if quoted { + append_backslashes(command_line, backslashes); + command_line.push(DOUBLE_QUOTE); + } +} + +fn append_backslashes(command_line: &mut Vec, count: usize) { + for _ in 0..count { + command_line.push(BACKSLASH); + } +} + +#[cfg(test)] +mod tests { + use std::{ + ffi::OsString, + os::windows::ffi::{OsStrExt, OsStringExt}, + }; + + use super::*; + + fn os(units: &[u16]) -> OsString { + OsString::from_wide(units) + } + + fn regular(value: &str) -> CommandArg { + CommandArg::Regular(OsString::from(value)) + } + + fn raw(value: &str) -> CommandArg { + CommandArg::Raw(OsString::from(value)) + } + + fn prepare(program: OsString, args: Vec) -> io::Result { + prepare_command_line(&program, &args) + } + + fn terminated(value: &str) -> Vec { + OsStr::new(value).encode_wide().chain([0]).collect() + } + + #[test] + fn quotes_regular_arguments_with_windows_crt_rules() { + let cases = [ + ("none", Vec::new(), "\"tool\""), + ("empty", vec![regular("")], "\"tool\" \"\""), + ("simple", vec![regular("plain")], "\"tool\" plain"), + ( + "space", + vec![regular("two words")], + "\"tool\" \"two words\"", + ), + ( + "tab", + vec![regular("two\twords")], + "\"tool\" \"two\twords\"", + ), + ("quote", vec![regular("a\"b")], "\"tool\" a\\\"b"), + ( + "backslash quote", + vec![regular("a\\\"b")], + "\"tool\" a\\\\\\\"b", + ), + ("unquoted slash", vec![regular("a\\")], "\"tool\" a\\"), + ( + "quoted trailing slash", + vec![regular("a b\\")], + "\"tool\" \"a b\\\\\"", + ), + ]; + + for (name, args, expected) in cases { + let prepared = prepare(OsString::from("tool"), args).unwrap(); + assert_eq!( + prepared.command_line.as_units(), + terminated(expected), + "{name}" + ); + assert_eq!( + prepared.application_name.as_units(), + terminated("tool"), + "{name}" + ); + } + } + + #[test] + fn preserves_interleaved_raw_fragments_exactly() { + let prepared = prepare( + OsString::from("tool"), + vec![ + regular("two words"), + raw(r#"/D "literal""#), + regular("plain"), + raw(""), + ], + ) + .unwrap(); + assert_eq!( + prepared.command_line.as_units(), + terminated(r#""tool" "two words" /D "literal" plain "#) + ); + } + + #[test] + fn preserves_lone_surrogates_in_program_and_arguments() { + let program = [b't' as u16, 0xd800, b'o' as u16]; + let prepared = prepare( + os(&program), + vec![ + CommandArg::Regular(os(&[0xdc00])), + CommandArg::Raw(os(&[0xd801])), + ], + ) + .unwrap(); + + assert_eq!( + prepared.application_name.as_units(), + [program.as_slice(), &[0]].concat() + ); + assert_eq!( + prepared.command_line.as_units(), + [ + DOUBLE_QUOTE, + program[0], + program[1], + program[2], + DOUBLE_QUOTE, + SPACE, + 0xdc00, + SPACE, + 0xd801, + 0, + ] + ); + } + + #[test] + fn encodes_argv0_with_its_special_windows_rules() { + let prepared = prepare(OsString::from(r"C:\Program Files\"), Vec::new()).unwrap(); + assert_eq!( + prepared.command_line.as_units(), + [ + DOUBLE_QUOTE, + b'C' as u16, + b':' as u16, + BACKSLASH, + b'P' as u16, + b'r' as u16, + b'o' as u16, + b'g' as u16, + b'r' as u16, + b'a' as u16, + b'm' as u16, + SPACE, + b'F' as u16, + b'i' as u16, + b'l' as u16, + b'e' as u16, + b's' as u16, + BACKSLASH, + DOUBLE_QUOTE, + 0, + ] + ); + } + + #[test] + fn quotes_multiple_backslash_runs_in_regular_arguments() { + let arg = os(&[ + b'a' as u16, + BACKSLASH, + BACKSLASH, + DOUBLE_QUOTE, + b'b' as u16, + BACKSLASH, + BACKSLASH, + ]); + let prepared = prepare(OsString::from("tool"), vec![CommandArg::Regular(arg)]).unwrap(); + assert_eq!( + prepared.command_line.as_units(), + [ + DOUBLE_QUOTE, + b't' as u16, + b'o' as u16, + b'o' as u16, + b'l' as u16, + DOUBLE_QUOTE, + SPACE, + b'a' as u16, + BACKSLASH, + BACKSLASH, + BACKSLASH, + BACKSLASH, + BACKSLASH, + DOUBLE_QUOTE, + b'b' as u16, + BACKSLASH, + BACKSLASH, + 0, + ] + ); + } + + #[test] + fn rejects_unrepresentable_quotes_in_argv0() { + let error = prepare(OsString::from("bad\"program"), Vec::new()).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert_eq!(error.to_string(), "PTY program contains a double quote"); + } + + #[test] + fn rejects_embedded_nuls_with_stable_errors() { + let cases = [ + ( + os(&[b't' as u16, 0]), + Vec::new(), + "PTY program contains an embedded NUL", + ), + ( + OsString::from("tool"), + vec![CommandArg::Regular(os(&[b'a' as u16, 0]))], + "PTY argument contains an embedded NUL", + ), + ( + OsString::from("tool"), + vec![CommandArg::Raw(os(&[b'a' as u16, 0]))], + "PTY raw argument contains an embedded NUL", + ), + ]; + + for (program, args, message) in cases { + let error = prepare(program, args).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert_eq!(error.to_string(), message); + } + } +} diff --git a/src/tokio/pty/windows/mod.rs b/src/tokio/pty/windows/mod.rs new file mode 100644 index 0000000..5ec6bda --- /dev/null +++ b/src/tokio/pty/windows/mod.rs @@ -0,0 +1,7 @@ +//! Exact Win32 process-spawn intent preparation. +//! +//! This private model consumes tracked command state directly. It retains the WTF-16 data needed by +//! the ConPTY backend's `CreateProcessW` call without committing those implementation invariants to +//! the public API. + +pub(super) mod command; From bce61c40f15235a16e9853961943a9a960a766b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Tue, 25 Aug 2026 14:44:52 +1200 Subject: [PATCH 2/3] feat(pty): construct exact Windows environments Port the exact Win32 environment-block construction from revisions b6af5f7fb913, 0dd905b2c331, e548472946ed, and 9b44a78c6167. Adapt it to consume portable inheritance and key/value changes without an intermediate PTY command model. Co-Authored-By: Claude --- Cargo.toml | 9 +- src/tokio/pty/windows/environment.rs | 529 +++++++++++++++++++++++++++ src/tokio/pty/windows/mod.rs | 1 + 3 files changed, 538 insertions(+), 1 deletion(-) create mode 100644 src/tokio/pty/windows/environment.rs diff --git a/Cargo.toml b/Cargo.toml index 3229801..db4afc1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,7 +53,14 @@ std = ["dep:nix"] tokio1 = ["dep:nix", "dep:futures", "dep:tokio"] ## Tokio pseudo-terminal transport -pty = ["tokio1", "nix/term", "tokio/net"] +pty = [ + "tokio1", + "nix/term", + "tokio/net", + "dep:windows", + "windows/Win32_Globalization", + "windows/Win32_System_Environment", +] ## Wrapper: Creation Flags creation-flags = ["dep:windows", "windows/Win32_System_Threading"] diff --git a/src/tokio/pty/windows/environment.rs b/src/tokio/pty/windows/environment.rs new file mode 100644 index 0000000..be36e53 --- /dev/null +++ b/src/tokio/pty/windows/environment.rs @@ -0,0 +1,529 @@ +//! Win32 environment inheritance and explicit block construction. +//! +//! An unchanged environment remains native inheritance. Once modified, the inherited block is read +//! without converting through Unicode scalar values, changes use Windows case-insensitive key +//! semantics, and the resulting WTF-16 block is sorted deterministically and double-NUL terminated. + +use std::{cmp::Ordering, ffi::OsStr, io, slice}; + +use windows::{ + Win32::{ + Globalization::{CSTR_EQUAL, CSTR_GREATER_THAN, CSTR_LESS_THAN, CompareStringOrdinal}, + System::Environment::{FreeEnvironmentStringsW, GetEnvironmentStringsW}, + }, + core::PWSTR, +}; + +use super::command::encode; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(super) struct EnvironmentVariable { + pub(super) key: Vec, + pub(super) value: Vec, +} + +#[derive(Debug, Eq, PartialEq)] +pub(super) enum PreparedEnvironment { + Inherit, + Block(Vec), +} + +impl PreparedEnvironment { + pub(super) fn as_ptr(&self) -> *const u16 { + match self { + Self::Inherit => std::ptr::null(), + Self::Block(block) => block.as_ptr(), + } + } + + pub(super) fn is_inherited(&self) -> bool { + matches!(self, Self::Inherit) + } +} + +fn encode_key(key: &OsStr) -> io::Result> { + let key = encode(key, "PTY environment key")?; + if key.is_empty() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "PTY environment key cannot be empty", + )); + } + let name = if key.first() == Some(&(b'=' as u16)) { + &key[1..] + } else { + &key[..] + }; + if name.is_empty() || name.contains(&(b'=' as u16)) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "PTY environment key contains an invalid equals sign", + )); + } + Ok(key) +} + +pub(super) fn prepare_environment<'a>( + inherits: bool, + changes: impl IntoIterator)>, +) -> io::Result { + prepare_environment_with(inherits, changes, inherited_environment) +} + +pub(super) fn prepare_environment_with<'a>( + inherits: bool, + changes: impl IntoIterator)>, + capture: impl FnOnce() -> io::Result>, +) -> io::Result { + let changes = changes + .into_iter() + .map(|(key, value)| { + Ok(match value { + Some(value) => EncodedChange::Set(EnvironmentVariable { + key: encode_key(key)?, + value: encode(value, "PTY environment value")?, + }), + None => EncodedChange::Remove(encode_key(key)?), + }) + }) + .collect::>>()?; + + if inherits && changes.is_empty() { + return Ok(PreparedEnvironment::Inherit); + } + + let mut variables = Vec::new(); + if inherits { + for variable in capture()? { + set_variable(&mut variables, variable); + } + } + for change in changes { + match change { + EncodedChange::Set(variable) => set_variable(&mut variables, variable), + EncodedChange::Remove(key) => { + variables.retain(|variable| !windows_equal(&variable.key, &key)); + } + } + } + variables.sort_by(|left, right| windows_compare(&left.key, &right.key)); + + let mut block = Vec::new(); + for variable in variables { + block.extend(variable.key); + block.push(b'=' as u16); + block.extend(variable.value); + block.push(0); + } + block.push(0); + if block.len() == 1 { + block.push(0); + } + Ok(PreparedEnvironment::Block(block)) +} + +#[derive(Debug)] +enum EncodedChange { + Set(EnvironmentVariable), + Remove(Vec), +} + +fn set_variable(variables: &mut Vec, variable: EnvironmentVariable) { + if let Some(existing) = variables + .iter_mut() + .find(|existing| windows_equal(&existing.key, &variable.key)) + { + *existing = variable; + } else { + variables.push(variable); + } +} + +fn windows_equal(left: &[u16], right: &[u16]) -> bool { + windows_compare(left, right) == Ordering::Equal +} + +fn windows_compare(left: &[u16], right: &[u16]) -> Ordering { + // SAFETY: both slices remain live for the call and CompareStringOrdinal accepts explicit lengths. + match unsafe { CompareStringOrdinal(left, right, true) } { + CSTR_LESS_THAN => Ordering::Less, + CSTR_EQUAL => Ordering::Equal, + CSTR_GREATER_THAN => Ordering::Greater, + _ => left.cmp(right), + } +} + +fn inherited_environment() -> io::Result> { + // SAFETY: ownership of a successful environment block is immediately placed in a drop guard. + let block = unsafe { GetEnvironmentStringsW() }; + if block.is_null() { + return Err(io::Error::last_os_error()); + } + let block = EnvironmentBlock(block); + parse_environment_block(block.0) +} + +struct EnvironmentBlock(PWSTR); + +impl Drop for EnvironmentBlock { + fn drop(&mut self) { + // SAFETY: this is the same non-null pointer returned by GetEnvironmentStringsW. + let _ = unsafe { FreeEnvironmentStringsW(self.0) }; + } +} + +fn parse_environment_block(block: PWSTR) -> io::Result> { + let mut variables = Vec::new(); + let mut cursor = block.0; + loop { + let mut len = 0; + // SAFETY: GetEnvironmentStringsW returns a double-NUL-terminated sequence. + while unsafe { *cursor.add(len) } != 0 { + len += 1; + } + if len == 0 { + break; + } + // SAFETY: the scan above established that this entry contains len initialized units. + let entry = unsafe { slice::from_raw_parts(cursor, len) }; + variables.push(parse_environment_entry(entry)?); + // SAFETY: advance over the entry and its terminating NUL within the environment block. + cursor = unsafe { cursor.add(len + 1) }; + } + Ok(variables) +} + +fn parse_environment_entry(entry: &[u16]) -> io::Result { + let separator = if entry.first() == Some(&(b'=' as u16)) { + entry[1..] + .iter() + .position(|unit| *unit == b'=' as u16) + .map(|position| position + 1) + } else { + entry.iter().position(|unit| *unit == b'=' as u16) + } + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + "Windows environment entry has no key-value separator", + ) + })?; + + Ok(EnvironmentVariable { + key: entry[..separator].to_vec(), + value: entry[separator + 1..].to_vec(), + }) +} + +#[cfg(test)] +mod tests { + use std::{ + ffi::{OsStr, OsString}, + os::windows::ffi::{OsStrExt, OsStringExt}, + }; + + use windows::core::PWSTR; + + use super::*; + + fn units(value: &str) -> Vec { + OsStr::new(value).encode_wide().collect() + } + + fn os(units: &[u16]) -> OsString { + OsString::from_wide(units) + } + + fn variable(key: &str, value: &str) -> EnvironmentVariable { + EnvironmentVariable { + key: units(key), + value: units(value), + } + } + + #[derive(Debug)] + enum EnvChange { + Set(OsString, OsString), + Remove(OsString), + } + + #[derive(Debug)] + struct EnvironmentIntent { + clear: bool, + changes: Vec, + } + + fn set(key: &str, value: &str) -> EnvChange { + EnvChange::Set(OsString::from(key), OsString::from(value)) + } + + fn remove(key: &str) -> EnvChange { + EnvChange::Remove(OsString::from(key)) + } + + fn intent(clear: bool, changes: Vec) -> EnvironmentIntent { + EnvironmentIntent { clear, changes } + } + + fn prepare_environment_with( + intent: &EnvironmentIntent, + capture: impl FnOnce() -> io::Result>, + ) -> io::Result { + super::prepare_environment_with( + !intent.clear, + intent.changes.iter().map(|change| match change { + EnvChange::Set(key, value) => (key.as_os_str(), Some(value.as_os_str())), + EnvChange::Remove(key) => (key.as_os_str(), None), + }), + capture, + ) + } + + fn block(entries: &[(&str, &str)]) -> PreparedEnvironment { + let mut block = Vec::new(); + for (key, value) in entries { + block.extend(units(key)); + block.push(b'=' as u16); + block.extend(units(value)); + block.push(0); + } + block.push(0); + if block.len() == 1 { + block.push(0); + } + PreparedEnvironment::Block(block) + } + + #[test] + fn inherits_without_capturing_an_unchanged_environment() { + let prepared = prepare_environment_with(&intent(false, Vec::new()), || { + panic!("unchanged environment must not be captured") + }) + .unwrap(); + assert!(prepared.is_inherited()); + assert!(prepared.as_ptr().is_null()); + } + + #[test] + fn clearing_to_an_empty_environment_does_not_capture_the_parent() { + let prepared = prepare_environment_with(&intent(true, Vec::new()), || { + panic!("cleared environment must not be captured") + }) + .unwrap(); + assert_eq!(prepared, PreparedEnvironment::Block(vec![0, 0])); + } + + #[test] + fn applies_case_insensitive_changes_and_sorts_deterministically() { + let parent = vec![ + variable("Path", "parent"), + variable("KEEP", "one"), + variable("remove", "gone"), + variable("=C:", r"C:\work"), + ]; + let changes = vec![ + set("PATH", "first"), + set("path", "second"), + remove("ReMoVe"), + set("Alpha", "a"), + remove("missing"), + ]; + + let first = prepare_environment_with(&intent(false, changes), || Ok(parent)).unwrap(); + assert_eq!( + first, + block(&[ + ("=C:", r"C:\work"), + ("Alpha", "a"), + ("KEEP", "one"), + ("path", "second"), + ]) + ); + + let second = prepare_environment_with( + &intent( + false, + vec![ + set("PATH", "first"), + set("path", "second"), + remove("ReMoVe"), + set("Alpha", "a"), + remove("missing"), + ], + ), + || { + Ok(vec![ + variable("Path", "parent"), + variable("KEEP", "one"), + variable("remove", "gone"), + variable("=C:", r"C:\work"), + ]) + }, + ) + .unwrap(); + assert_eq!(first, second); + } + + #[test] + fn clear_discards_the_parent_before_applying_changes() { + let prepared = prepare_environment_with(&intent(true, vec![set("Only", "value")]), || { + panic!("cleared environment must not be captured") + }) + .unwrap(); + assert_eq!(prepared, block(&[("Only", "value")])); + } + + #[test] + fn normalizes_case_collisions_in_the_inherited_environment() { + let prepared = prepare_environment_with(&intent(false, vec![remove("absent")]), || { + Ok(vec![variable("Name", "first"), variable("NAME", "second")]) + }) + .unwrap(); + assert_eq!(prepared, block(&[("NAME", "second")])); + } + + #[test] + fn applies_non_ascii_case_insensitive_collisions() { + let prepared = prepare_environment_with(&intent(false, vec![set("äBC", "child")]), || { + Ok(vec![variable("Äbc", "parent")]) + }) + .unwrap(); + assert_eq!(prepared, block(&[("äBC", "child")])); + } + + #[test] + fn preserves_lone_surrogates() { + let key = os(&[b'K' as u16, 0xd800]); + let value = os(&[b'V' as u16, 0xdc00]); + let prepared = + prepare_environment_with(&intent(false, vec![EnvChange::Set(key, value)]), || { + Ok(Vec::new()) + }) + .unwrap(); + assert_eq!( + prepared, + PreparedEnvironment::Block(vec![ + b'K' as u16, + 0xd800, + b'=' as u16, + b'V' as u16, + 0xdc00, + 0, + 0, + ]) + ); + } + + #[test] + fn captures_and_releases_the_native_parent_environment() { + let variables = inherited_environment().unwrap(); + assert!( + variables + .iter() + .all(|variable| !variable.key.contains(&0) && !variable.value.contains(&0)) + ); + } + + #[test] + fn parses_drive_current_directory_variables() { + let mut raw = units(r"=C:=C:\work"); + raw.push(0); + raw.extend(units("Name=value")); + raw.extend([0, 0]); + let parsed = parse_environment_block(PWSTR(raw.as_mut_ptr())).unwrap(); + assert_eq!( + parsed, + vec![variable("=C:", r"C:\work"), variable("Name", "value")] + ); + } + + #[test] + fn applies_explicit_drive_current_directory_changes() { + let prepared = prepare_environment_with( + &intent(false, vec![remove("=C:"), set("=D:", r"D:\child")]), + || { + Ok(vec![ + variable("=C:", r"C:\parent"), + variable("Name", "value"), + ]) + }, + ) + .unwrap(); + assert_eq!(prepared, block(&[("=D:", r"D:\child"), ("Name", "value")])); + + let cleared = + prepare_environment_with(&intent(true, vec![set("=C:", r"C:\restored")]), || { + panic!("a cleared environment must not be captured") + }) + .unwrap(); + assert_eq!(cleared, block(&[("=C:", r"C:\restored")])); + } + + #[test] + fn rejects_invalid_explicit_keys_before_capturing_the_parent() { + let cases = [ + ( + intent(false, vec![set("", "value")]), + "PTY environment key cannot be empty", + ), + ( + intent(false, vec![remove("")]), + "PTY environment key cannot be empty", + ), + ( + intent(false, vec![set("A=B", "value")]), + "PTY environment key contains an invalid equals sign", + ), + ( + intent(false, vec![remove("=")]), + "PTY environment key contains an invalid equals sign", + ), + ( + intent(false, vec![remove("=C:=")]), + "PTY environment key contains an invalid equals sign", + ), + ]; + + for (intent, message) in cases { + let error = prepare_environment_with(&intent, || { + panic!("invalid explicit keys must be rejected before capture") + }) + .unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert_eq!(error.to_string(), message); + } + } + + #[test] + fn rejects_environment_nuls_before_capturing_the_parent() { + let cases = [ + ( + intent( + false, + vec![EnvChange::Set(os(&[b'K' as u16, 0]), OsString::from("v"))], + ), + "PTY environment key contains an embedded NUL", + ), + ( + intent( + false, + vec![EnvChange::Set(OsString::from("K"), os(&[b'V' as u16, 0]))], + ), + "PTY environment value contains an embedded NUL", + ), + ( + intent(false, vec![EnvChange::Remove(os(&[b'K' as u16, 0]))]), + "PTY environment key contains an embedded NUL", + ), + ]; + + for (intent, message) in cases { + let error = prepare_environment_with(&intent, || { + panic!("invalid explicit environment must be rejected before capture") + }) + .unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert_eq!(error.to_string(), message); + } + } +} diff --git a/src/tokio/pty/windows/mod.rs b/src/tokio/pty/windows/mod.rs index 5ec6bda..91219b4 100644 --- a/src/tokio/pty/windows/mod.rs +++ b/src/tokio/pty/windows/mod.rs @@ -5,3 +5,4 @@ //! the public API. pub(super) mod command; +pub(super) mod environment; From e3d63390827eb83c85308854d86b83bd1e2f2094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Tue, 25 Aug 2026 14:54:29 +1200 Subject: [PATCH 3/3] feat(pty): prepare tracked Windows attempts Port the aggregate Win32 preparation and policy coverage from revisions 9d48043aaf30, ccaa006716e0, 7acca84d7ff4, and 827b1483430e. Consume SpawnAttempt getters and WindowsSpawnPolicy directly, allowing any wrapper that leaves the shared attempt portable. Co-Authored-By: Claude --- Cargo.toml | 1 + src/tokio/pty/windows/mod.rs | 302 ++++++++++++++++++++++++++++++++++- 2 files changed, 300 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index db4afc1..c991070 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,6 +60,7 @@ pty = [ "dep:windows", "windows/Win32_Globalization", "windows/Win32_System_Environment", + "windows/Win32_System_Threading", ] ## Wrapper: Creation Flags diff --git a/src/tokio/pty/windows/mod.rs b/src/tokio/pty/windows/mod.rs index 91219b4..6ef721d 100644 --- a/src/tokio/pty/windows/mod.rs +++ b/src/tokio/pty/windows/mod.rs @@ -1,8 +1,304 @@ //! Exact Win32 process-spawn intent preparation. //! -//! This private model consumes tracked command state directly. It retains the WTF-16 data needed by -//! the ConPTY backend's `CreateProcessW` call without committing those implementation invariants to -//! the public API. +//! This private model consumes tracked spawn-attempt state directly. It retains the WTF-16 data and +//! portable wrapper policy needed by the ConPTY backend's `CreateProcessW` call without committing +//! those implementation invariants to the public API. + +use std::{ffi::OsStr, io}; + +use crate::WindowsSpawnPolicy; + +use super::super::SpawnAttempt; +use command::{PreparedCommandLine, WideCString, prepare_command_line}; +use environment::{PreparedEnvironment, prepare_environment}; pub(super) mod command; pub(super) mod environment; + +#[derive(Debug, Eq, PartialEq)] +struct PreparedWindowsCommand { + application_name: WideCString, + command_line: WideCString, + environment: PreparedEnvironment, + current_dir: Option, + creation: WindowsSpawnPolicy, +} + +fn prepare(attempt: &SpawnAttempt) -> io::Result { + prepare_with(attempt, |inherits, changes| { + prepare_environment(inherits, changes) + }) +} + +fn prepare_with<'a>( + attempt: &'a SpawnAttempt, + build_environment: impl FnOnce( + bool, + Box)> + 'a>, + ) -> io::Result, +) -> io::Result { + let args = attempt.get_portable_args().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "Windows PTY preparation requires portable command arguments", + ) + })?; + let PreparedCommandLine { + application_name, + command_line, + } = prepare_command_line(attempt.get_program(), args)?; + let current_dir = attempt + .get_current_dir() + .map(|directory| WideCString::from_os(directory.as_os_str(), "PTY current directory")) + .transpose()?; + let inherits = attempt.inherits_environment().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "Windows PTY preparation requires portable environment state", + ) + })?; + let environment = build_environment(inherits, attempt.get_envs())?; + + Ok(PreparedWindowsCommand { + application_name, + command_line, + environment, + current_dir, + creation: attempt.windows_spawn_policy(), + }) +} + +#[cfg(test)] +mod tests { + use std::{ + ffi::OsString, + os::windows::ffi::OsStringExt, + path::PathBuf, + sync::{Arc, Mutex}, + }; + + #[cfg(all(feature = "creation-flags", feature = "job-object"))] + use windows::Win32::System::Threading::{ + CREATE_NEW_PROCESS_GROUP, CREATE_NO_WINDOW, CREATE_SUSPENDED, + }; + + use super::*; + #[cfg(feature = "creation-flags")] + use crate::tokio::CreationFlags; + #[cfg(feature = "job-object")] + use crate::tokio::JobObject; + #[cfg(feature = "kill-on-drop")] + use crate::tokio::KillOnDrop; + use crate::tokio::{Command, CommandWrapper, ProviderProduct, SpawnProvider}; + + #[derive(Debug)] + struct PortableWrapper; + + impl CommandWrapper for PortableWrapper { + fn pre_spawn(&mut self, attempt: &mut SpawnAttempt, _command: &Command) -> io::Result<()> { + attempt.arg("from-wrapper"); + Ok(()) + } + } + + #[derive(Debug)] + struct CaptureProvider(Arc>>>); + + impl SpawnProvider for CaptureProvider { + fn validate_attempt(&self, attempt: &SpawnAttempt, _command: &Command) -> io::Result<()> { + *self + .0 + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(prepare(attempt)); + Err(io::Error::other("Windows command model captured")) + } + + fn spawn( + &self, + _attempt: &mut SpawnAttempt, + _command: &Command, + ) -> io::Result { + unreachable!("attempt validation stops before provider allocation") + } + } + + #[derive(Debug)] + struct CaptureModel(CaptureProvider); + + impl CommandWrapper for CaptureModel { + fn spawn_provider(&self) -> Option<&dyn SpawnProvider> { + Some(&self.0) + } + } + + fn prepare_command(command: &mut Command) -> io::Result { + let captured = Arc::new(Mutex::new(None)); + command.wrap(CaptureModel(CaptureProvider(Arc::clone(&captured)))); + let error = command.spawn().unwrap_err(); + assert_eq!(error.to_string(), "Windows command model captured"); + captured + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .take() + .expect("model validation records one preparation result") + } + + fn terminated(value: &str) -> Vec { + value.encode_utf16().chain([0]).collect() + } + + fn environment_block(entries: &[(&str, &str)]) -> PreparedEnvironment { + let mut block = Vec::new(); + for (key, value) in entries { + block.extend(key.encode_utf16()); + block.push(b'=' as u16); + block.extend(value.encode_utf16()); + block.push(0); + } + block.push(0); + PreparedEnvironment::Block(block) + } + + #[test] + fn prepares_ordered_public_builder_intent_and_portable_wrappers() { + let mut command = Command::new("tool"); + command + .arg("two words") + .raw_arg(r#"/D "literal""#) + .args(["tail"]) + .env_clear() + .envs([("Name", "child"), ("Second", "two")]) + .env_remove("gone") + .wrap(PortableWrapper); + + let prepared = prepare_command(&mut command).unwrap(); + + assert_eq!( + prepared.command_line.as_units(), + terminated(r#""tool" "two words" /D "literal" tail from-wrapper"#) + ); + assert_eq!( + prepared.environment, + environment_block(&[("Name", "child"), ("Second", "two")]) + ); + } + + #[test] + fn preserves_wtf16_current_directories() { + let directory = OsString::from_wide(&[b'C' as u16, b':' as u16, b'\\' as u16, 0xd800]); + let mut command = Command::new("tool"); + command.current_dir(PathBuf::from(directory)); + + let prepared = prepare_command(&mut command).unwrap(); + assert_eq!( + prepared.current_dir.unwrap().as_units(), + [b'C' as u16, b':' as u16, b'\\' as u16, 0xd800, 0] + ); + } + + #[test] + fn rejects_current_directory_nuls_before_preparing_the_environment() { + let directory = OsString::from_wide(&[b'C' as u16, b':' as u16, b'\\' as u16, 0]); + let mut command = Command::new("tool"); + command.current_dir(PathBuf::from(directory)); + + let error = prepare_command(&mut command).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert_eq!( + error.to_string(), + "PTY current directory contains an embedded NUL" + ); + } + + #[cfg(all(feature = "creation-flags", feature = "job-object"))] + #[test] + fn derives_job_policy_in_both_wrapper_orders() { + let user_flags = CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW; + for reverse in [false, true] { + let mut command = Command::new("tool"); + if reverse { + command.wrap(JobObject).wrap(CreationFlags(user_flags)); + } else { + command.wrap(CreationFlags(user_flags)).wrap(JobObject); + } + + let policy = prepare_command(&mut command).unwrap().creation; + assert_eq!(policy.user_creation_flags(), user_flags.0); + assert_eq!( + policy.spawn_creation_flags(), + (user_flags | CREATE_SUSPENDED).0 + ); + assert!(!policy.is_explicitly_suspended()); + assert!(policy.has_job_object()); + assert!(policy.is_temporarily_suspended()); + assert!(!policy.kills_on_drop()); + } + } + + #[cfg(all(feature = "creation-flags", feature = "job-object"))] + #[test] + fn preserves_explicit_suspension_for_job_assignment() { + let user_flags = CREATE_NO_WINDOW | CREATE_SUSPENDED; + let mut command = Command::new("tool"); + command.wrap(CreationFlags(user_flags)).wrap(JobObject); + + let policy = prepare_command(&mut command).unwrap().creation; + assert_eq!(policy.user_creation_flags(), user_flags.0); + assert_eq!(policy.spawn_creation_flags(), user_flags.0); + assert!(policy.is_explicitly_suspended()); + assert!(policy.has_job_object()); + assert!(!policy.is_temporarily_suspended()); + } + + #[cfg(feature = "creation-flags")] + #[test] + fn preserves_creation_flags_without_a_job_object() { + use windows::Win32::System::Threading::CREATE_NEW_PROCESS_GROUP; + + let mut command = Command::new("tool"); + command.wrap(CreationFlags(CREATE_NEW_PROCESS_GROUP)); + + let policy = prepare_command(&mut command).unwrap().creation; + assert_eq!(policy.user_creation_flags(), CREATE_NEW_PROCESS_GROUP.0); + assert_eq!(policy.spawn_creation_flags(), CREATE_NEW_PROCESS_GROUP.0); + assert!(!policy.is_explicitly_suspended()); + assert!(!policy.has_job_object()); + assert!(!policy.is_temporarily_suspended()); + } + + #[cfg(feature = "job-object")] + #[test] + fn derives_temporary_suspension_for_a_job_without_creation_flags() { + let mut command = Command::new("tool"); + command.wrap(JobObject); + + let policy = prepare_command(&mut command).unwrap().creation; + assert_eq!(policy.user_creation_flags(), 0); + assert_eq!(policy.spawn_creation_flags(), 0x0000_0004); + assert!(!policy.is_explicitly_suspended()); + assert!(policy.has_job_object()); + assert!(policy.is_temporarily_suspended()); + } + + #[cfg(feature = "kill-on-drop")] + #[test] + fn records_kill_on_drop_without_a_job_object() { + let mut command = Command::new("tool"); + command.wrap(KillOnDrop); + + let policy = prepare_command(&mut command).unwrap().creation; + assert!(!policy.has_job_object()); + assert!(policy.kills_on_drop()); + } + + #[cfg(all(feature = "job-object", feature = "kill-on-drop"))] + #[test] + fn records_kill_on_drop_for_job_policy() { + let mut command = Command::new("tool"); + command.wrap(JobObject).wrap(KillOnDrop); + + let policy = prepare_command(&mut command).unwrap().creation; + assert!(policy.has_job_object()); + assert!(policy.kills_on_drop()); + } +}