From 32a0b64cb2b2800227b7f4dc8a52816fbebe0240 Mon Sep 17 00:00:00 2001 From: TimeEngineer Date: Tue, 7 Nov 2023 13:13:55 +0100 Subject: [PATCH] Use FsPath for mountpoint --- .../crates/platform_mountpoint/src/lib.rs | 26 ++-- .../crates/platform_mountpoint/src/memfs.rs | 144 +++++++----------- .../platform_mountpoint/src/unix/inode.rs | 59 +++---- .../platform_mountpoint/src/unix/mod.rs | 58 ++++++- .../platform_mountpoint/src/windows/mod.rs | 63 +++++--- .../platform_mountpoint/src/windows/winify.rs | 4 - .../platform_mountpoint/tests/unit/inode.rs | 89 ++++++----- .../platform_mountpoint/tests/unit/winfsp.rs | 49 ++++++ .../platform_mountpoint/tests/winfsp_tests.rs | 9 +- libparsec/crates/types/src/fs_path.rs | 36 ++++- 10 files changed, 326 insertions(+), 211 deletions(-) create mode 100644 libparsec/crates/platform_mountpoint/tests/unit/winfsp.rs diff --git a/libparsec/crates/platform_mountpoint/src/lib.rs b/libparsec/crates/platform_mountpoint/src/lib.rs index 283bb6a3a59..78452e3ea80 100644 --- a/libparsec/crates/platform_mountpoint/src/lib.rs +++ b/libparsec/crates/platform_mountpoint/src/lib.rs @@ -17,7 +17,7 @@ pub(crate) use unix as platform; #[cfg(target_os = "windows")] pub(crate) use windows as platform; -use libparsec_types::{anyhow, EntryName, FileDescriptor, VlobID}; +use libparsec_types::{anyhow, EntryName, FileDescriptor, FsPath, VlobID}; pub use error::{MountpointError, MountpointResult}; pub use memfs::MemFS; @@ -89,29 +89,33 @@ impl FileSystemWrapper { pub trait MountpointInterface { // Rights check - fn check_read_rights(&self, path: &Path) -> MountpointResult<()>; - fn check_write_rights(&self, path: &Path) -> MountpointResult<()>; + fn check_read_rights(&self, path: &FsPath) -> MountpointResult<()>; + fn check_write_rights(&self, path: &FsPath) -> MountpointResult<()>; // Entry transactions - fn entry_info(&self, path: &Path) -> MountpointResult; + fn entry_info(&self, path: &FsPath) -> MountpointResult; fn entry_rename( &self, - source: &Path, - destination: &Path, + source: &FsPath, + destination: &FsPath, overwrite: bool, ) -> MountpointResult<()>; // Directory transactions - fn dir_create(&self, path: &Path) -> MountpointResult<()>; - fn dir_delete(&self, path: &Path) -> MountpointResult<()>; + fn dir_create(&self, path: &FsPath) -> MountpointResult<()>; + fn dir_delete(&self, path: &FsPath) -> MountpointResult<()>; // File transactions - fn file_create(&self, path: &Path, open: bool) -> MountpointResult; - fn file_open(&self, path: &Path, write_mode: bool) -> MountpointResult>; - fn file_delete(&self, path: &Path) -> MountpointResult<()>; + fn file_create(&self, path: &FsPath, open: bool) -> MountpointResult; + fn file_open( + &self, + path: &FsPath, + write_mode: bool, + ) -> MountpointResult>; + fn file_delete(&self, path: &FsPath) -> MountpointResult<()>; // File descriptor transactions diff --git a/libparsec/crates/platform_mountpoint/src/memfs.rs b/libparsec/crates/platform_mountpoint/src/memfs.rs index 861619ecf2b..7b1ff17bb28 100644 --- a/libparsec/crates/platform_mountpoint/src/memfs.rs +++ b/libparsec/crates/platform_mountpoint/src/memfs.rs @@ -6,17 +6,16 @@ use chrono::Utc; use std::{ collections::HashMap, - path::{Path, PathBuf}, sync::{Arc, Mutex}, }; -use libparsec_types::{EntryName, FileDescriptor, VlobID}; +use libparsec_types::{FileDescriptor, FsPath, VlobID}; use crate::{ EntryInfo, EntryInfoType, MountpointError, MountpointInterface, MountpointResult, WriteMode, }; -type Entries = Mutex>>>)>>; +type Entries = Mutex>>>)>>; pub struct MemFS { entries: Entries, @@ -25,7 +24,7 @@ pub struct MemFS { } struct FileData { - path: PathBuf, + path: FsPath, data: Arc>>, } @@ -35,7 +34,7 @@ impl Default for MemFS { let now = Utc::now(); entries.insert( - PathBuf::from("/"), + "/".parse().expect("unreachable"), ( EntryInfo::new(VlobID::default(), EntryInfoType::Dir, now), None, @@ -72,15 +71,15 @@ impl MemFS { } impl MountpointInterface for MemFS { - fn check_read_rights(&self, _path: &Path) -> MountpointResult<()> { + fn check_read_rights(&self, _path: &FsPath) -> MountpointResult<()> { Ok(()) } - fn check_write_rights(&self, _path: &Path) -> MountpointResult<()> { + fn check_write_rights(&self, _path: &FsPath) -> MountpointResult<()> { Ok(()) } - fn entry_info(&self, path: &Path) -> MountpointResult { + fn entry_info(&self, path: &FsPath) -> MountpointResult { self.entries .lock() .expect("Mutex is poisoned") @@ -91,19 +90,16 @@ impl MountpointInterface for MemFS { fn entry_rename( &self, - source: &Path, - destination: &Path, + source: &FsPath, + destination: &FsPath, overwrite: bool, ) -> MountpointResult<()> { let mut entries = self.entries.lock().expect("Mutex is poisoned"); - let source_str = source.to_str().ok_or(MountpointError::NotFound)?; - let destination_str = destination.to_str().ok_or(MountpointError::InvalidName)?; - let root = Path::new("/"); // Source is root // Destination is root // Cross-directory renaming is not supported - if source == root || destination == root || source.parent() != destination.parent() { + if source.is_root() || destination.is_root() || source.parent() != destination.parent() { return Err(MountpointError::AccessDenied); } @@ -118,55 +114,43 @@ impl MountpointInterface for MemFS { } } - let iter_entries = entries - .keys() - .filter_map(|path| match path.to_str() { - Some(path) if path.starts_with(source_str) => Some(path.to_string()), - _ => None, + *entries = entries + .drain() + .map(|(path, info)| { + if path.starts_with(source) { + ( + path.replace_parent(source.parts().len(), destination.clone()), + info, + ) + } else { + (path, info) + } }) - .collect::>(); - - for entry_path in iter_entries { - let new_entry_path = PathBuf::from(entry_path.replacen(source_str, destination_str, 1)); - - let entry = entries - .remove(Path::new(&entry_path)) - .ok_or(MountpointError::NotFound)?; - entries.insert(new_entry_path, entry); - } + .collect(); let (parent, _) = entries - .get_mut(source.parent().expect("Can't be root")) + .get_mut(&source.parent()) .ok_or(MountpointError::NotFound)?; let id = parent .children - .remove(&EntryName::try_from( - source - .file_name() - .ok_or(MountpointError::InvalidName)? - .to_str() - .ok_or(MountpointError::InvalidName)?, - )?) + .remove(source.name().ok_or(MountpointError::InvalidName)?) .ok_or(MountpointError::NotFound)?; let (parent, _) = entries - .get_mut(destination.parent().expect("Can't be root")) + .get_mut(&destination.parent()) .ok_or(MountpointError::NotFound)?; parent.children.insert( - EntryName::try_from( - destination - .file_name() - .ok_or(MountpointError::InvalidName)? - .to_str() - .ok_or(MountpointError::InvalidName)?, - )?, + destination + .name() + .ok_or(MountpointError::InvalidName)? + .clone(), id, ); Ok(()) } - fn file_create(&self, path: &Path, _open: bool) -> MountpointResult { + fn file_create(&self, path: &FsPath, _open: bool) -> MountpointResult { let mut entries = self.entries.lock().expect("Mutex is poisoned"); if entries.contains_key(path) { @@ -178,24 +162,20 @@ impl MountpointInterface for MemFS { let id = VlobID::default(); let now = Utc::now(); let fd = self.create_file_descriptor(FileData { - path: path.into(), + path: path.clone(), data: data.clone(), }); let (parent, _) = entries - .get_mut(path.parent().expect("Can't be root")) + .get_mut(&path.parent()) .ok_or(MountpointError::NotFound)?; - let file_name = path - .file_name() - .ok_or(MountpointError::InvalidName)? - .to_str() - .ok_or(MountpointError::InvalidName)?; + let file_name = path.name().ok_or(MountpointError::InvalidName)?; - parent.children.insert(EntryName::try_from(file_name)?, id); + parent.children.insert(file_name.clone(), id); entries.insert( - path.to_path_buf(), + path.clone(), ( EntryInfo::new(VlobID::default(), EntryInfoType::File, now), Some(data), @@ -207,7 +187,7 @@ impl MountpointInterface for MemFS { fn file_open( &self, - path: &Path, + path: &FsPath, _write_mode: bool, ) -> MountpointResult> { let entries = self.entries.lock().expect("Mutex is poisoned"); @@ -216,7 +196,7 @@ impl MountpointInterface for MemFS { if let Some(data) = data { Ok(Some(self.create_file_descriptor(FileData { - path: path.into(), + path: path.clone(), data: data.clone(), }))) } else { @@ -224,19 +204,16 @@ impl MountpointInterface for MemFS { } } - fn file_delete(&self, path: &Path) -> MountpointResult<()> { + fn file_delete(&self, path: &FsPath) -> MountpointResult<()> { let mut entries = self.entries.lock().expect("Mutex is poisoned"); let (parent, _) = entries - .get_mut(path.parent().expect("Can't be root")) + .get_mut(&path.parent()) .ok_or(MountpointError::NotFound)?; - parent.children.remove(&EntryName::try_from( - path.file_name() - .ok_or(MountpointError::InvalidName)? - .to_str() - .ok_or(MountpointError::InvalidName)?, - )?); + parent + .children + .remove(path.name().ok_or(MountpointError::InvalidName)?); entries.remove(path).ok_or(MountpointError::NotFound)?; @@ -344,7 +321,7 @@ impl MountpointInterface for MemFS { Ok(transferred_length) } - fn dir_create(&self, path: &Path) -> MountpointResult<()> { + fn dir_create(&self, path: &FsPath) -> MountpointResult<()> { let mut entries = self.entries.lock().expect("Mutex is poisoned"); if entries.contains_key(path) { @@ -355,47 +332,42 @@ impl MountpointInterface for MemFS { let now = Utc::now(); let (parent, _) = entries - .get_mut(path.parent().expect("Can't be root")) + .get_mut(&path.parent()) .ok_or(MountpointError::NotFound)?; - parent.children.insert( - EntryName::try_from( - path.file_name() - .ok_or(MountpointError::InvalidName)? - .to_str() - .ok_or(MountpointError::InvalidName)?, - )?, - id, - ); + parent + .children + .insert(path.name().ok_or(MountpointError::InvalidName)?.clone(), id); entries.insert( - path.to_path_buf(), + path.clone(), (EntryInfo::new(id, EntryInfoType::Dir, now), None), ); Ok(()) } - fn dir_delete(&self, path: &Path) -> MountpointResult<()> { + fn dir_delete(&self, path: &FsPath) -> MountpointResult<()> { let mut entries = self.entries.lock().expect("Mutex is poisoned"); - if entries.keys().any(|entry| entry.parent() == Some(path)) { + if entries.keys().any(|entry| &entry.parent() == path) { return Err(MountpointError::DirNotEmpty); } let (parent, _) = entries - .get_mut(path.parent().expect("Can't be root")) + .get_mut(&path.parent()) .ok_or(MountpointError::NotFound)?; - parent.children.remove(&EntryName::try_from( - path.file_name() - .ok_or(MountpointError::InvalidName)? - .to_str() - .ok_or(MountpointError::InvalidName)?, - )?); + parent + .children + .remove(path.name().ok_or(MountpointError::InvalidName)?); entries.remove(path).ok_or(MountpointError::NotFound)?; Ok(()) } } + +#[cfg(all(test, target_os = "windows"))] +#[path = "../tests/unit/winfsp.rs"] +mod tests; diff --git a/libparsec/crates/platform_mountpoint/src/unix/inode.rs b/libparsec/crates/platform_mountpoint/src/unix/inode.rs index bdcdb76f829..dddd6f115d2 100644 --- a/libparsec/crates/platform_mountpoint/src/unix/inode.rs +++ b/libparsec/crates/platform_mountpoint/src/unix/inode.rs @@ -2,26 +2,31 @@ use std::{ collections::HashMap, - ffi::OsString, - os::unix::prelude::{OsStrExt, OsStringExt}, - path::{Path, PathBuf}, sync::{Mutex, RwLock}, }; +use libparsec_types::FsPath; + use crate::{FileSystemWrapper, MountpointInterface}; /// `paths_indexed_by_inode` allocator indexed by `inode` /// The index of path is the inode number and the free inodes are stored /// `Pasteur` must contain his rage when he sees this code struct PathsStore { - paths_indexed_by_inode: Vec, + paths_indexed_by_inode: Vec, stack_unused_inodes: Vec, } impl Default for PathsStore { fn default() -> Self { + let root: FsPath = "/".parse().expect("unreachable"); + Self { - paths_indexed_by_inode: vec![PathBuf::from(""), PathBuf::from("/")], + paths_indexed_by_inode: vec![ + // Inode 0 is error prone, so we fill it with a dummy value + root.clone(), + root, + ], stack_unused_inodes: vec![], } } @@ -93,11 +98,11 @@ impl From for u64 { #[derive(Default)] pub(crate) struct InodeManager { paths_store: RwLock, - opened: Mutex>, + opened: Mutex>, } impl FileSystemWrapper { - pub(super) fn insert_path(&self, path: PathBuf) -> Inode { + pub(super) fn insert_path(&self, path: FsPath) -> Inode { let mut paths_store = self .inode_manager .paths_store @@ -152,7 +157,7 @@ impl FileSystemWrapper { /// Safety: /// It will panic if: /// - `inode` does not exist - pub(super) unsafe fn get_path(&self, inode: Inode) -> PathBuf { + pub(super) unsafe fn get_path(&self, inode: Inode) -> FsPath { let paths_store = self .inode_manager .paths_store @@ -162,47 +167,33 @@ impl FileSystemWrapper { paths_store.paths_indexed_by_inode[usize::from(inode)].clone() } - pub(super) fn rename_path(&self, source: &Path, destination: &Path) { + pub(super) fn rename_path(&self, source: &FsPath, destination: &FsPath) { let mut paths_store = self .inode_manager .paths_store .write() .expect("Mutex is poisoned"); let mut opened = self.inode_manager.opened.lock().expect("Mutex is poisoned"); - let source_bytes = source.as_os_str().as_bytes(); - let destination_bytes = destination.as_os_str().as_bytes(); for path in paths_store .paths_indexed_by_inode .iter_mut() - .filter(|path| path.as_os_str().as_bytes().starts_with(source_bytes)) + .filter(|path| path.starts_with(source)) { - let path_bytes = path.as_os_str().as_bytes(); - - let mut new_path = - Vec::with_capacity(path_bytes.len() - source_bytes.len() + destination_bytes.len()); - - new_path.extend(destination_bytes); - new_path.extend(&path_bytes[source_bytes.len()..]); - - *path.as_mut_os_string() = OsString::from_vec(new_path); + *path = path.replace_parent(source.parts().len(), destination.clone()); } *opened = opened .drain() - .filter(|(path, _)| path.as_os_str().as_bytes().starts_with(source_bytes)) - .map(|(mut path, v)| { - let path_bytes = path.as_os_str().as_bytes(); - - let mut new_path = Vec::with_capacity( - path_bytes.len() - source_bytes.len() + destination_bytes.len(), - ); - - new_path.extend(destination_bytes); - new_path.extend(&path_bytes[source_bytes.len()..]); - *path.as_mut_os_string() = OsString::from_vec(new_path); - - (path, v) + .map(|(path, ino)| { + if path.starts_with(source) { + ( + path.replace_parent(source.parts().len(), destination.clone()), + ino, + ) + } else { + (path, ino) + } }) .collect(); } diff --git a/libparsec/crates/platform_mountpoint/src/unix/mod.rs b/libparsec/crates/platform_mountpoint/src/unix/mod.rs index 2ed65a10a22..6ba34f499bf 100644 --- a/libparsec/crates/platform_mountpoint/src/unix/mod.rs +++ b/libparsec/crates/platform_mountpoint/src/unix/mod.rs @@ -7,7 +7,7 @@ use fuser::{ ReplyEmpty, ReplyEntry, ReplyOpen, ReplyWrite, Request, Session, }; use libc::{EINVAL, ENAMETOOLONG, ENOENT, ENOTEMPTY, EPERM}; -use libparsec_types::{anyhow, FileDescriptor}; +use libparsec_types::{anyhow, EntryName, EntryNameError, EntryNameResult, FileDescriptor}; use std::{ffi::OsStr, path::Path, time::Duration}; use crate::{ @@ -76,6 +76,14 @@ impl Filesystem for FileSystemWrapper { /// does not know. It transforms the path name to `inode`. /// The `inode` is then used by all other operations and is freed by `forget`. fn lookup(&mut self, _req: &Request, parent: u64, name: &OsStr, reply: ReplyEntry) { + let name = match os_name_to_entry_name(name) { + Ok(name) => name, + _ => { + reply.error(EINVAL); + return; + } + }; + // Safety: Parent should exists (resolved by lookup method) let parent_path = unsafe { self.get_path(Inode::from(parent)) }; let path = parent_path.join(name); @@ -175,7 +183,7 @@ impl Filesystem for FileSystemWrapper { if offset >= 2 { offset -= 2; for (i, (name, id)) in entry.children.iter().enumerate().skip(offset as usize) { - let path = path.join(name.as_ref()); + let path = path.join(name.clone()); let entry = self.interface.entry_info(&path).expect("Should exists"); if reply.add( @@ -205,6 +213,14 @@ impl Filesystem for FileSystemWrapper { flags: i32, reply: ReplyCreate, ) { + let name = match os_name_to_entry_name(name) { + Ok(name) => name, + _ => { + reply.error(EINVAL); + return; + } + }; + // Safety: Parent should exists (resolved by lookup method) let parent_path = unsafe { self.get_path(Inode::from(parent)) }; let path = parent_path.join(name); @@ -310,6 +326,14 @@ impl Filesystem for FileSystemWrapper { } fn unlink(&mut self, _req: &Request, parent: u64, name: &OsStr, reply: ReplyEmpty) { + let name = match os_name_to_entry_name(name) { + Ok(name) => name, + _ => { + reply.error(EINVAL); + return; + } + }; + // Safety: Parent should exists (resolved by lookup method) let parent_path = unsafe { self.get_path(Inode::from(parent)) }; let path = parent_path.join(name); @@ -330,6 +354,14 @@ impl Filesystem for FileSystemWrapper { _umask: u32, reply: ReplyEntry, ) { + let name = match os_name_to_entry_name(name) { + Ok(name) => name, + _ => { + reply.error(EINVAL); + return; + } + }; + // Safety: Parent should exists (resolved by lookup method) let parent_path = unsafe { self.get_path(Inode::from(parent)) }; let path = parent_path.join(name); @@ -347,6 +379,14 @@ impl Filesystem for FileSystemWrapper { } fn rmdir(&mut self, _req: &Request, parent: u64, name: &OsStr, reply: ReplyEmpty) { + let name = match os_name_to_entry_name(name) { + Ok(name) => name, + _ => { + reply.error(EINVAL); + return; + } + }; + // Safety: Parent should exists (resolved by lookup method) let parent_path = unsafe { self.get_path(Inode::from(parent)) }; let path = parent_path.join(name); @@ -369,6 +409,14 @@ impl Filesystem for FileSystemWrapper { _flags: u32, reply: ReplyEmpty, ) { + let (name, newname) = match (os_name_to_entry_name(name), os_name_to_entry_name(newname)) { + (Ok(name), Ok(newname)) => (name, newname), + _ => { + reply.error(EINVAL); + return; + } + }; + // Safety: Parent should exists (resolved by lookup method) let parent_path = unsafe { self.get_path(Inode::from(parent)) }; let source = parent_path.join(name); @@ -438,3 +486,9 @@ pub(super) fn mount( phantom: Default::default(), }) } + +fn os_name_to_entry_name(name: &OsStr) -> EntryNameResult { + name.to_str() + .map(|s| s.parse()) + .ok_or(EntryNameError::InvalidName)? +} diff --git a/libparsec/crates/platform_mountpoint/src/windows/mod.rs b/libparsec/crates/platform_mountpoint/src/windows/mod.rs index 60d6267b017..db6ddbd05f1 100644 --- a/libparsec/crates/platform_mountpoint/src/windows/mod.rs +++ b/libparsec/crates/platform_mountpoint/src/windows/mod.rs @@ -6,7 +6,6 @@ mod winify; use once_cell::sync::{Lazy, OnceCell}; use std::{ ops::Deref, - path::{Path, PathBuf}, sync::{Arc, Mutex}, }; @@ -15,7 +14,7 @@ use winfsp_wrs::{ FileAccessRights, FileAttributes, FileInfo, FileSystem, FileSystemContext, PSecurityDescriptor, Params, SecurityDescriptor, U16CStr, U16CString, VolumeInfo, VolumeParams, WriteMode as WinWriteMode, NTSTATUS, STATUS_DIRECTORY_NOT_EMPTY, STATUS_NOT_IMPLEMENTED, - STATUS_RESOURCEMANAGER_READ_ONLY, + STATUS_OBJECT_NAME_INVALID, STATUS_RESOURCEMANAGER_READ_ONLY, }; use libparsec_types::prelude::*; @@ -23,8 +22,7 @@ use libparsec_types::prelude::*; use crate::{ EntryInfo, EntryInfoType, FileSystemMounted, FileSystemWrapper, MountpointInterface, WriteMode, }; -// TODO: Remove this when used. -#[allow(unused_imports)] + pub(crate) use winify::{unwinify_entry_name, winify_entry_name}; /// we currently don't support arbitrary security descriptor and instead use only this one @@ -51,7 +49,7 @@ const VOLUME_LABEL: &U16CStr = u16cstr!("parsec"); /// see: https://github.com/winfsp/winfsp/issues/240#issuecomment-518629301 const SECTOR_SIZE: u16 = 512; -fn is_entry_info_path(path: &Path) -> bool { +fn is_entry_info_path(path: &FsPath) -> bool { path.extension() .map(|ext| ext == ENTRY_INFO_EXTENSION) .unwrap_or_default() @@ -116,7 +114,7 @@ pub enum Context { } impl Context { - fn path(&self) -> &Path { + fn path(&self) -> &FsPath { match self { Self::File(file) => &file.path, Self::Dir(dir) => &dir.path, @@ -127,19 +125,19 @@ impl Context { #[derive(Debug, Clone)] pub struct File { - path: PathBuf, + path: FsPath, fd: FileDescriptor, } #[derive(Debug, Clone)] pub struct Dir { - path: PathBuf, + path: FsPath, } /// Special Parsec file for icon overlay handler #[derive(Debug, Clone)] pub struct ParsecEntryInfo { - path: PathBuf, + path: FsPath, encoded: String, } @@ -184,7 +182,7 @@ impl FileSystemContext for FileSystemWrapper { file_name: &U16CStr, _find_reparse_point: impl Fn() -> Option, ) -> Result<(FileAttributes, PSecurityDescriptor, bool), NTSTATUS> { - let path = PathBuf::from(file_name.to_os_string()); + let path = os_path_to_fs_path(file_name)?; let entry_info = self.interface.entry_info(&path)?; Ok(( @@ -201,7 +199,7 @@ impl FileSystemContext for FileSystemWrapper { granted_access: FileAccessRights, ) -> Result { let write_mode = granted_access.is(FileAccessRights::file_write_data()); - let path = PathBuf::from(file_name.to_os_string()); + let path = os_path_to_fs_path(file_name)?; let fd = self.interface.file_open(&path, write_mode)?; @@ -239,10 +237,10 @@ impl FileSystemContext for FileSystemWrapper { // NOTE: The "." and ".." directories should ONLY be included // if the queried directory is not root - if fc.path() != Path::new("/") && marker.is_none() { + if !fc.path().is_root() && marker.is_none() { entries.push((u16cstr!(".").into(), FileInfo::from(&stat))); - let parent_path = fc.path().parent().expect("FileContext can't be root"); - let parent_stat = self.interface.entry_info(parent_path)?; + let parent_path = fc.path().parent(); + let parent_stat = self.interface.entry_info(&parent_path)?; entries.push((u16cstr!("..").into(), FileInfo::from(&parent_stat))); } @@ -258,19 +256,21 @@ impl FileSystemContext for FileSystemWrapper { .skip_while(|name| name.as_ref() != marker.to_string_lossy()) .skip(1) { - let child_path = fc.path().join(child_name.as_ref()); + let child_path = fc.path().join(child_name.clone()); let child_stat = self.interface.entry_info(&child_path)?; entries.push(( - U16CString::from_str(child_name).expect("Contains a nul value"), + U16CString::from_str(winify_entry_name(child_name)) + .expect("Contains a nul value"), FileInfo::from(&child_stat), )); } } else { for child_name in stat.children.keys() { - let child_path = fc.path().join(child_name.as_ref()); + let child_path = fc.path().join(child_name.clone()); let child_stat = self.interface.entry_info(&child_path)?; entries.push(( - U16CString::from_str(child_name).expect("Contains a nul value"), + U16CString::from_str(winify_entry_name(child_name)) + .expect("Contains a nul value"), FileInfo::from(&child_stat), )); } @@ -305,7 +305,7 @@ impl FileSystemContext for FileSystemWrapper { ) -> Result { // `security_descriptor` is not supported yet // `reparse_point` is not supported yet - let path = PathBuf::from(file_name.to_os_string()); + let path = os_path_to_fs_path(file_name)?; Ok(Arc::new(Mutex::new( if create_file_info @@ -441,8 +441,8 @@ impl FileSystemContext for FileSystemWrapper { new_file_name: &U16CStr, replace_if_exists: bool, ) -> Result<(), NTSTATUS> { - let path = PathBuf::from(file_name.to_os_string()); - let new_path = PathBuf::from(new_file_name.to_os_string()); + let path = os_path_to_fs_path(file_name)?; + let new_path = os_path_to_fs_path(new_file_name)?; self.interface .entry_rename(&path, &new_path, replace_if_exists)?; Ok(()) @@ -461,10 +461,10 @@ impl FileSystemContext for FileSystemWrapper { file_name: &U16CStr, _delete_file: bool, ) -> Result<(), NTSTATUS> { - let path = PathBuf::from(file_name.to_os_string()); + let path = os_path_to_fs_path(file_name)?; let stat = self.interface.entry_info(&path)?; - if path == Path::new("/") { + if path.is_root() { return Err(STATUS_RESOURCEMANAGER_READ_ONLY); } else if !stat.children.is_empty() { return Err(STATUS_DIRECTORY_NOT_EMPTY); @@ -475,7 +475,7 @@ impl FileSystemContext for FileSystemWrapper { } pub fn mount( - mountpoint: &Path, + mountpoint: &std::path::Path, interface: T, ) -> anyhow::Result> { let fs_wrapper = FileSystemWrapper::new(interface); @@ -510,3 +510,18 @@ pub fn mount( .map(|fs| FileSystemMounted { fs }) .map_err(|status| anyhow::anyhow!("Failed to init FileSystem {status}")) } + +fn os_path_to_fs_path(path: &U16CStr) -> Result { + // Windows name doesn't allow `/` character, so no need to check if path + // already contains it. + path.to_os_string() + .to_str() + .ok_or(STATUS_OBJECT_NAME_INVALID)? + .replace('\\', "/") + .split('/') + .filter(|&part| !(part.is_empty() || part == ".")) + .map(unwinify_entry_name) + .collect::, _>>() + .map(FsPath::from_parts) + .map_err(|_| STATUS_OBJECT_NAME_INVALID) +} diff --git a/libparsec/crates/platform_mountpoint/src/windows/winify.rs b/libparsec/crates/platform_mountpoint/src/windows/winify.rs index 920448810fe..423efba6305 100644 --- a/libparsec/crates/platform_mountpoint/src/windows/winify.rs +++ b/libparsec/crates/platform_mountpoint/src/windows/winify.rs @@ -21,8 +21,6 @@ const WIN32_RES_NAMES: [&str; 22] = [ static RE: OnceCell = OnceCell::new(); -// TODO: remove me once winify is used -#[allow(dead_code)] pub(crate) fn winify_entry_name(name: &EntryName) -> String { let mut name = name.to_string(); let (prefix, suffix) = name.split_once('.').unwrap_or((&name, "")); @@ -52,8 +50,6 @@ pub(crate) fn winify_entry_name(name: &EntryName) -> String { name } -// TODO: remove me once winify is used -#[allow(dead_code)] pub(crate) fn unwinify_entry_name(name: &str) -> EntryNameResult { // Given / is not allowed, no need to check if path already contains it if name.contains('~') { diff --git a/libparsec/crates/platform_mountpoint/tests/unit/inode.rs b/libparsec/crates/platform_mountpoint/tests/unit/inode.rs index 1b577698e29..8632592fca2 100644 --- a/libparsec/crates/platform_mountpoint/tests/unit/inode.rs +++ b/libparsec/crates/platform_mountpoint/tests/unit/inode.rs @@ -1,8 +1,7 @@ // Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS -use std::path::Path; - use libparsec_tests_lite::prelude::*; +use libparsec_types::FsPath; use super::{Counter, Inode}; use crate::{FileSystemWrapper, MemFS}; @@ -23,7 +22,7 @@ fn init() { .expect("Mutex is poisoned"); // Should contains root only - p_assert_eq!(paths_store.paths_indexed_by_inode[1], Path::new("/")); + assert!(paths_store.paths_indexed_by_inode[1].is_root()); // All paths_indexed_by_inode are used assert!(paths_store.stack_unused_inodes.is_empty()); // Nothing should be opened @@ -33,9 +32,9 @@ fn init() { #[test] fn insert_path() { let manager = FileSystemWrapper::new(MemFS::default()); - let path = Path::new("/foo"); + let path: FsPath = "/foo".parse().unwrap(); - let inode = manager.insert_path(path.into()); + let inode = manager.insert_path(path.clone()); let paths_store = manager .inode_manager @@ -54,17 +53,17 @@ fn insert_path() { assert!(paths_store.stack_unused_inodes.is_empty()); // We opened /foo p_assert_eq!(opened.len(), 1); - p_assert_eq!(opened[path], (Counter(1), inode)); + p_assert_eq!(opened[&path], (Counter(1), inode)); } #[test] fn insert_two_paths() { let manager = FileSystemWrapper::new(MemFS::default()); - let path0 = Path::new("/foo"); - let path1 = Path::new("/bar"); + let path0: FsPath = "/foo".parse().unwrap(); + let path1: FsPath = "/bar".parse().unwrap(); - let ino0 = manager.insert_path(path0.into()); - let ino1 = manager.insert_path(path1.into()); + let ino0 = manager.insert_path(path0.clone()); + let ino1 = manager.insert_path(path1.clone()); let paths_store = manager .inode_manager @@ -85,17 +84,17 @@ fn insert_two_paths() { assert!(paths_store.stack_unused_inodes.is_empty()); // We opened /foo and /bar p_assert_eq!(opened.len(), 2); - p_assert_eq!(opened[path0], (Counter(1), ino0)); - p_assert_eq!(opened[path1], (Counter(1), ino1)); + p_assert_eq!(opened[&path0], (Counter(1), ino0)); + p_assert_eq!(opened[&path1], (Counter(1), ino1)); } #[test] fn insert_same_path_increment_counter() { let manager = FileSystemWrapper::new(MemFS::default()); - let path = Path::new("/foo"); + let path: FsPath = "/foo".parse().unwrap(); - let inode = manager.insert_path(path.into()); - let same_ino = manager.insert_path(path.into()); + let inode = manager.insert_path(path.clone()); + let same_ino = manager.insert_path(path.clone()); p_assert_eq!(inode, same_ino); let paths_store = manager @@ -116,7 +115,7 @@ fn insert_same_path_increment_counter() { assert!(paths_store.stack_unused_inodes.is_empty()); // We opened /foo p_assert_eq!(opened.len(), 1); - p_assert_eq!(opened[path], (Counter(2), inode)); + p_assert_eq!(opened[&path], (Counter(2), inode)); } #[parsec_test] @@ -126,12 +125,12 @@ fn insert_same_path_increment_counter() { #[case(3, 3)] fn remove_path(#[case] opened_x_times: u64, #[case] closed_x_times: u64) { let manager = FileSystemWrapper::new(MemFS::default()); - let path = Path::new("/foo"); + let path: FsPath = "/foo".parse().unwrap(); - let inode = manager.insert_path(path.into()); + let inode = manager.insert_path(path.clone()); for _ in 1..opened_x_times { - manager.insert_path(path.into()); + manager.insert_path(path.clone()); } // Safety: The inode and nlookup are valid @@ -158,7 +157,7 @@ fn remove_path(#[case] opened_x_times: u64, #[case] closed_x_times: u64) { // still opened p_assert_eq!(opened.len(), 1); p_assert_eq!( - opened[path], + opened[&path], (Counter(opened_x_times - closed_x_times), inode) ); } else { @@ -174,14 +173,14 @@ fn remove_path(#[case] opened_x_times: u64, #[case] closed_x_times: u64) { #[test] fn get_path() { let manager = FileSystemWrapper::new(MemFS::default()); - let path = Path::new("/foo"); + let path: FsPath = "/foo".parse().unwrap(); // Safety: The inode exists unsafe { - p_assert_eq!(manager.get_path(Inode::from(1usize)), Path::new("/")); + assert!(manager.get_path(Inode::from(1usize)).is_root()); } - let inode = manager.insert_path(path.into()); + let inode = manager.insert_path(path.clone()); // Safety: The inode exists unsafe { @@ -192,12 +191,12 @@ fn get_path() { #[test] fn rename_path() { let manager = FileSystemWrapper::new(MemFS::default()); - let path = Path::new("/foo"); - let new_path = Path::new("/bar"); + let path: FsPath = "/foo".parse().unwrap(); + let new_path = "/bar".parse().unwrap(); - let inode = manager.insert_path(path.into()); + let inode = manager.insert_path(path.clone()); - manager.rename_path(path, new_path); + manager.rename_path(&path, &new_path); let paths_store = manager .inode_manager @@ -217,21 +216,21 @@ fn rename_path() { assert!(paths_store.stack_unused_inodes.is_empty()); // Opened with the path /bar and same inode p_assert_eq!(opened.len(), 1); - p_assert_eq!(opened[new_path], (Counter(1), inode)); + p_assert_eq!(opened[&new_path], (Counter(1), inode)); } #[test] fn rename_path_non_empty() { let manager = FileSystemWrapper::new(MemFS::default()); - let path = Path::new("/foo"); - let path_child = Path::new("/foo/baz"); - let new_path = Path::new("/bar"); - let new_path_child = Path::new("/bar/baz"); + let path: FsPath = "/foo".parse().unwrap(); + let path_child = "/foo/baz".parse().unwrap(); + let new_path = "/bar".parse().unwrap(); + let new_path_child = "/bar/baz".parse().unwrap(); - let inode = manager.insert_path(path.into()); - let ino_child = manager.insert_path(path_child.into()); + let inode = manager.insert_path(path.clone()); + let ino_child = manager.insert_path(path_child); - manager.rename_path(path, new_path); + manager.rename_path(&path, &new_path); let paths_store = manager .inode_manager @@ -252,20 +251,20 @@ fn rename_path_non_empty() { assert!(paths_store.stack_unused_inodes.is_empty()); // Opened with the path /bar and same inode p_assert_eq!(opened.len(), 2); - p_assert_eq!(opened[new_path], (Counter(1), inode)); - p_assert_eq!(opened[new_path_child], (Counter(1), ino_child)); + p_assert_eq!(opened[&new_path], (Counter(1), inode)); + p_assert_eq!(opened[&new_path_child], (Counter(1), ino_child)); } #[test] fn realloc() { let manager = FileSystemWrapper::new(MemFS::default()); - let path0 = Path::new("/foo"); - let path1 = Path::new("/bar"); - let path2 = Path::new("/baz"); + let path0: FsPath = "/foo".parse().unwrap(); + let path1: FsPath = "/bar".parse().unwrap(); + let path2: FsPath = "/baz".parse().unwrap(); - let ino0 = manager.insert_path(path0.into()); - let ino1 = manager.insert_path(path1.into()); - manager.insert_path(path2.into()); + let ino0 = manager.insert_path(path0.clone()); + let ino1 = manager.insert_path(path1.clone()); + manager.insert_path(path2); // Check that contains 4 paths_indexed_by_inode { @@ -297,7 +296,7 @@ fn realloc() { p_assert_eq!(paths_store.stack_unused_inodes[1], 3); } - manager.insert_path(path0.into()); + manager.insert_path(path0.clone()); { let paths_store = manager @@ -315,7 +314,7 @@ fn realloc() { p_assert_eq!(paths_store.paths_indexed_by_inode.len(), 5); } - manager.insert_path(path1.into()); + manager.insert_path(path1.clone()); { let paths_store = manager diff --git a/libparsec/crates/platform_mountpoint/tests/unit/winfsp.rs b/libparsec/crates/platform_mountpoint/tests/unit/winfsp.rs new file mode 100644 index 00000000000..cfa7c46ba8f --- /dev/null +++ b/libparsec/crates/platform_mountpoint/tests/unit/winfsp.rs @@ -0,0 +1,49 @@ +// Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS + +use chrono::Utc; + +use libparsec_tests_fixtures::{tmp_path, TmpPath}; +use libparsec_tests_lite::{p_assert_eq, parsec_test}; +use libparsec_types::{EntryName, FsPath, VlobID}; + +use crate::{EntryInfo, EntryInfoType, FileSystemMounted, MemFS}; + +#[parsec_test] +fn winify_applied_for_read_dir(tmp_path: TmpPath) { + winfsp_wrs::init().expect("Can't init WinFSP"); + + // Windows can't mount on existing directory + let path = tmp_path.join("mountpoint"); + let memfs = MemFS::default(); + + { + let mut entries = memfs.entries.lock().unwrap(); + let root: FsPath = "/".parse().unwrap(); + + let now = Utc::now(); + for i in 1..10 { + let id = VlobID::default(); + let name: EntryName = format!("COM{i}").parse().unwrap(); + let path = root.join(name.clone()); + + entries.get_mut(&root).unwrap().0.children.insert(name, id); + + entries.insert(path, (EntryInfo::new(id, EntryInfoType::Dir, now), None)); + } + } + + let fs = FileSystemMounted::mount(&path, memfs).unwrap(); + + let mut entries = std::fs::read_dir(path) + .unwrap() + .map(|x| x.unwrap().file_name()) + .collect::>(); + + entries.sort(); + + for (i, entry) in entries.iter().enumerate() { + p_assert_eq!(entry, format!("COM~{:02x}", b'0' + i as u8 + 1).as_str()) + } + + fs.stop(); +} diff --git a/libparsec/crates/platform_mountpoint/tests/winfsp_tests.rs b/libparsec/crates/platform_mountpoint/tests/winfsp_tests.rs index 6e6848793d8..c7406df1de9 100644 --- a/libparsec/crates/platform_mountpoint/tests/winfsp_tests.rs +++ b/libparsec/crates/platform_mountpoint/tests/winfsp_tests.rs @@ -1,11 +1,12 @@ // Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS -#[cfg(target_os = "windows")] +#![cfg(target_os = "windows")] + +use libparsec_platform_mountpoint::{FileSystemMounted, MemFS}; +use std::{path::Path, process::Command}; + #[test] fn winfsp_tests() { - use libparsec_platform_mountpoint::{FileSystemMounted, MemFS}; - use std::{path::Path, process::Command}; - let (tx, rx) = std::sync::mpsc::channel(); let handle = std::thread::spawn(move || { diff --git a/libparsec/crates/types/src/fs_path.rs b/libparsec/crates/types/src/fs_path.rs index 8b35ab199f0..3b69ad6235f 100644 --- a/libparsec/crates/types/src/fs_path.rs +++ b/libparsec/crates/types/src/fs_path.rs @@ -52,6 +52,10 @@ impl EntryName { Ok(()) } } + + pub fn extension(&self) -> Option<&str> { + self.0.split_once('.').map(|(_, ext)| ext) + } } impl std::convert::AsRef for EntryName { @@ -181,7 +185,7 @@ impl FsPath { &self.parts } - pub fn parent(&self) -> FsPath { + pub fn parent(&self) -> Self { if self.parts.is_empty() { self.clone() } else { @@ -214,6 +218,36 @@ impl FsPath { } path } + + pub fn starts_with(&self, path: &Self) -> bool { + if path.parts.len() > self.parts.len() { + return false; + } + + for (self_part, part) in self.parts().iter().zip(path.parts()) { + if self_part != part { + return false; + } + } + + true + } + + /// Use `dest` for allocation and return it. + pub fn replace_parent(&self, offset: usize, mut dest: Self) -> Self { + if self.parts.len() > offset { + dest.parts.extend_from_slice(&self.parts[offset..]); + } + dest + } + + pub fn extension(&self) -> Option<&str> { + self.name().and_then(|name| name.extension()) + } + + pub fn from_parts(parts: Vec) -> Self { + Self { parts } + } } impl std::fmt::Debug for FsPath {