Skip to content

Commit 45128be

Browse files
authored
Unrolled build for rust-lang#138869
Rollup merge of rust-lang#138869 - ChrisDenton:command-curdir, r=tgross35 Try not to use 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 in the subprocess that assume the current directory isn't verbatim (including Windows APIs). cc ``@ehuss`` Side note: we now have a lot of ad-hoc fixes like this spread about the place. It'd be good to make a proper `WindowsPath` type that handles all this in one place. But that's a bigger job for another PR.
2 parents f06e5c1 + edfc747 commit 45128be

File tree

4 files changed

+119
-4
lines changed

4 files changed

+119
-4
lines changed

library/std/src/sys/path/windows.rs

+43
Original file line numberDiff line numberDiff line change
@@ -350,3 +350,46 @@ pub(crate) fn absolute(path: &Path) -> io::Result<PathBuf> {
350350
pub(crate) fn is_absolute(path: &Path) -> bool {
351351
path.has_root() && path.prefix().is_some()
352352
}
353+
354+
/// Test that the path is absolute, fully qualified and unchanged when processed by the Windows API.
355+
///
356+
/// For example:
357+
///
358+
/// - `C:\path\to\file` will return true.
359+
/// - `C:\path\to\nul` returns false because the Windows API will convert it to \\.\NUL
360+
/// - `C:\path\to\..\file` returns false because it will be resolved to `C:\path\file`.
361+
///
362+
/// This is a useful property because it means the path can be converted from and to and verbatim
363+
/// path just by changing the prefix.
364+
pub(crate) fn is_absolute_exact(path: &[u16]) -> bool {
365+
// This is implemented by checking that passing the path through
366+
// GetFullPathNameW does not change the path in any way.
367+
368+
// Windows paths are limited to i16::MAX length
369+
// though the API here accepts a u32 for the length.
370+
if path.is_empty() || path.len() > u32::MAX as usize || path.last() != Some(&0) {
371+
return false;
372+
}
373+
// The path returned by `GetFullPathNameW` must be the same length as the
374+
// given path, otherwise they're not equal.
375+
let buffer_len = path.len();
376+
let mut new_path = Vec::with_capacity(buffer_len);
377+
let result = unsafe {
378+
c::GetFullPathNameW(
379+
path.as_ptr(),
380+
new_path.capacity() as u32,
381+
new_path.as_mut_ptr(),
382+
crate::ptr::null_mut(),
383+
)
384+
};
385+
// Note: if non-zero, the returned result is the length of the buffer without the null termination
386+
if result == 0 || result as usize != buffer_len - 1 {
387+
false
388+
} else {
389+
// SAFETY: `GetFullPathNameW` initialized `result` bytes and does not exceed `nBufferLength - 1` (capacity).
390+
unsafe {
391+
new_path.set_len((result as usize) + 1);
392+
}
393+
path == &new_path
394+
}
395+
}

library/std/src/sys/path/windows/tests.rs

+12
Original file line numberDiff line numberDiff line change
@@ -135,3 +135,15 @@ fn broken_unc_path() {
135135
assert_eq!(components.next(), Some(Component::Normal("foo".as_ref())));
136136
assert_eq!(components.next(), Some(Component::Normal("bar".as_ref())));
137137
}
138+
139+
#[test]
140+
fn test_is_absolute_exact() {
141+
use crate::sys::pal::api::wide_str;
142+
// These paths can be made verbatim by only changing their prefix.
143+
assert!(is_absolute_exact(wide_str!(r"C:\path\to\file")));
144+
assert!(is_absolute_exact(wide_str!(r"\\server\share\path\to\file")));
145+
// These paths change more substantially
146+
assert!(!is_absolute_exact(wide_str!(r"C:\path\to\..\file")));
147+
assert!(!is_absolute_exact(wide_str!(r"\\server\share\path\to\..\file")));
148+
assert!(!is_absolute_exact(wide_str!(r"C:\path\to\NUL"))); // Converts to \\.\NUL
149+
}

library/std/src/sys/process/windows.rs

+28-4
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::sys::args::{self, Arg};
1919
use crate::sys::c::{self, EXIT_FAILURE, EXIT_SUCCESS};
2020
use crate::sys::fs::{File, OpenOptions};
2121
use crate::sys::handle::Handle;
22-
use crate::sys::pal::api::{self, WinError};
22+
use crate::sys::pal::api::{self, WinError, utf16};
2323
use crate::sys::pal::{ensure_no_nuls, fill_utf16_buf};
2424
use crate::sys::pipe::{self, AnonPipe};
2525
use crate::sys::{cvt, path, stdio};
@@ -880,9 +880,33 @@ fn make_envp(maybe_env: Option<BTreeMap<EnvKey, OsString>>) -> io::Result<(*mut
880880
fn make_dirp(d: Option<&OsString>) -> io::Result<(*const u16, Vec<u16>)> {
881881
match d {
882882
Some(dir) => {
883-
let mut dir_str: Vec<u16> = ensure_no_nuls(dir)?.encode_wide().collect();
884-
dir_str.push(0);
885-
Ok((dir_str.as_ptr(), dir_str))
883+
let mut dir_str: Vec<u16> = ensure_no_nuls(dir)?.encode_wide().chain([0]).collect();
884+
// Try to remove the `\\?\` prefix, if any.
885+
// This is necessary because the current directory does not support verbatim paths.
886+
// However. this can only be done if it doesn't change how the path will be resolved.
887+
let ptr = if dir_str.starts_with(utf16!(r"\\?\UNC")) {
888+
// Turn the `C` in `UNC` into a `\` so we can then use `\\rest\of\path`.
889+
let start = r"\\?\UN".len();
890+
dir_str[start] = b'\\' as u16;
891+
if path::is_absolute_exact(&dir_str[start..]) {
892+
dir_str[start..].as_ptr()
893+
} else {
894+
// Revert the above change.
895+
dir_str[start] = b'C' as u16;
896+
dir_str.as_ptr()
897+
}
898+
} else if dir_str.starts_with(utf16!(r"\\?\")) {
899+
// Strip the leading `\\?\`
900+
let start = r"\\?\".len();
901+
if path::is_absolute_exact(&dir_str[start..]) {
902+
dir_str[start..].as_ptr()
903+
} else {
904+
dir_str.as_ptr()
905+
}
906+
} else {
907+
dir_str.as_ptr()
908+
};
909+
Ok((ptr, dir_str))
886910
}
887911
None => Ok((ptr::null(), Vec::new())),
888912
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Test that windows verbatim paths in `Command::current_dir` are converted to
2+
// non-verbatim paths before passing to the subprocess.
3+
4+
//@ run-pass
5+
//@ only-windows
6+
//@ needs-subprocess
7+
8+
use std::env;
9+
use std::process::Command;
10+
11+
fn main() {
12+
if env::args().skip(1).any(|s| s == "--child") {
13+
child();
14+
} else {
15+
parent();
16+
}
17+
}
18+
19+
fn parent() {
20+
let exe = env::current_exe().unwrap();
21+
let dir = env::current_dir().unwrap();
22+
let status = Command::new(&exe)
23+
.arg("--child")
24+
.current_dir(dir.canonicalize().unwrap())
25+
.spawn()
26+
.unwrap()
27+
.wait()
28+
.unwrap();
29+
assert_eq!(status.code(), Some(0));
30+
}
31+
32+
fn child() {
33+
let current_dir = env::current_dir().unwrap();
34+
let current_dir = current_dir.as_os_str().as_encoded_bytes();
35+
assert!(!current_dir.starts_with(br"\\?\"));
36+
}

0 commit comments

Comments
 (0)