Skip to content

Commit

Permalink
Path::join(): treat '..' at root as valid
Browse files Browse the repository at this point in the history 
In unix systems, trying to access the parent of root will return root.
This commit implements that behavior.
  • Loading branch information
@Property404
Property404 committed Dec 2, 2022
1 parent a406210 commit 8cdf9e9
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 107 deletions.
4 changes: 2 additions & 2 deletions src/impls/altroot.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/impls/overlay.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand All @@ -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(())
}
Expand Down
63 changes: 26 additions & 37 deletions src/path.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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<Self> {
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
Expand Down
88 changes: 22 additions & 66 deletions src/test_macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
Expand All @@ -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());
Expand Down Expand Up @@ -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("/"),
Expand Down Expand Up @@ -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"
);
}
Expand All @@ -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());
Expand Down Expand Up @@ -1131,38 +1110,15 @@ 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
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("/"),
Expand Down

0 comments on commit 8cdf9e9

Please sign in to comment.