From 4145877731e9f38a4640d2dfeed7b123d81d061a Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Mon, 3 Jan 2022 12:55:42 +0000 Subject: [PATCH] Explicitly pass `PATH` to the Windows exe resolver --- library/std/src/sys/windows/process.rs | 31 ++++++++++------ library/std/src/sys/windows/process/tests.rs | 39 +++++++++----------- 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index e84dfbce4a754..5ad570427978e 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -268,7 +268,7 @@ impl Command { } else { None }; - let program = resolve_exe(&self.program, child_paths)?; + let program = resolve_exe(&self.program, || env::var_os("PATH"), 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 @@ -362,7 +362,11 @@ impl fmt::Debug for Command { // 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 { +fn resolve_exe<'a>( + exe_path: &'a OsStr, + parent_paths: impl FnOnce() -> Option, + 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( @@ -406,7 +410,7 @@ fn resolve_exe<'a>(exe_path: &'a OsStr, child_paths: Option<&OsStr>) -> io::Resu let has_extension = exe_path.bytes().contains(&b'.'); // Search the directories given by `search_paths`. - let result = search_paths(child_paths, |mut path| { + let result = search_paths(parent_paths, child_paths, |mut path| { path.push(&exe_path); if !has_extension { path.set_extension(EXE_EXTENSION); @@ -423,15 +427,20 @@ fn resolve_exe<'a>(exe_path: &'a OsStr, child_paths: Option<&OsStr>) -> io::Resu // 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 +fn search_paths( + parent_paths: Paths, + child_paths: Option<&OsStr>, + mut exists: Exists, +) -> Option where - F: FnMut(PathBuf) -> Option, + Paths: FnOnce() -> Option, + Exists: 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) { + if let Some(path) = exists(path) { return Some(path); } } @@ -440,7 +449,7 @@ where // 2. Application path if let Ok(mut app_path) = env::current_exe() { app_path.pop(); - if let Some(path) = f(app_path) { + if let Some(path) = exists(app_path) { return Some(path); } } @@ -450,7 +459,7 @@ where unsafe { if let Ok(Some(path)) = super::fill_utf16_buf( |buf, size| c::GetSystemDirectoryW(buf, size), - |buf| f(PathBuf::from(OsString::from_wide(buf))), + |buf| exists(PathBuf::from(OsString::from_wide(buf))), ) { return Some(path); } @@ -458,7 +467,7 @@ where { if let Ok(Some(path)) = super::fill_utf16_buf( |buf, size| c::GetWindowsDirectoryW(buf, size), - |buf| f(PathBuf::from(OsString::from_wide(buf))), + |buf| exists(PathBuf::from(OsString::from_wide(buf))), ) { return Some(path); } @@ -466,9 +475,9 @@ where } // 5. Parent paths - if let Some(parent_paths) = env::var_os("PATH") { + if let Some(parent_paths) = parent_paths() { for path in env::split_paths(&parent_paths).filter(|p| !p.as_os_str().is_empty()) { - if let Some(path) = f(path) { + if let Some(path) = exists(path) { return Some(path); } } diff --git a/library/std/src/sys/windows/process/tests.rs b/library/std/src/sys/windows/process/tests.rs index 6159a679c0e69..f1221767af30e 100644 --- a/library/std/src/sys/windows/process/tests.rs +++ b/library/std/src/sys/windows/process/tests.rs @@ -136,51 +136,46 @@ fn windows_exe_resolver() { use super::resolve_exe; use crate::io; + let env_paths = || env::var_os("PATH"); + // 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()); + assert!(resolve_exe(current_exe.as_ref(), env_paths, None).is_ok()); current_exe.set_extension(""); - assert!(resolve_exe(current_exe.as_ref(), None).is_ok()); + assert!(resolve_exe(current_exe.as_ref(), env_paths, 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()); + assert!(resolve_exe(OsStr::new("cmd"), env_paths, None).is_ok()); + assert!(resolve_exe(OsStr::new("cmd.exe"), env_paths, None).is_ok()); + assert!(resolve_exe(OsStr::new("cmd.EXE"), env_paths, None).is_ok()); + assert!(resolve_exe(OsStr::new("fc"), env_paths, 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(), + resolve_exe(OsStr::new(""), env_paths, None).unwrap_err().kind(), + io::ErrorKind::InvalidInput + ); + assert_eq!( + resolve_exe(OsStr::new("\0"), env_paths, 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(), + resolve_exe(OsStr::new(r"C:\Path\to\"), env_paths, None).unwrap_err().kind(), io::ErrorKind::InvalidInput ); - /* FIXME: fix and re-enable these tests before making changes to the resolver. - /* 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()); + let empty_paths = || None; // The resolver looks in system directories even when `PATH` is empty. - assert!(resolve_exe(OsStr::new("cmd.exe"), None).is_ok()); + assert!(resolve_exe(OsStr::new("cmd.exe"), empty_paths, 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()); - - */ + assert!(resolve_exe(current_exe.file_name().unwrap().as_ref(), empty_paths, None).is_ok()); }