From d9a1f9a79c853f7f4678bfe43905ccc7560974bb Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sat, 30 Oct 2021 12:20:50 +0100 Subject: [PATCH] Windows: Resolve Command program without using the current directory --- library/std/src/sys/windows/c.rs | 2 + library/std/src/sys/windows/path.rs | 24 ++- library/std/src/sys/windows/process.rs | 167 ++++++++++++++++--- library/std/src/sys/windows/process/tests.rs | 52 ++++++ 4 files changed, 216 insertions(+), 29 deletions(-) diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index 9dfc8114eb596..50c4547de85f6 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -734,6 +734,7 @@ if #[cfg(not(target_vendor = "uwp"))] { lpSecurityAttributes: LPSECURITY_ATTRIBUTES, ) -> BOOL; pub fn SetThreadStackGuarantee(_size: *mut c_ulong) -> BOOL; + pub fn GetWindowsDirectoryW(lpBuffer: LPWSTR, uSize: UINT) -> UINT; } } } @@ -773,6 +774,7 @@ extern "system" { pub fn LeaveCriticalSection(CriticalSection: *mut CRITICAL_SECTION); pub fn DeleteCriticalSection(CriticalSection: *mut CRITICAL_SECTION); + pub fn GetSystemDirectoryW(lpBuffer: LPWSTR, uSize: UINT) -> UINT; pub fn RemoveDirectoryW(lpPathName: LPCWSTR) -> BOOL; pub fn SetFileAttributesW(lpFileName: LPCWSTR, dwFileAttributes: DWORD) -> BOOL; pub fn SetLastError(dwErrCode: DWORD); diff --git a/library/std/src/sys/windows/path.rs b/library/std/src/sys/windows/path.rs index 460c1eff7788d..8a5a5f56aa8fa 100644 --- a/library/std/src/sys/windows/path.rs +++ b/library/std/src/sys/windows/path.rs @@ -1,9 +1,8 @@ use super::{c, fill_utf16_buf, to_u16s}; -use crate::ffi::OsStr; +use crate::ffi::{OsStr, OsString}; use crate::io; use crate::mem; -use crate::path::Path; -use crate::path::Prefix; +use crate::path::{Path, PathBuf, Prefix}; use crate::ptr; #[cfg(test)] @@ -32,6 +31,25 @@ pub fn is_verbatim_sep(b: u8) -> bool { b == b'\\' } +/// Returns true if `path` looks like a lone filename. +pub(crate) fn is_file_name(path: &OsStr) -> bool { + !path.bytes().iter().copied().any(is_sep_byte) +} +pub(crate) fn has_trailing_slash(path: &OsStr) -> bool { + let is_verbatim = path.bytes().starts_with(br"\\?\"); + let is_separator = if is_verbatim { is_verbatim_sep } else { is_sep_byte }; + if let Some(&c) = path.bytes().last() { is_separator(c) } else { false } +} + +/// Appends a suffix to a path. +/// +/// Can be used to append an extension without removing an existing extension. +pub(crate) fn append_suffix(path: PathBuf, suffix: &OsStr) -> PathBuf { + let mut path = OsString::from(path); + path.push(suffix); + path.into() +} + pub fn parse_prefix(path: &OsStr) -> Option> { use Prefix::{DeviceNS, Disk, Verbatim, VerbatimDisk, VerbatimUNC, UNC}; diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index ccff90629a371..66b210ce1bfb3 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -7,24 +7,24 @@ use crate::cmp; use crate::collections::BTreeMap; use crate::convert::{TryFrom, TryInto}; use crate::env; -use crate::env::split_paths; +use crate::env::consts::{EXE_EXTENSION, EXE_SUFFIX}; use crate::ffi::{OsStr, OsString}; use crate::fmt; -use crate::fs; use crate::io::{self, Error, ErrorKind}; use crate::mem; use crate::num::NonZeroI32; -use crate::os::windows::ffi::OsStrExt; +use crate::os::windows::ffi::{OsStrExt, OsStringExt}; use crate::os::windows::io::{AsRawHandle, FromRawHandle, IntoRawHandle}; -use crate::path::Path; +use crate::path::{Path, PathBuf}; use crate::ptr; use crate::sys::c; use crate::sys::c::NonZeroDWORD; -use crate::sys::cvt; use crate::sys::fs::{File, OpenOptions}; use crate::sys::handle::Handle; +use crate::sys::path; use crate::sys::pipe::{self, AnonPipe}; use crate::sys::stdio; +use crate::sys::{cvt, to_u16s}; use crate::sys_common::mutex::StaticMutex; use crate::sys_common::process::{CommandEnv, CommandEnvs}; use crate::sys_common::{AsInner, IntoInner}; @@ -258,31 +258,19 @@ impl Command { needs_stdin: bool, ) -> io::Result<(Process, StdioPipes)> { let maybe_env = self.env.capture_if_changed(); - // To have the spawning semantics of unix/windows stay the same, we need - // to read the *child's* PATH if one is provided. See #15149 for more - // details. - let program = maybe_env.as_ref().and_then(|env| { - if let Some(v) = env.get(&EnvKey::new("PATH")) { - // Split the value and test each path to see if the - // program exists. - for path in split_paths(&v) { - let path = path - .join(self.program.to_str().unwrap()) - .with_extension(env::consts::EXE_EXTENSION); - if fs::metadata(&path).is_ok() { - return Some(path.into_os_string()); - } - } - } - None - }); let mut si = zeroed_startupinfo(); si.cb = mem::size_of::() as c::DWORD; si.dwFlags = c::STARTF_USESTDHANDLES; - let program = program.as_ref().unwrap_or(&self.program); - let mut cmd_str = make_command_line(program, &self.args, self.force_quotes_enabled)?; + let child_paths = if let Some(env) = maybe_env.as_ref() { + env.get(&EnvKey::new("PATH")).map(|s| s.as_os_str()) + } else { + None + }; + let program = resolve_exe(&self.program, child_paths)?; + let mut cmd_str = + make_command_line(program.as_os_str(), &self.args, self.force_quotes_enabled)?; cmd_str.push(0); // add null terminator // stolen from the libuv code. @@ -321,9 +309,10 @@ impl Command { si.hStdOutput = stdout.as_raw_handle(); si.hStdError = stderr.as_raw_handle(); + let program = to_u16s(&program)?; unsafe { cvt(c::CreateProcessW( - ptr::null(), + program.as_ptr(), cmd_str.as_mut_ptr(), ptr::null_mut(), ptr::null_mut(), @@ -361,6 +350,132 @@ impl fmt::Debug for Command { } } +// Resolve `exe_path` to the executable name. +// +// * If the path is simply a file name then use the paths given by `search_paths` to find the executable. +// * Otherwise use the `exe_path` as given. +// +// This function may also append `.exe` to the name. The rationale for doing so is as follows: +// +// It is a very strong convention that Windows executables have the `exe` extension. +// In Rust, it is common to omit this extension. +// Therefore this functions first assumes `.exe` was intended. +// It falls back to the plain file name if a full path is given and the extension is omitted +// or if only a file name is given and it already contains an extension. +fn resolve_exe<'a>(exe_path: &'a OsStr, child_paths: Option<&OsStr>) -> io::Result { + // Early return if there is no filename. + if exe_path.is_empty() || path::has_trailing_slash(exe_path) { + return Err(io::Error::new_const( + io::ErrorKind::InvalidInput, + &"program path has no file name", + )); + } + // Test if the file name has the `exe` extension. + // This does a case-insensitive `ends_with`. + let has_exe_suffix = if exe_path.len() >= EXE_SUFFIX.len() { + exe_path.bytes()[exe_path.len() - EXE_SUFFIX.len()..] + .eq_ignore_ascii_case(EXE_SUFFIX.as_bytes()) + } else { + false + }; + + // If `exe_path` is an absolute path or a sub-path then don't search `PATH` for it. + if !path::is_file_name(exe_path) { + if has_exe_suffix { + // The application name is a path to a `.exe` file. + // Let `CreateProcessW` figure out if it exists or not. + return Ok(exe_path.into()); + } + let mut path = PathBuf::from(exe_path); + + // Append `.exe` if not already there. + path = path::append_suffix(path, EXE_SUFFIX.as_ref()); + if path.try_exists().unwrap_or(false) { + return Ok(path); + } else { + // It's ok to use `set_extension` here because the intent is to + // remove the extension that was just added. + path.set_extension(""); + return Ok(path); + } + } else { + ensure_no_nuls(exe_path)?; + // From the `CreateProcessW` docs: + // > If the file name does not contain an extension, .exe is appended. + // Note that this rule only applies when searching paths. + let has_extension = exe_path.bytes().contains(&b'.'); + + // Search the directories given by `search_paths`. + let result = search_paths(child_paths, |mut path| { + path.push(&exe_path); + if !has_extension { + path.set_extension(EXE_EXTENSION); + } + if let Ok(true) = path.try_exists() { Some(path) } else { None } + }); + if let Some(path) = result { + return Ok(path); + } + } + // If we get here then the executable cannot be found. + Err(io::Error::new_const(io::ErrorKind::NotFound, &"program not found")) +} + +// Calls `f` for every path that should be used to find an executable. +// Returns once `f` returns the path to an executable or all paths have been searched. +fn search_paths(child_paths: Option<&OsStr>, mut f: F) -> Option +where + F: FnMut(PathBuf) -> Option, +{ + // 1. Child paths + // This is for consistency with Rust's historic behaviour. + if let Some(paths) = child_paths { + for path in env::split_paths(paths).filter(|p| !p.as_os_str().is_empty()) { + if let Some(path) = f(path) { + return Some(path); + } + } + } + + // 2. Application path + if let Ok(mut app_path) = env::current_exe() { + app_path.pop(); + if let Some(path) = f(app_path) { + return Some(path); + } + } + + // 3 & 4. System paths + // SAFETY: This uses `fill_utf16_buf` to safely call the OS functions. + unsafe { + if let Ok(Some(path)) = super::fill_utf16_buf( + |buf, size| c::GetSystemDirectoryW(buf, size), + |buf| f(PathBuf::from(OsString::from_wide(buf))), + ) { + return Some(path); + } + #[cfg(not(target_vendor = "uwp"))] + { + if let Ok(Some(path)) = super::fill_utf16_buf( + |buf, size| c::GetWindowsDirectoryW(buf, size), + |buf| f(PathBuf::from(OsString::from_wide(buf))), + ) { + return Some(path); + } + } + } + + // 5. Parent paths + if let Some(parent_paths) = env::var_os("PATH") { + for path in env::split_paths(&parent_paths).filter(|p| !p.as_os_str().is_empty()) { + if let Some(path) = f(path) { + return Some(path); + } + } + } + None +} + impl Stdio { fn to_handle(&self, stdio_id: c::DWORD, pipe: &mut Option) -> io::Result { match *self { diff --git a/library/std/src/sys/windows/process/tests.rs b/library/std/src/sys/windows/process/tests.rs index 3b65856dcaca6..6c862edc2370a 100644 --- a/library/std/src/sys/windows/process/tests.rs +++ b/library/std/src/sys/windows/process/tests.rs @@ -128,3 +128,55 @@ fn windows_env_unicode_case() { } } } + +// UWP applications run in a restricted environment which means this test may not work. +#[cfg(not(target_vendor = "uwp"))] +#[test] +fn windows_exe_resolver() { + use super::resolve_exe; + use crate::io; + + // Test a full path, with and without the `exe` extension. + let mut current_exe = env::current_exe().unwrap(); + assert!(resolve_exe(current_exe.as_ref(), None).is_ok()); + current_exe.set_extension(""); + assert!(resolve_exe(current_exe.as_ref(), None).is_ok()); + + // Test lone file names. + assert!(resolve_exe(OsStr::new("cmd"), None).is_ok()); + assert!(resolve_exe(OsStr::new("cmd.exe"), None).is_ok()); + assert!(resolve_exe(OsStr::new("cmd.EXE"), None).is_ok()); + assert!(resolve_exe(OsStr::new("fc"), None).is_ok()); + + // Invalid file names should return InvalidInput. + assert_eq!(resolve_exe(OsStr::new(""), None).unwrap_err().kind(), io::ErrorKind::InvalidInput); + assert_eq!( + resolve_exe(OsStr::new("\0"), None).unwrap_err().kind(), + io::ErrorKind::InvalidInput + ); + // Trailing slash, therefore there's no file name component. + assert_eq!( + resolve_exe(OsStr::new(r"C:\Path\to\"), None).unwrap_err().kind(), + io::ErrorKind::InvalidInput + ); + + /* + Some of the following tests may need to be changed if you are deliberately + changing the behaviour of `resolve_exe`. + */ + + let paths = env::var_os("PATH").unwrap(); + env::set_var("PATH", ""); + + assert_eq!(resolve_exe(OsStr::new("rustc"), None).unwrap_err().kind(), io::ErrorKind::NotFound); + + let child_paths = Some(paths.as_os_str()); + assert!(resolve_exe(OsStr::new("rustc"), child_paths).is_ok()); + + // The resolver looks in system directories even when `PATH` is empty. + assert!(resolve_exe(OsStr::new("cmd.exe"), None).is_ok()); + + // The application's directory is also searched. + let current_exe = env::current_exe().unwrap(); + assert!(resolve_exe(current_exe.file_name().unwrap().as_ref(), None).is_ok()); +}