diff --git a/helix-core/src/path.rs b/helix-core/src/path.rs index 85c602555263c..a62117b6fee7e 100644 --- a/helix-core/src/path.rs +++ b/helix-core/src/path.rs @@ -30,30 +30,10 @@ pub fn expand_tilde(path: &Path) -> PathBuf { path.to_path_buf() } -/// Normalize a path, removing things like `.` and `..`. -/// -/// CAUTION: This does not resolve symlinks (unlike -/// [`std::fs::canonicalize`]). This may cause incorrect or surprising -/// behavior at times. This should be used carefully. Unfortunately, -/// [`std::fs::canonicalize`] can be hard to use correctly, since it can often -/// fail, or on Windows returns annoying device paths. This is a problem Cargo -/// needs to improve on. -/// Copied from cargo: +/// Normalize a path without resolving symlinks +/// Strategy: https://github.com/helix-editor/helix/pull/7658#issuecomment-1654633705 pub fn get_normalized_path(path: &Path) -> PathBuf { - // normalization strategy is to canonicalize first ancestor path that exists (i.e., canonicalize as much as possible), - // then run handrolled normalization on the non-existent remainder - let (base, path) = path - .ancestors() - .find_map(|base| { - let canonicalized_base = dunce::canonicalize(base).ok()?; - let remainder = path.strip_prefix(base).ok()?.into(); - Some((canonicalized_base, remainder)) - }) - .unwrap_or_else(|| (PathBuf::new(), PathBuf::from(path))); - - if path.as_os_str().is_empty() { - return base; - } + log::debug!("path before normalization: {}", path.to_string_lossy()); let mut components = path.components().peekable(); let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() { @@ -71,14 +51,39 @@ pub fn get_normalized_path(path: &Path) -> PathBuf { } Component::CurDir => {} Component::ParentDir => { - ret.pop(); + if let Some(head) = ret.components().next_back() { + match head { + Component::Prefix(_) | Component::RootDir => {} + Component::CurDir | Component::ParentDir => unreachable!(), + Component::Normal(_) => { + ret.pop(); + } + } + } } Component::Normal(c) => { - ret.push(c); + let new_path = ret.join(c); + // If current and next paths are symlinks, we can't strip prefix because the next path might resolve to a completely different location + if ret.is_symlink() && !new_path.is_symlink() { + if let Ok(canon) = dunce::canonicalize(&new_path) { + // If we can canonicalize new path, then it's safe to assume we can also canonicalize the old path + ret.push( + canon + .strip_prefix(dunce::canonicalize(&ret).unwrap()) + .unwrap(), + ) + } else { + ret.push(c); + } + } else { + ret.push(c); + } } } } - base.join(ret) + + log::debug!("normalized path: {}", ret.to_string_lossy()); + ret } /// Returns the canonical, absolute form of a path with all intermediate components normalized. @@ -160,3 +165,27 @@ pub fn get_truncated_path>(path: P) -> PathBuf { ret.push(file); ret } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + #[cfg(unix)] + fn test_normalize_path() { + let tests = [ + ("/../../..", "/"), + ("/home/blah/./../blah/", "/home/blah/"), + ("~/..", ""), + ]; + for (unit, ans) in tests { + assert_eq!( + get_normalized_path(&PathBuf::from(unit)), + PathBuf::from(ans), + "expected normalized path from `{}` is `{}`", + unit, + ans + ); + } + } +}