Skip to content

Commit b38439b

Browse files
authored
Avoid uv cache clean errors due to Win32 path normalization (#18856)
## Summary Closes #16586 Adds a public function to `uv-fs` crate called `verbatim_path` which is now leveraged by `rm_rf` in `uv-cache` crate for cleaning paths that require verbatim composition to avoid failures seen in #16586. ## Test Plan Tested locally on Windows 10 and 11 by setting `UV_CACHE_DIR` to a local directory and then running `uv init`, `uv add uwsgi`, and `uv cache clean` to ensure there is no failures. Additionally unit and integration tests were added to avoid future regressions.
1 parent a0e461a commit b38439b

3 files changed

Lines changed: 215 additions & 10 deletions

File tree

crates/uv-cache/src/removal.rs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ impl Removal {
6262
reporter: Option<&dyn CleanReporter>,
6363
skip_locked_file: bool,
6464
) -> io::Result<()> {
65-
let metadata = match fs_err::symlink_metadata(path) {
65+
let path = uv_fs::verbatim_path(path);
66+
67+
let metadata = match fs_err::symlink_metadata(&path) {
6668
Ok(metadata) => metadata,
6769
Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(()),
6870
Err(err) => return Err(err),
@@ -79,26 +81,26 @@ impl Removal {
7981
use std::os::windows::fs::FileTypeExt;
8082

8183
if metadata.file_type().is_symlink_dir() {
82-
remove_dir(path)?;
84+
remove_dir(&path)?;
8385
} else {
84-
remove_file(path)?;
86+
remove_file(&path)?;
8587
}
8688
}
8789

8890
#[cfg(not(windows))]
8991
{
90-
remove_file(path)?;
92+
remove_file(&path)?;
9193
}
9294
} else {
93-
remove_file(path)?;
95+
remove_file(&path)?;
9496
}
9597

9698
reporter.map(CleanReporter::on_clean);
9799

98100
return Ok(());
99101
}
100102

101-
for entry in walkdir::WalkDir::new(path).contents_first(true) {
103+
for entry in walkdir::WalkDir::new(&path).contents_first(true) {
102104
// If we hit a directory that lacks read permissions, try to make it readable.
103105
if let Err(ref err) = entry {
104106
if err
@@ -109,7 +111,7 @@ impl Removal {
109111
if set_readable(dir).unwrap_or(false) {
110112
// Retry the operation; if we _just_ `self.rm_rf(dir)` and continue,
111113
// `walkdir` may give us duplicate entries for the directory.
112-
return self.rm_rf(path, reporter, skip_locked_file);
114+
return self.rm_rf(&path, reporter, skip_locked_file);
113115
}
114116
}
115117
}
@@ -122,7 +124,7 @@ impl Removal {
122124
&& entry.file_name() == ".lock"
123125
&& entry
124126
.path()
125-
.strip_prefix(path)
127+
.strip_prefix(&path)
126128
.is_ok_and(|suffix| suffix == Path::new(".lock"))
127129
{
128130
continue;
@@ -143,7 +145,7 @@ impl Removal {
143145
remove_dir(entry.path())?;
144146
} else if entry.file_type().is_dir() {
145147
// Remove the directory with the exclusive lock last.
146-
if skip_locked_file && entry.path() == path {
148+
if skip_locked_file && entry.path() == path.as_ref() {
147149
continue;
148150
}
149151

crates/uv-fs/src/path.rs

Lines changed: 159 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::borrow::Cow;
2-
use std::path::{Component, Path, PathBuf};
2+
use std::ffi::OsString;
3+
use std::path::{Component, Path, PathBuf, Prefix};
34
use std::sync::LazyLock;
45

56
use either::Either;
@@ -351,6 +352,107 @@ pub fn try_relative_to_if(
351352
}
352353
}
353354

355+
/// Convert a [`Path`] to a Windows `verbatim` path (prefixed with `\\?\`) when possible to bypass
356+
/// Win32 path normalization such as [`MAX_PATH`] and removed trailing characters (dot, space).
357+
/// Other characters as defined by [`Path.GetInvalidFileNameChars`] are still prohibited. This
358+
/// function will attempt to perform path normalization similar to Win32 default normalization
359+
/// without triggering the existing Win32 limitations.
360+
///
361+
/// Only [`Prefix::UNC`] and [`Prefix::Disk`] conversion compatible components are supported.
362+
/// * [`Prefix::UNC`] `\\server\share` becomes `\\?\UNC\server\share`
363+
/// * [`Prefix::Disk`] `DriveLetter:` becomes `\\?\DriveLetter:`
364+
///
365+
/// Other representations do not yield a `verbatim` path. The following cases are returned as-is:
366+
/// * Non-Windows systems.
367+
/// * Device paths such as those starting with `\\.\`.
368+
/// * Paths already prefixed with `\\?\` or `\\?\UNC\`.
369+
///
370+
/// WARNING: Adding the `\\?\` prefix effectively skips Win32 default path normalization. Even
371+
/// though it allows operations on paths that are normally unavailable, it can also be used to
372+
/// create entries that can potentially lead to further issues with operations that expect
373+
/// normalization such as symbolic links, junctions or reparse points.
374+
///
375+
/// [`MAX_PATH`]: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
376+
/// [`Path.GetInvalidFileNameChars`]: https://learn.microsoft.com/en-us/dotnet/api/system.io.path.getinvalidfilenamechars
377+
///
378+
/// See:
379+
/// * <https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file>
380+
/// * <https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats>
381+
pub fn verbatim_path(path: &Path) -> Cow<'_, Path> {
382+
if !cfg!(windows) {
383+
return Cow::Borrowed(path);
384+
}
385+
386+
// Attempt to resolve a fully qualified path just like Win32 path normalization would.
387+
// std::path::absolute calls GetFullPathNameW which defeats the purpose of this function
388+
// as it results in Win32 default path normalization.
389+
let resolved_path = if path.is_relative() {
390+
Cow::Owned(CWD.join(path))
391+
} else {
392+
Cow::Borrowed(path)
393+
};
394+
395+
// Fast Path: we only support verbatim conversion for Prefix::UNC and Prefix::Disk
396+
if let Some(Component::Prefix(prefix)) = resolved_path.components().next() {
397+
match prefix.kind() {
398+
Prefix::UNC(..) | Prefix::Disk(_) => {},
399+
// return as-is as there's no verbatim equivalent for `\\.\device`
400+
Prefix::DeviceNS(_)
401+
// return as-is as its already verbatim
402+
| Prefix::Verbatim(_)
403+
| Prefix::VerbatimDisk(_)
404+
| Prefix::VerbatimUNC(..) => return Cow::Borrowed(path)
405+
}
406+
}
407+
408+
// Resolve relative directory components while avoiding default Win32 path normalization
409+
let normalized_path = normalized(&resolved_path);
410+
411+
let mut components = normalized_path.components();
412+
let Some(Component::Prefix(prefix)) = components.next() else {
413+
return Cow::Borrowed(path);
414+
};
415+
416+
match prefix.kind() {
417+
// `DriveLetter:` -> `\\?\DriveLetter:`
418+
Prefix::Disk(_) => {
419+
let mut result = OsString::from(r"\\?\");
420+
result.push(normalized_path.as_os_str()); // e.g. "C:"
421+
Cow::Owned(PathBuf::from(result))
422+
}
423+
// `\\server\share` -> `\\?\UNC\server\share`
424+
Prefix::UNC(server, share) => {
425+
let mut result = OsString::from(r"\\?\UNC\");
426+
result.push(server);
427+
result.push(r"\");
428+
result.push(share);
429+
for component in components {
430+
match component {
431+
Component::RootDir => {} // being cautious
432+
Component::Prefix(_) => {
433+
debug_assert!(false, "prefix already consumed");
434+
}
435+
Component::CurDir | Component::ParentDir => {
436+
debug_assert!(false, "path already normalized");
437+
}
438+
Component::Normal(_) => {
439+
result.push(r"\");
440+
result.push(component.as_os_str());
441+
}
442+
}
443+
}
444+
Cow::Owned(PathBuf::from(result))
445+
}
446+
Prefix::DeviceNS(_)
447+
| Prefix::Verbatim(_)
448+
| Prefix::VerbatimDisk(_)
449+
| Prefix::VerbatimUNC(..) => {
450+
debug_assert!(false, "skipped via fast path");
451+
Cow::Borrowed(path)
452+
}
453+
}
454+
}
455+
354456
/// A path that can be serialized and deserialized in a portable way by converting Windows-style
355457
/// backslashes to forward slashes, and using a `.` for an empty path.
356458
///
@@ -610,4 +712,60 @@ mod tests {
610712
assert_eq!(normalize_path(Path::new(input)), Path::new(expected));
611713
}
612714
}
715+
716+
#[cfg(windows)]
717+
#[test]
718+
fn test_verbatim_path() {
719+
let relative_path = format!(r"\\?\{}\path\to\logging.", CWD.simplified_display());
720+
let relative_root = format!(
721+
r"\\?\{}\path\to\logging.",
722+
CWD.components()
723+
.next()
724+
.expect("expected a drive letter prefix")
725+
.simplified_display()
726+
);
727+
let cases = [
728+
// Non-Verbatim disk
729+
(r"C:\path\to\logging.", r"\\?\C:\path\to\logging."),
730+
(r"C:\path\to\.\logging.", r"\\?\C:\path\to\logging."),
731+
(r"C:\path\to\..\to\logging.", r"\\?\C:\path\to\logging."),
732+
(r"C:/path/to/../to/./logging.", r"\\?\C:\path\to\logging."),
733+
(r"C:path\to\..\to\logging.", r"\\?\C:path\to\logging."), // @TODO(samypr100) we do not support expanding drive-relative paths
734+
(r".\path\to\.\logging.", relative_path.as_str()),
735+
(r"path\to\..\to\logging.", relative_path.as_str()),
736+
(r"./path/to/logging.", relative_path.as_str()),
737+
(r"\path\to\logging.", relative_root.as_str()),
738+
// Non-Verbatim UNC
739+
(
740+
r"\\127.0.0.1\c$\path\to\logging.",
741+
r"\\?\UNC\127.0.0.1\c$\path\to\logging.",
742+
),
743+
(
744+
r"\\127.0.0.1\c$\path\to\.\logging.",
745+
r"\\?\UNC\127.0.0.1\c$\path\to\logging.",
746+
),
747+
(
748+
r"\\127.0.0.1\c$\path\to\..\to\logging.",
749+
r"\\?\UNC\127.0.0.1\c$\path\to\logging.",
750+
),
751+
(
752+
r"//127.0.0.1/c$/path/to/../to/./logging.",
753+
r"\\?\UNC\127.0.0.1\c$\path\to\logging.",
754+
),
755+
// Verbatim Disk
756+
(r"\\?\C:\path\to\logging.", r"\\?\C:\path\to\logging."),
757+
// Verbatim UNC
758+
(
759+
r"\\?\UNC\127.0.0.1\c$\path\to\logging.",
760+
r"\\?\UNC\127.0.0.1\c$\path\to\logging.",
761+
),
762+
// Device Namespace
763+
(r"\\.\PhysicalDrive0", r"\\.\PhysicalDrive0"),
764+
(r"\\.\NUL", r"\\.\NUL"),
765+
];
766+
767+
for (input, expected) in cases {
768+
assert_eq!(verbatim_path(Path::new(input)), Path::new(expected));
769+
}
770+
}
613771
}

crates/uv/tests/it/cache_clean.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,3 +275,48 @@ async fn cache_timeout() {
275275
error: Timeout ([TIME]) when waiting for lock on `[CACHE_DIR]/` at `[CACHE_DIR]/.lock`, is another uv process running? You can set `UV_LOCK_TIMEOUT` to increase the timeout.
276276
");
277277
}
278+
279+
/// `cache clean` should handle file paths normally restricted by Win32 path normalization.
280+
#[cfg(windows)]
281+
#[test]
282+
fn clean_handles_verbatim_paths() -> Result<()> {
283+
let context = uv_test::test_context!("3.12");
284+
285+
// Clean slate
286+
fs_err::remove_dir_all(&context.cache_dir)?;
287+
288+
// Cached sdist path resembling the uwsgi==2.0.31 build failure.
289+
let uwsgi_shard = context
290+
.cache_dir
291+
.child("sdists-v9")
292+
.child("pypi")
293+
.child("uwsgi")
294+
.child("2.0.31")
295+
.child("QxDIp0qpjbsWjWURKmegK")
296+
.child("src")
297+
.child("core");
298+
299+
// Attempt to create a file with a trailing dot (we need to make it verbatim to do so)
300+
uwsgi_shard.create_dir_all()?;
301+
let invalid_path = uwsgi_shard.child("logging.").to_path_buf();
302+
let invalid_file = uv_fs::verbatim_path(invalid_path.as_path());
303+
fs_err::write(&invalid_file, b"")?;
304+
305+
// Confirm Win32 normalized path causes an os error when attempting to remove
306+
let remove_err = fs_err::remove_file(&invalid_path).expect_err("expected to fail");
307+
assert_eq!(remove_err.kind(), std::io::ErrorKind::NotFound);
308+
309+
// Tests cache clean leverages verbatim conversion
310+
uv_snapshot!(context.filters(), context.clean().arg("--verbose"), @"
311+
success: true
312+
exit_code: 0
313+
----- stdout -----
314+
315+
----- stderr -----
316+
DEBUG uv [VERSION] ([COMMIT] DATE)
317+
Clearing cache at: [CACHE_DIR]/
318+
Removed 2 files
319+
");
320+
321+
Ok(())
322+
}

0 commit comments

Comments
 (0)