Explicitly pass PATH to the Windows exe resolver

This commit is contained in:
Chris Denton 2022-01-03 12:55:42 +00:00
parent d367c349ef
commit 4145877731
No known key found for this signature in database
GPG key ID: 713472F2F45627DE
2 changed files with 37 additions and 33 deletions

View file

@ -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<PathBuf> {
fn resolve_exe<'a>(
exe_path: &'a OsStr,
parent_paths: impl FnOnce() -> Option<OsString>,
child_paths: Option<&OsStr>,
) -> io::Result<PathBuf> {
// 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<F>(child_paths: Option<&OsStr>, mut f: F) -> Option<PathBuf>
fn search_paths<Paths, Exists>(
parent_paths: Paths,
child_paths: Option<&OsStr>,
mut exists: Exists,
) -> Option<PathBuf>
where
F: FnMut(PathBuf) -> Option<PathBuf>,
Paths: FnOnce() -> Option<OsString>,
Exists: FnMut(PathBuf) -> Option<PathBuf>,
{
// 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);
}
}

View file

@ -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());
}