Skip to content

Try not to use verbatim paths in Command::current_dir #138869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions library/std/src/sys/path/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +354 to +363
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verbatim paths can contain special names like COM while regular paths cannot right? That may be worth mentioning and adding a test for.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The COM1, CON, AUX, etc cases is the same as the NUL case mentioned and tested. It can occur in paths but may be changed to e.g. \\.\COM1 for non-verbatim paths. I didn't add a test for the COM1 case because it is actually allowed on recent versions of Windows. Only the relative path COM1 would be converted to \\.\COM1 but that's not absolute in any case.

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
}
}
12 changes: 12 additions & 0 deletions library/std/src/sys/path/windows/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
32 changes: 28 additions & 4 deletions library/std/src/sys/process/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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())),
}
Expand Down
36 changes: 36 additions & 0 deletions tests/ui/process/win-command-curdir-no-verbatim.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Test that windows verbatim paths in `Command::current_dir` are converted to
// non-verbatim paths before passing to the subprocess.

//@ run-pass
//@ only-windows
//@ needs-subprocess

use std::env;
use std::process::Command;

fn main() {
if env::args().skip(1).any(|s| s == "--child") {
child();
} else {
parent();
}
}

fn parent() {
let exe = env::current_exe().unwrap();
let dir = env::current_dir().unwrap();
let status = Command::new(&exe)
.arg("--child")
.current_dir(dir.canonicalize().unwrap())
.spawn()
.unwrap()
.wait()
.unwrap();
assert_eq!(status.code(), Some(0));
}

fn child() {
let current_dir = env::current_dir().unwrap();
let current_dir = current_dir.as_os_str().as_encoded_bytes();
assert!(!current_dir.starts_with(br"\\?\"));
}
Loading