Skip to content

Commit

Permalink
Fix panic caused by path prefix on Windows
Browse files Browse the repository at this point in the history
Paths on Windows can have a multi-component root.
E.g. C:\ is encoded as two components
'C:' and '\'. Unfortunately Path::root() and
Path::strip_root() didn't account for that properly
and stripped only the first component instead of stripping the
whole root. That led to incorrect path processing in
`fclones move` command and panic.

Fixes #185
  • Loading branch information
pkolaczk committed Mar 11, 2023
1 parent 2a68ac4 commit 1b5bbee
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 27 deletions.
10 changes: 4 additions & 6 deletions src/dedupe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,13 +658,11 @@ impl PartitionedFileGroup {
fn move_target(target_dir: &Arc<Path>, source_path: &Path) -> Path {
let root = source_path
.root()
.to_string_lossy()
.replace(['/', '\\', ':'], "");
.map(|p| p.to_string_lossy().replace(['/', '\\', ':'], ""));
let suffix = source_path.strip_root();
if root.is_empty() {
target_dir.join(suffix)
} else {
Arc::new(target_dir.join(Path::from(root))).join(suffix)
match root {
None => target_dir.join(suffix),
Some(root) => Arc::new(target_dir.join(Path::from(root))).join(suffix),
}
}

Expand Down
57 changes: 36 additions & 21 deletions src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,15 @@ pub const PATH_ESCAPE_CHAR: &str = "\\";
#[cfg(windows)]
pub const PATH_ESCAPE_CHAR: &str = "^";

#[cfg(unix)]
const ROOT_BYTES: &[u8] = b"/";

#[cfg(windows)]
const ROOT_BYTES: &[u8] = b"\\";

/// Memory-efficient file path representation.
///
/// When storing multiple paths with common parent, the standard [`std::path::PathBuf`]
/// When storing multiple paths with common parent, the standard [`PathBuf`]
/// would keep the parent path text duplicated in memory, wasting a lot of memory.
/// This structure here shares the common parent between many paths by reference-counted
/// references.
Expand All @@ -44,20 +50,31 @@ impl Path {
}
}

#[cfg(unix)]
pub fn is_absolute(&self) -> bool {
let root = self.root().component.as_bytes();
root == b"/"
self.root().is_some()
}

#[cfg(windows)]
pub fn is_absolute(&self) -> bool {
let root = self.root().component.as_bytes();
root.len() >= 1 && root[0] == b'\\' || root.len() == 2 && root[1] == b':'
pub fn is_relative(&self) -> bool {
self.root().is_none()
}

pub fn is_relative(&self) -> bool {
!self.is_absolute()
/// Returns the absolute root of the path if the path is absolute.
/// In Unix, returns "/".
/// In Windows this can return a root with prefix e.g. "C:\".
/// If path is relative, returns None.
pub fn root(&self) -> Option<&Path> {
let mut result = self;
loop {
if result.component.as_bytes() == ROOT_BYTES {
return Some(result);
}
if let Some(parent) = &result.parent {
result = parent.as_ref()
} else {
break;
}
}
None
}

/// Moves this [`Path`] under an [`Arc`].
Expand Down Expand Up @@ -147,8 +164,8 @@ impl Path {
/// Otherwise returns a clone of this path.
/// E.g. `/foo/bar` becomes `foo/bar`
pub fn strip_root(&self) -> Path {
if self.is_absolute() {
Path::make(self.components().into_iter().skip(1))
if let Some(root) = self.root() {
self.strip_prefix(root).unwrap()
} else {
self.clone()
}
Expand Down Expand Up @@ -284,15 +301,6 @@ impl Path {
}
result
}

/// Returns the first component of this path
pub fn root(&self) -> &Path {
let mut result = self;
while let Some(parent) = &result.parent {
result = parent.as_ref();
}
result
}
}

impl AsRef<Path> for Path {
Expand Down Expand Up @@ -507,6 +515,13 @@ mod test {
roundtrip("a \\x7F b");
}

#[test]
fn root() {
assert!(Path::from("foo/bar").root().is_none());
assert_eq!(Path::from("/foo/bar").root().unwrap(), &Path::from("/"));
assert_eq!(Path::from("/foo/bar").strip_root(), Path::from("foo/bar"));
}

#[test]
fn serialize() {
assert_ser_tokens(&Path::from("a \n b"), &[Token::String("a \\n b")])
Expand Down

0 comments on commit 1b5bbee

Please sign in to comment.