real_clean initial implementation and some Linux tests

src/lib.rs

@@ -24,6 +24,10 @@ pub trait PathExt {
     /// - where `Path::parent()` returns `None`, `real_parent()` returns self for absolute root path, and appends `..` otherwise
     fn real_parent(&self) -> io::Result<PathBuf>;
 
+    /// Return a clean path, with dotdot folded away as much as possible, and without expanding symlinks except where required
+    /// for correctness.
+    fn real_clean(&self) -> io::Result<PathBuf>;
+
     /// Return whether this is a path to the root directory, regardless of whether or not it is relative or contains symlinks.
     /// Empty path is treated as `.`, that is, current directory, for compatibility with `Path::parent`.
     fn is_real_root(&self) -> io::Result<bool>;
@@ -45,6 +49,13 @@ impl PathExt for Path {
             .map_err(|e| io::Error::new(io::ErrorKind::Other, e))
     }
 
+    fn real_clean(&self) -> io::Result<PathBuf> {
+        let mut real_path = RealPath::default();
+        real_path
+            .real_clean(self)
+            .map_err(|e| io::Error::new(io::ErrorKind::Other, e))
+    }
+
     fn is_real_root(&self) -> io::Result<bool> {
         let path = if self.as_os_str().is_empty() {
             AsRef::<Path>::as_ref(DOT)
@@ -101,11 +112,7 @@ impl RealPath {
         // unwrap is safe because the last path component is a symlink
         let symlink_dir = path.parent().unwrap();
 
-        let resolved_target = if target.is_relative() {
-            self.real_join(symlink_dir, &target)?
-        } else {
-            target
-        };
+        let resolved_target = self.real_join(symlink_dir, &target)?;
 
         self.parent(resolved_target.as_path()).map(|p| p.into())
     }
@@ -137,45 +144,59 @@ impl RealPath {
         Ok(path.parent().unwrap().into())
     }
 
-    // join paths
-    // TODO maybe this should have a public interface
+    // join paths, folding away dotdot
     fn real_join<P1, P2>(&mut self, origin: P1, other: P2) -> Result<PathBuf, Error>
     where
         P1: AsRef<Path>,
         P2: AsRef<Path>,
     {
-        let origin = origin.as_ref();
+        use Component::*;
+
         let other = other.as_ref();
 
-        let mut resolving = origin.to_path_buf();
+        let (root, relative) = other
+            .components()
+            .partition::<Vec<_>, _>(|c| matches!(c, Prefix(_) | RootDir));
 
-        for component in other.components() {
-            use Component::*;
+        let mut resolving = if root.is_empty() {
+            origin.as_ref().to_path_buf()
+        } else {
+            root.iter().collect::<PathBuf>()
+        };
 
+        for component in relative {
             match component {
                 CurDir => (),
                 Prefix(_) | RootDir => {
                     panic!(
-                        "impossible absolute component in relative path \"{}\"",
-                        other.to_string_lossy()
+                        "impossible absolute component in relative part of path {:?}",
+                        other
                     )
                 }
                 ParentDir => match self.parent(resolving.as_path()) {
                     Ok(path) => {
-                        resolving = path.to_path_buf();
+                        resolving = path;
                     }
                     Err(e) => {
                         return Err(e);
                     }
                 },
-                Normal(path_component) => {
-                    resolving.push(path_component);
+                Normal(_) => {
+                    resolving.push(component);
                 }
             }
         }
 
         Ok(resolving)
     }
+
+    // clean a path, folding away dotdot
+    fn real_clean<P>(&mut self, path: P) -> Result<PathBuf, Error>
+    where
+        P: AsRef<Path>,
+    {
+        self.real_join("", path)
+    }
 }
 
 /// Our internal error type is an io:Error which includes the path which failed, or a cycle error.

tests/helpers/mod.rs

@@ -240,11 +240,12 @@ impl LinkFarm {
     }
 }
 
-// check real_parent() is as expected, with both absolute and relative paths
-pub fn check_real_parent_ok<P1, P2>(farm: &LinkFarm, path: P1, expected: P2)
+// check actual is as expected, with both absolute and relative paths
+pub fn check_ok<P1, P2, F>(farm: &LinkFarm, path: P1, expected: P2, f: F)
 where
     P1: AsRef<Path> + Debug,
     P2: AsRef<Path> + Debug,
+    F: FnOnce(&Path) -> io::Result<PathBuf> + Copy,
 {
     let path: &Path = path.as_ref();
     let expected: &Path = expected.as_ref();
@@ -255,7 +256,7 @@ where
     // test with relative paths
     farm.run_within(
         |path| {
-            let actual = path.real_parent();
+            let actual = f(path);
             is_expected_or_alt_path_ok(path, actual, expected, None, true);
         },
         path,
@@ -267,7 +268,7 @@ where
     let other_dir = tempdir().unwrap();
     farm.run_without(
         |path| {
-            let actual = path.real_parent();
+            let actual = f(path);
             // if we ascended out of the farm rootdir it's not straightforward to verify the logical path
             // that was returned, so we simply check the canonical version matches what was expected
             let check_logical = actual.as_ref().is_ok_and(|actual| farm.contains(actual));
@@ -283,7 +284,7 @@ where
         other_dir.path(),
     );
 
-    test_real_parent_with_unc_path(farm, &abs_path, &abs_expected);
+    test_with_unc_path(farm, &abs_path, &abs_expected, f);
 }
 
 #[cfg(target_family = "windows")]
@@ -315,18 +316,19 @@ where
 }
 
 #[cfg(target_family = "windows")]
-fn test_real_parent_with_unc_path<P1, P2>(farm: &LinkFarm, abs_path: P1, abs_expected: P2)
+fn test_with_unc_path<P1, P2, F>(farm: &LinkFarm, abs_path: P1, abs_expected: P2, f: F)
 where
     P1: AsRef<Path> + Debug,
     P2: AsRef<Path> + Debug,
+    F: FnOnce(&Path) -> io::Result<PathBuf> + Copy,
 {
     let unc_path = convert_disk_to_unc(&abs_path);
     let unc_expected = convert_disk_to_unc(&abs_expected);
 
     let other_dir = tempdir().unwrap();
     farm.run_without(
         |path| {
-            let actual = path.real_parent();
+            let actual = f(path);
             // if we ascended out of the farm rootdir it's not straightforward to verify the logical path
             // that was returned, so we simply check the canonical version matches what was expected
             let check_logical = actual.as_ref().is_ok_and(|actual| farm.contains(actual));
@@ -347,10 +349,11 @@ where
 }
 
 #[cfg(target_family = "unix")]
-fn test_real_parent_with_unc_path<P1, P2>(_farm: &LinkFarm, _abs_path: P1, _abs_expected: P2)
+fn test_with_unc_path<P1, P2, F>(_farm: &LinkFarm, _abs_path: P1, _abs_expected: P2, _f: F)
 where
     P1: AsRef<Path> + Debug,
     P2: AsRef<Path> + Debug,
+    F: FnOnce(&Path) -> io::Result<PathBuf> + Copy,
 {
     // nothing to do here, no UNC paths on unix
 }

tests/path_ext.rs

@@ -36,7 +36,7 @@ fn test_real_parent_files_directories(path: &str, expected: &str) {
         .file("A/.D/d1")
         .file("A/.D/.d1");
 
-    check_real_parent_ok(&farm, path, expected);
+    check_ok(&farm, path, expected, |p| p.real_parent());
 }
 
 #[test]
@@ -45,7 +45,7 @@ fn test_real_parent_root_dir() {
 
     let path = root_dir();
     let expected = path.as_path();
-    check_real_parent_ok(&farm, &path, expected);
+    check_ok(&farm, &path, expected, |p| p.real_parent());
 }
 
 #[test_case("A/B/_b1", "A/B")]
@@ -54,6 +54,7 @@ fn test_real_parent_root_dir() {
 #[test_case("A/_dot", "..")]
 #[test_case("A/B/_A", ".")]
 #[test_case("A/B/_B", "A")]
+#[test_case("A/B/C/_b1", "A/B")]
 #[test_case("_B/.", "A")]
 #[test_case("_B/..", "_B/../..")] // we don't attempt to fold away dotdot in base path
 #[test_case("_x1", ".")]
@@ -66,16 +67,18 @@ fn test_real_parent_rel_symlinks(path: &str, expected: &str) {
         .dir("A/B/C")
         .file("A/a1")
         .file("A/B/b1")
+        .file("A/B/C/c1")
         .symlink_rel("_x1", "x1")
         .symlink_rel("_B", "A/B")
         .symlink_rel("A/_dot", "..")
         .symlink_rel("A/B/_A", "..")
         .symlink_rel("A/B/_B", ".")
         .symlink_rel("A/B/_b1", "b1")
         .symlink_rel("A/B/_a1", "../a1")
-        .symlink_rel("A/B/C/_a1", "../../a1");
+        .symlink_rel("A/B/C/_a1", "../../a1")
+        .symlink_rel("A/B/C/_b1", "./.././b1");
 
-    check_real_parent_ok(&farm, path, expected);
+    check_ok(&farm, path, expected, |p| p.real_parent());
 }
 
 #[test_case("_B/b1", "_B")]
@@ -98,7 +101,7 @@ fn test_real_parent_rel_symlinks_not_windows(path: &str, expected: &str) {
         .symlink_rel("A/B/_a1", "../a1")
         .symlink_rel("A/B/C/_a1", "../../a1");
 
-    check_real_parent_ok(&farm, path, expected);
+    check_ok(&farm, path, expected, |p| p.real_parent());
 }
 
 #[test_case("A/B/__c", "A/B/C")]
@@ -116,7 +119,7 @@ fn test_real_parent_rel_indirect_symlinks(path: &str, expected: &str) {
         .symlink_rel("_c", "A/B/C/c1")
         .symlink_rel("A/B/__c", "../../_c");
 
-    check_real_parent_ok(&farm, path, expected);
+    check_ok(&farm, path, expected, |p| p.real_parent());
 }
 
 #[test_case("A/B/C/_b", "_B")]
@@ -136,7 +139,7 @@ fn test_real_parent_rel_indirect_symlinks_not_windows(path: &str, expected: &str
         .symlink_rel("_c", "A/B/C/c1")
         .symlink_rel("A/B/__c", "../../_c");
 
-    check_real_parent_ok(&farm, path, expected);
+    check_ok(&farm, path, expected, |p| p.real_parent());
 }
 
 #[test_case("A/B/=b1", "A/B")]
@@ -155,7 +158,7 @@ fn test_real_parent_abs_symlinks(path: &str, expected: &str) {
         .symlink_abs("A/B/=a1", "A/a1")
         .symlink_abs("A/B/=C", "A/C");
 
-    check_real_parent_ok(&farm, path, farm.absolute(expected));
+    check_ok(&farm, path, farm.absolute(expected), |p| p.real_parent());
 }
 
 #[test_case("A/_a1")]
@@ -207,9 +210,41 @@ fn test_real_parent_symlink_cycle_look_alikes(path: &str, expected: &str) {
         .symlink_rel("A/_a", "A/_a")
         .symlink_rel("A/A/_a", "A/a1");
 
-    check_real_parent_ok(&farm, path, expected);
+    check_ok(&farm, path, expected, |p| p.real_parent());
 }
 
+// #[test_case("x1", "x1")]
+// #[test_case("A", "A")]
+// #[test_case("A/a1", "A/a1")]
+// #[test_case("A/B/b1", "A/B/b1")]
+// #[test_case("A/B/C", "A/B/C")]
+// #[test_case("A/B/C/..", "A/B")]
+// #[test_case("A/B/C/.", "A/B/C"; "trailing dot is ignored")]
+// #[test_case("A/./B/C", "A/B/C"; "intermediate dot removed")]
+// #[test_case("A/../A/B/C", "A/B/C"; "intermediate dotdot folded away")]
+// #[test_case("A/.D", "A/.D" ; "hidden directory")]
+// #[test_case("A/.D/d1", "A/.D/d1" ; "file in hidden directory")]
+// #[test_case("A/.D/.d1", "A/.D/.d1" ; "hidden file in hidden directory")]
+// #[test_case("", "."; "empty path")]
+// #[test_case(".", "."; "bare dot")]
+// #[test_case("..", ".."; "bare dotdot")]
+// #[test_case("../../../../../../../../../..", "../../../../../../../../../.."; "dotdot overflow is ignored")]
+// fn test_real_clean_files_directories(path: &str, expected: &str) {
+//     let farm = LinkFarm::new();
+
+//     farm.file("x1")
+//         .dir("A")
+//         .dir("A/B")
+//         .dir("A/B/C")
+//         .dir("A/.D")
+//         .file("A/a1")
+//         .file("A/B/b1")
+//         .file("A/.D/d1")
+//         .file("A/.D/.d1");
+
+//     check_real_parent_ok(&farm, path, expected);
+// }
+
 #[test]
 fn test_is_real_root_root_dir() {
     let root_dir = root_dir();