From 8cdf9e91143535d9913d89f798ae844ab7fe2473 Mon Sep 17 00:00:00 2001 From: Property404 Date: Sat, 26 Nov 2022 17:24:47 -0500 Subject: [PATCH] Path::join(): treat '..' at root as valid In unix systems, trying to access the parent of root will return root. This commit implements that behavior. --- src/impls/altroot.rs | 4 +- src/impls/overlay.rs | 4 +- src/path.rs | 63 +++++++++++++------------------ src/test_macros.rs | 88 +++++++++++--------------------------------- 4 files changed, 52 insertions(+), 107 deletions(-) diff --git a/src/impls/altroot.rs b/src/impls/altroot.rs index 87908f4..8377da8 100644 --- a/src/impls/altroot.rs +++ b/src/impls/altroot.rs @@ -101,8 +101,8 @@ mod tests { let altroot_path = memory_root.join("altroot").unwrap(); altroot_path.create_dir().unwrap(); let altroot: VfsPath = AltrootFS::new(altroot_path.clone()).into(); - assert_eq!(altroot.parent(), None); - assert_eq!(altroot_path.parent(), Some(memory_root)); + assert_eq!(altroot.parent(), altroot.root()); + assert_eq!(altroot_path.parent(), memory_root); } } diff --git a/src/impls/overlay.rs b/src/impls/overlay.rs index 5bf231e..b126f7b 100644 --- a/src/impls/overlay.rs +++ b/src/impls/overlay.rs @@ -165,7 +165,7 @@ impl FileSystem for OverlayFS { write_path.remove_file()?; } let whiteout_path = self.whiteout_path(path)?; - whiteout_path.parent().unwrap().create_dir_all()?; + whiteout_path.parent().create_dir_all()?; whiteout_path.create_file()?; Ok(()) } @@ -178,7 +178,7 @@ impl FileSystem for OverlayFS { write_path.remove_dir()?; } let whiteout_path = self.whiteout_path(path)?; - whiteout_path.parent().unwrap().create_dir_all()?; + whiteout_path.parent().create_dir_all()?; whiteout_path.create_file()?; Ok(()) } diff --git a/src/path.rs b/src/path.rs index a4ddc0f..d868efd 100644 --- a/src/path.rs +++ b/src/path.rs @@ -118,10 +118,8 @@ impl VfsPath { if component == ".." { if !new_components.is_empty() { new_components.truncate(new_components.len() - 1); - } else if let Some(parent) = base_path.parent() { - base_path = parent; } else { - return Err(VfsError::from(VfsErrorKind::InvalidPath).with_path(path)); + base_path = self.parent(); } } else { new_components.push(component); @@ -299,31 +297,20 @@ impl VfsPath { /// Checks whether parent is a directory fn get_parent(&self, action: &str) -> VfsResult<()> { let parent = self.parent(); - match parent { - None => { - return Err(VfsError::from(VfsErrorKind::Other(format!( - "Could not {}, not a valid location", - action - ))) - .with_path(&self.path)); - } - Some(directory) => { - if !directory.exists()? { - return Err(VfsError::from(VfsErrorKind::Other(format!( - "Could not {}, parent directory does not exist", - action - ))) - .with_path(&self.path)); - } - let metadata = directory.metadata()?; - if metadata.file_type != VfsFileType::Directory { - return Err(VfsError::from(VfsErrorKind::Other(format!( - "Could not {}, parent path is not a directory", - action - ))) - .with_path(&self.path)); - } - } + if !parent.exists()? { + return Err(VfsError::from(VfsErrorKind::Other(format!( + "Could not {}, parent directory does not exist", + action + ))) + .with_path(&self.path)); + } + let metadata = parent.metadata()?; + if metadata.file_type != VfsFileType::Directory { + return Err(VfsError::from(VfsErrorKind::Other(format!( + "Could not {}, parent path is not a directory", + action + ))) + .with_path(&self.path)); } Ok(()) } @@ -559,24 +546,26 @@ impl VfsPath { /// Returns the parent path of this portion of this path /// - /// Returns `None` if this is a root path + /// Root will return itself. /// /// ``` /// # use std::io::Read; /// use vfs::{MemoryFS, VfsError, VfsFileType, VfsMetadata, VfsPath}; /// let path = VfsPath::new(MemoryFS::new()); /// - /// assert_eq!(path.parent(), None); - /// assert_eq!(path.join("foo/bar")?.parent(), Some(path.join("foo")?)); - /// assert_eq!(path.join("foo")?.parent(), Some(path)); + /// assert_eq!(path.parent(), path.root()); + /// assert_eq!(path.join("foo/bar")?.parent(), path.join("foo")?); + /// assert_eq!(path.join("foo")?.parent(), path); /// /// # Ok::<(), VfsError>(()) - pub fn parent(&self) -> Option { + pub fn parent(&self) -> Self { let index = self.path.rfind('/'); - index.map(|idx| VfsPath { - path: self.path[..idx].to_string(), - fs: self.fs.clone(), - }) + index + .map(|idx| VfsPath { + path: self.path[..idx].to_string(), + fs: self.fs.clone(), + }) + .unwrap_or_else(|| self.root()) } /// Recursively iterates over all the directories and files at this path diff --git a/src/test_macros.rs b/src/test_macros.rs index b0365bd..da0e33e 100644 --- a/src/test_macros.rs +++ b/src/test_macros.rs @@ -275,20 +275,20 @@ macro_rules! test_vfs { #[test] fn parent() { let root = create_root(); - assert_eq!(root.parent(), None, "root"); + assert_eq!(root.parent(), root.clone(), "root"); assert_eq!( root.join("foo").unwrap().parent(), - Some(root.clone()), + root.clone(), "foo" ); assert_eq!( root.join("foo/bar").unwrap().parent(), - Some(root.join("foo").unwrap()), + root.join("foo").unwrap(), "foo/bar" ); assert_eq!( root.join("foo/bar/baz").unwrap().parent(), - Some(root.join("foo/bar").unwrap()), + root.join("foo/bar").unwrap(), "foo/bar/baz" ); } @@ -301,9 +301,9 @@ macro_rules! test_vfs { assert_eq!(root.join("foo").unwrap(), root.join("foo").unwrap()); assert_eq!( root.join("foo").unwrap(), - root.join("foo/bar").unwrap().parent().unwrap() + root.join("foo/bar").unwrap().parent() ); - assert_eq!(root, root.join("foo").unwrap().parent().unwrap()); + assert_eq!(root, root.join("foo").unwrap().parent()); assert_ne!(root, root.join("foo").unwrap()); assert_ne!(root.join("bar").unwrap(), root.join("foo").unwrap()); @@ -375,37 +375,20 @@ macro_rules! test_vfs { .as_str(), "/foo" ); + assert_eq!( + root.join("..").unwrap(), + root + ); + assert_eq!( + root.join("../foo").unwrap(), + root.join("foo").unwrap() + ); /// Utility function for templating the same error message fn invalid_path_message(path: &str) -> String { format!("An error occured for '{}': The path is invalid", path) } - assert_eq!( - root.join("..").unwrap_err().to_string(), - invalid_path_message(".."), - ".." - ); - assert_eq!( - root.join("../foo").unwrap_err().to_string(), - invalid_path_message("../foo"), - "../foo" - ); - assert_eq!( - root.join("foo/../..").unwrap_err().to_string(), - invalid_path_message("foo/../.."), - "foo/../.." - ); - assert_eq!( - root.join("foo") - .unwrap() - .join("../..") - .unwrap_err() - .to_string(), - invalid_path_message("../.."), - "foo+../.." - ); - assert_eq!( root.join("/").unwrap_err().to_string(), invalid_path_message("/"), @@ -1024,20 +1007,16 @@ macro_rules! test_vfs_readonly { #[test] fn parent() { let root = create_root(); - assert_eq!(root.parent(), None, "root"); - assert_eq!( - root.join("foo").unwrap().parent(), - Some(root.clone()), - "foo" - ); + assert_eq!(root.parent(), root.clone(), "root"); + assert_eq!(root.join("foo").unwrap().parent(), root.clone(), "foo"); assert_eq!( root.join("foo/bar").unwrap().parent(), - Some(root.join("foo").unwrap()), + root.join("foo").unwrap(), "foo/bar" ); assert_eq!( root.join("foo/bar/baz").unwrap().parent(), - Some(root.join("foo/bar").unwrap()), + root.join("foo/bar").unwrap(), "foo/bar/baz" ); } @@ -1057,9 +1036,9 @@ macro_rules! test_vfs_readonly { assert_eq!(root.join("foo").unwrap(), root.join("foo").unwrap()); assert_eq!( root.join("foo").unwrap(), - root.join("foo/bar").unwrap().parent().unwrap() + root.join("foo/bar").unwrap().parent() ); - assert_eq!(root, root.join("foo").unwrap().parent().unwrap()); + assert_eq!(root, root.join("foo").unwrap().parent()); assert_ne!(root, root.join("foo").unwrap()); assert_ne!(root.join("bar").unwrap(), root.join("foo").unwrap()); @@ -1131,6 +1110,8 @@ macro_rules! test_vfs_readonly { .as_str(), "/foo" ); + assert_eq!(root.join("..").unwrap(), root); + assert_eq!(root.join("../foo").unwrap(), root.join("foo").unwrap()); /// Utility function for templating the same error message // TODO: Maybe deduplicate this function @@ -1138,31 +1119,6 @@ macro_rules! test_vfs_readonly { format!("An error occured for '{}': The path is invalid", path) } - assert_eq!( - root.join("..").unwrap_err().to_string(), - invalid_path_message(".."), - ".." - ); - assert_eq!( - root.join("../foo").unwrap_err().to_string(), - invalid_path_message("../foo"), - "../foo" - ); - assert_eq!( - root.join("foo/../..").unwrap_err().to_string(), - invalid_path_message("foo/../.."), - "foo/../.." - ); - assert_eq!( - root.join("foo") - .unwrap() - .join("../..") - .unwrap_err() - .to_string(), - invalid_path_message("../.."), - "foo+../.." - ); - assert_eq!( root.join("/").unwrap_err().to_string(), invalid_path_message("/"),