Windows: Resolve process::Command program without using the current directory

library/std/src/process/tests.rs

@@ -297,23 +297,23 @@ fn test_interior_nul_in_progname_is_error() {
 
 #[test]
 fn test_interior_nul_in_arg_is_error() {
-    match Command::new("echo").arg("has-some-\0\0s-inside").spawn() {
+    match Command::new("rustc").arg("has-some-\0\0s-inside").spawn() {
         Err(e) => assert_eq!(e.kind(), ErrorKind::InvalidInput),
         Ok(_) => panic!(),
     }
 }
 
 #[test]
 fn test_interior_nul_in_args_is_error() {
-    match Command::new("echo").args(&["has-some-\0\0s-inside"]).spawn() {
+    match Command::new("rustc").args(&["has-some-\0\0s-inside"]).spawn() {
         Err(e) => assert_eq!(e.kind(), ErrorKind::InvalidInput),
         Ok(_) => panic!(),
     }
 }
 
 #[test]
 fn test_interior_nul_in_current_dir_is_error() {
-    match Command::new("echo").current_dir("has-some-\0\0s-inside").spawn() {
+    match Command::new("rustc").current_dir("has-some-\0\0s-inside").spawn() {
         Err(e) => assert_eq!(e.kind(), ErrorKind::InvalidInput),
         Ok(_) => panic!(),
     }

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);

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<Prefix<'_>> {
     use Prefix::{DeviceNS, Disk, Verbatim, VerbatimDisk, VerbatimUNC, UNC};
 

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::<c::STARTUPINFO>() 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<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(
+            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<F>(child_paths: Option<&OsStr>, mut f: F) -> Option<PathBuf>
+where
+    F: 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) {
+                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<AnonPipe>) -> io::Result<Handle> {
         match *self {

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