Skip to content

Commit

Permalink
Auto merge of #91182 - ChrisDenton:command-broken-symlink, r=m-ou-se
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Feb 16, 2022
2 parents 75d9a0a + 9a7a8b9 commit 2ff7ea4
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 2 deletions.
2 changes: 2 additions & 0 deletions library/std/src/sys/windows/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ pub const CSTR_GREATER_THAN: c_int = 3;
pub const FILE_ATTRIBUTE_READONLY: DWORD = 0x1;
pub const FILE_ATTRIBUTE_DIRECTORY: DWORD = 0x10;
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_READ: DWORD = 0x1;
Expand Down Expand Up @@ -1075,6 +1076,7 @@ extern "system" {
lpBuffer: LPWSTR,
lpFilePart: *mut LPWSTR,
) -> DWORD;
pub fn GetFileAttributesW(lpFileName: LPCWSTR) -> DWORD;
}

#[link(name = "ws2_32")]
Expand Down
19 changes: 17 additions & 2 deletions library/std/src/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ fn resolve_exe<'a>(

// Append `.exe` if not already there.
path = path::append_suffix(path, EXE_SUFFIX.as_ref());
if path.try_exists().unwrap_or(false) {
if program_exists(&path) {
return Ok(path);
} else {
// It's ok to use `set_extension` here because the intent is to
Expand All @@ -415,7 +415,7 @@ fn resolve_exe<'a>(
if !has_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 {
return Ok(path);
Expand Down Expand Up @@ -485,6 +485,21 @@ where
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 {
fn to_handle(&self, stdio_id: c::DWORD, pipe: &mut Option<AnonPipe>) -> io::Result<Handle> {
match *self {
Expand Down
11 changes: 11 additions & 0 deletions library/std/src/sys/windows/process/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ fn windows_env_unicode_case() {
fn windows_exe_resolver() {
use super::resolve_exe;
use crate::io;
use crate::sys::fs::symlink;
use crate::sys_common::io::test::tmpdir;

let env_paths = || env::var_os("PATH");

Expand Down Expand Up @@ -178,4 +180,13 @@ fn windows_exe_resolver() {
// The application's directory is also searched.
let current_exe = env::current_exe().unwrap();
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());
}

0 comments on commit 2ff7ea4

Please sign in to comment.