1
Fork 0

Auto merge of #91182 - ChrisDenton:command-broken-symlink, r=m-ou-se

Maintain broken symlink behaviour for the Windows exe resolver

When the resolver was updated to remove the current directory from the search path (see #87704), care was take to avoid unintentional changes that hadn't been discussed. However, I missed the broken symlink behaviour. This PR fixes that.

**Edit** This turned out to be more important than I first realised. There are some types of application stubs that will redirect to the actual process when run using `CreateProcessW`, but due to the way they're implemented they cannot be opened normally using a `File::open` that follows reparse points. So this doesn't work with our current `exists` and `try_exists` methods.

Fixes #91177
This commit is contained in:
bors 2022-02-16 22:11:22 +00:00
commit 2ff7ea4de2
3 changed files with 30 additions and 2 deletions

View file

@ -83,6 +83,7 @@ pub const CSTR_GREATER_THAN: c_int = 3;
pub const FILE_ATTRIBUTE_READONLY: DWORD = 0x1; pub const FILE_ATTRIBUTE_READONLY: DWORD = 0x1;
pub const FILE_ATTRIBUTE_DIRECTORY: DWORD = 0x10; pub const FILE_ATTRIBUTE_DIRECTORY: DWORD = 0x10;
pub const FILE_ATTRIBUTE_REPARSE_POINT: DWORD = 0x400; pub const FILE_ATTRIBUTE_REPARSE_POINT: DWORD = 0x400;
pub const INVALID_FILE_ATTRIBUTES: DWORD = DWORD::MAX;
pub const FILE_SHARE_DELETE: DWORD = 0x4; pub const FILE_SHARE_DELETE: DWORD = 0x4;
pub const FILE_SHARE_READ: DWORD = 0x1; pub const FILE_SHARE_READ: DWORD = 0x1;
@ -1075,6 +1076,7 @@ extern "system" {
lpBuffer: LPWSTR, lpBuffer: LPWSTR,
lpFilePart: *mut LPWSTR, lpFilePart: *mut LPWSTR,
) -> DWORD; ) -> DWORD;
pub fn GetFileAttributesW(lpFileName: LPCWSTR) -> DWORD;
} }
#[link(name = "ws2_32")] #[link(name = "ws2_32")]

View file

@ -394,7 +394,7 @@ fn resolve_exe<'a>(
// Append `.exe` if not already there. // Append `.exe` if not already there.
path = path::append_suffix(path, EXE_SUFFIX.as_ref()); path = path::append_suffix(path, EXE_SUFFIX.as_ref());
if path.try_exists().unwrap_or(false) { if program_exists(&path) {
return Ok(path); return Ok(path);
} else { } else {
// It's ok to use `set_extension` here because the intent is to // It's ok to use `set_extension` here because the intent is to
@ -415,7 +415,7 @@ fn resolve_exe<'a>(
if !has_extension { if !has_extension {
path.set_extension(EXE_EXTENSION); path.set_extension(EXE_EXTENSION);
} }
if let Ok(true) = path.try_exists() { Some(path) } else { None } if program_exists(&path) { Some(path) } else { None }
}); });
if let Some(path) = result { if let Some(path) = result {
return Ok(path); return Ok(path);
@ -485,6 +485,21 @@ where
None None
} }
/// Check if a file exists without following symlinks.
fn program_exists(path: &Path) -> bool {
unsafe {
to_u16s(path)
.map(|path| {
// Getting attributes using `GetFileAttributesW` does not follow symlinks
// and it will almost always be successful if the link exists.
// There are some exceptions for special system files (e.g. the pagefile)
// but these are not executable.
c::GetFileAttributesW(path.as_ptr()) != c::INVALID_FILE_ATTRIBUTES
})
.unwrap_or(false)
}
}
impl Stdio { impl Stdio {
fn to_handle(&self, stdio_id: c::DWORD, pipe: &mut Option<AnonPipe>) -> io::Result<Handle> { fn to_handle(&self, stdio_id: c::DWORD, pipe: &mut Option<AnonPipe>) -> io::Result<Handle> {
match *self { match *self {

View file

@ -135,6 +135,8 @@ fn windows_env_unicode_case() {
fn windows_exe_resolver() { fn windows_exe_resolver() {
use super::resolve_exe; use super::resolve_exe;
use crate::io; use crate::io;
use crate::sys::fs::symlink;
use crate::sys_common::io::test::tmpdir;
let env_paths = || env::var_os("PATH"); let env_paths = || env::var_os("PATH");
@ -178,4 +180,13 @@ fn windows_exe_resolver() {
// The application's directory is also searched. // The application's directory is also searched.
let current_exe = env::current_exe().unwrap(); let current_exe = env::current_exe().unwrap();
assert!(resolve_exe(current_exe.file_name().unwrap().as_ref(), empty_paths, None).is_ok()); assert!(resolve_exe(current_exe.file_name().unwrap().as_ref(), empty_paths, None).is_ok());
// Create a temporary path and add a broken symlink.
let temp = tmpdir();
let mut exe_path = temp.path().to_owned();
exe_path.push("exists.exe");
symlink("<DOES NOT EXIST>".as_ref(), &exe_path).unwrap();
// A broken symlink should still be resolved.
assert!(resolve_exe(OsStr::new("exists.exe"), empty_paths, Some(temp.path().as_ref())).is_ok());
} }