Remove std::fs::read_link in fs (#50974)

Alex Mihaiuc and John Tur created

Closes #46307

Before you mark this PR as ready for review, make sure that you have:
- [ ] Added a solid test coverage and/or screenshots from doing manual
testing
- [X] Done a self-review taking into account security and performance
aspects
- [ ] Aligned any UI changes with the [UI
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)

Release Notes:

- Improved compatibility with mounted VHDs on Windows.

---------

Co-authored-by: John Tur <john-tur@outlook.com>

Change summary

Cargo.lock             |   1 
Cargo.toml             |   1 
crates/fs/Cargo.toml   |   1 
crates/fs/src/fs.rs    | 109 ++++++++++++++-----------------------------
crates/util/Cargo.toml |   2 
5 files changed, 39 insertions(+), 75 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -6583,6 +6583,7 @@ dependencies = [
  "async-trait",
  "cocoa 0.26.0",
  "collections",
+ "dunce",
  "fs",
  "futures 0.3.31",
  "git",

Cargo.toml 🔗

@@ -548,6 +548,7 @@ derive_more = { version = "2.1.1", features = [
 dirs = "4.0"
 documented = "0.9.1"
 dotenvy = "0.15.0"
+dunce = "1.0"
 ec4rs = "1.1"
 emojis = "0.6.1"
 env_logger = "0.11"

crates/fs/Cargo.toml 🔗

@@ -48,6 +48,7 @@ cocoa = "0.26"
 
 [target.'cfg(target_os = "windows")'.dependencies]
 windows.workspace = true
+dunce.workspace = true
 
 [target.'cfg(any(target_os = "linux", target_os = "freebsd"))'.dependencies]
 ashpd.workspace = true

crates/fs/src/fs.rs 🔗

@@ -431,82 +431,43 @@ impl RealFs {
 
     #[cfg(target_os = "windows")]
     fn canonicalize(path: &Path) -> Result<PathBuf> {
-        let mut strip_prefix = None;
+        use std::ffi::OsString;
+        use std::os::windows::ffi::OsStringExt;
+        use windows::Win32::Storage::FileSystem::GetVolumePathNameW;
+        use windows::core::HSTRING;
 
-        let mut new_path = PathBuf::new();
-        for component in path.components() {
-            match component {
-                std::path::Component::Prefix(_) => {
-                    let component = component.as_os_str();
-                    let canonicalized = if component
-                        .to_str()
-                        .map(|e| e.ends_with("\\"))
-                        .unwrap_or(false)
-                    {
-                        std::fs::canonicalize(component)
-                    } else {
-                        let mut component = component.to_os_string();
-                        component.push("\\");
-                        std::fs::canonicalize(component)
-                    }?;
-
-                    let mut strip = PathBuf::new();
-                    for component in canonicalized.components() {
-                        match component {
-                            Component::Prefix(prefix_component) => {
-                                match prefix_component.kind() {
-                                    std::path::Prefix::Verbatim(os_str) => {
-                                        strip.push(os_str);
-                                    }
-                                    std::path::Prefix::VerbatimUNC(host, share) => {
-                                        strip.push("\\\\");
-                                        strip.push(host);
-                                        strip.push(share);
-                                    }
-                                    std::path::Prefix::VerbatimDisk(disk) => {
-                                        strip.push(format!("{}:", disk as char));
-                                    }
-                                    _ => strip.push(component),
-                                };
-                            }
-                            _ => strip.push(component),
-                        }
-                    }
-                    strip_prefix = Some(strip);
-                    new_path.push(component);
-                }
-                std::path::Component::RootDir => {
-                    new_path.push(component);
-                }
-                std::path::Component::CurDir => {
-                    if strip_prefix.is_none() {
-                        // unrooted path
-                        new_path.push(component);
-                    }
-                }
-                std::path::Component::ParentDir => {
-                    if strip_prefix.is_some() {
-                        // rooted path
-                        new_path.pop();
-                    } else {
-                        new_path.push(component);
-                    }
-                }
-                std::path::Component::Normal(_) => {
-                    if let Ok(link) = std::fs::read_link(new_path.join(component)) {
-                        let link = match &strip_prefix {
-                            Some(e) => link.strip_prefix(e).unwrap_or(&link),
-                            None => &link,
-                        };
-                        new_path.extend(link);
-                    } else {
-                        new_path.push(component);
-                    }
-                }
-            }
-        }
+        // std::fs::canonicalize resolves mapped network paths to UNC paths, which can
+        // confuse some software. To mitigate this, we canonicalize the input, then rebase
+        // the result onto the input's original volume root if both paths are on the same
+        // volume. This keeps the same drive letter or mount point the caller used.
 
-        Ok(new_path)
+        let abs_path = if path.is_relative() {
+            std::env::current_dir()?.join(path)
+        } else {
+            path.to_path_buf()
+        };
+
+        let path_hstring = HSTRING::from(abs_path.as_os_str());
+        let mut vol_buf = vec![0u16; abs_path.as_os_str().len() + 2];
+        unsafe { GetVolumePathNameW(&path_hstring, &mut vol_buf)? };
+        let volume_root = {
+            let len = vol_buf
+                .iter()
+                .position(|&c| c == 0)
+                .unwrap_or(vol_buf.len());
+            PathBuf::from(OsString::from_wide(&vol_buf[..len]))
+        };
+
+        let resolved_path = dunce::canonicalize(&abs_path)?;
+        let resolved_root = dunce::canonicalize(&volume_root)?;
+
+        if let Ok(relative) = resolved_path.strip_prefix(&resolved_root) {
+            let mut result = volume_root;
+            result.push(relative);
+            Ok(result)
+        } else {
+            Ok(resolved_path)
+        }
     }
 }
 

crates/util/Cargo.toml 🔗

@@ -21,7 +21,7 @@ test-support = ["git2", "rand", "util_macros"]
 anyhow.workspace = true
 async_zip.workspace = true
 collections.workspace = true
-dunce = "1.0"
+dunce.workspace = true
 futures-lite.workspace = true
 futures.workspace = true
 globset.workspace = true