Avoid verbatim paths in Command::current_dir

If possible, we should try not to use verbatim paths in Command::current_dir. It might work but it might also break code (including some Windows APIs) that assume the current directory isn't verbatim.
This commit is contained in:
Chris Denton 2025-03-24 18:57:07 +00:00
parent a6e4d03086
commit edfc747225
No known key found for this signature in database
GPG key ID: 713472F2F45627DE
3 changed files with 83 additions and 4 deletions

View file

@ -350,3 +350,46 @@ pub(crate) fn absolute(path: &Path) -> io::Result<PathBuf> {
pub(crate) fn is_absolute(path: &Path) -> bool {
path.has_root() && path.prefix().is_some()
}
/// Test that the path is absolute, fully qualified and unchanged when processed by the Windows API.
///
/// For example:
///
/// - `C:\path\to\file` will return true.
/// - `C:\path\to\nul` returns false because the Windows API will convert it to \\.\NUL
/// - `C:\path\to\..\file` returns false because it will be resolved to `C:\path\file`.
///
/// This is a useful property because it means the path can be converted from and to and verbatim
/// path just by changing the prefix.
pub(crate) fn is_absolute_exact(path: &[u16]) -> bool {
// This is implemented by checking that passing the path through
// GetFullPathNameW does not change the path in any way.
// Windows paths are limited to i16::MAX length
// though the API here accepts a u32 for the length.
if path.is_empty() || path.len() > u32::MAX as usize || path.last() != Some(&0) {
return false;
}
// The path returned by `GetFullPathNameW` must be the same length as the
// given path, otherwise they're not equal.
let buffer_len = path.len();
let mut new_path = Vec::with_capacity(buffer_len);
let result = unsafe {
c::GetFullPathNameW(
path.as_ptr(),
new_path.capacity() as u32,
new_path.as_mut_ptr(),
crate::ptr::null_mut(),
)
};
// Note: if non-zero, the returned result is the length of the buffer without the null termination
if result == 0 || result as usize != buffer_len - 1 {
false
} else {
// SAFETY: `GetFullPathNameW` initialized `result` bytes and does not exceed `nBufferLength - 1` (capacity).
unsafe {
new_path.set_len((result as usize) + 1);
}
path == &new_path
}
}

View file

@ -135,3 +135,15 @@ fn broken_unc_path() {
assert_eq!(components.next(), Some(Component::Normal("foo".as_ref())));
assert_eq!(components.next(), Some(Component::Normal("bar".as_ref())));
}
#[test]
fn test_is_absolute_exact() {
use crate::sys::pal::api::wide_str;
// These paths can be made verbatim by only changing their prefix.
assert!(is_absolute_exact(wide_str!(r"C:\path\to\file")));
assert!(is_absolute_exact(wide_str!(r"\\server\share\path\to\file")));
// These paths change more substantially
assert!(!is_absolute_exact(wide_str!(r"C:\path\to\..\file")));
assert!(!is_absolute_exact(wide_str!(r"\\server\share\path\to\..\file")));
assert!(!is_absolute_exact(wide_str!(r"C:\path\to\NUL"))); // Converts to \\.\NUL
}

View file

@ -19,7 +19,7 @@ use crate::sys::args::{self, Arg};
use crate::sys::c::{self, EXIT_FAILURE, EXIT_SUCCESS};
use crate::sys::fs::{File, OpenOptions};
use crate::sys::handle::Handle;
use crate::sys::pal::api::{self, WinError};
use crate::sys::pal::api::{self, WinError, utf16};
use crate::sys::pal::{ensure_no_nuls, fill_utf16_buf};
use crate::sys::pipe::{self, AnonPipe};
use crate::sys::{cvt, path, stdio};
@ -880,9 +880,33 @@ fn make_envp(maybe_env: Option<BTreeMap<EnvKey, OsString>>) -> io::Result<(*mut
fn make_dirp(d: Option<&OsString>) -> io::Result<(*const u16, Vec<u16>)> {
match d {
Some(dir) => {
let mut dir_str: Vec<u16> = ensure_no_nuls(dir)?.encode_wide().collect();
dir_str.push(0);
Ok((dir_str.as_ptr(), dir_str))
let mut dir_str: Vec<u16> = ensure_no_nuls(dir)?.encode_wide().chain([0]).collect();
// Try to remove the `\\?\` prefix, if any.
// This is necessary because the current directory does not support verbatim paths.
// However. this can only be done if it doesn't change how the path will be resolved.
let ptr = if dir_str.starts_with(utf16!(r"\\?\UNC")) {
// Turn the `C` in `UNC` into a `\` so we can then use `\\rest\of\path`.
let start = r"\\?\UN".len();
dir_str[start] = b'\\' as u16;
if path::is_absolute_exact(&dir_str[start..]) {
dir_str[start..].as_ptr()
} else {
// Revert the above change.
dir_str[start] = b'C' as u16;
dir_str.as_ptr()
}
} else if dir_str.starts_with(utf16!(r"\\?\")) {
// Strip the leading `\\?\`
let start = r"\\?\".len();
if path::is_absolute_exact(&dir_str[start..]) {
dir_str[start..].as_ptr()
} else {
dir_str.as_ptr()
}
} else {
dir_str.as_ptr()
};
Ok((ptr, dir_str))
}
None => Ok((ptr::null(), Vec::new())),
}