From e7e641e8c41bf9589cab705705e7b2c4facddbc5 Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Tue, 5 May 2026 10:33:20 +0100 Subject: [PATCH 1/7] Stubbing out util filesystems --- Cargo.lock | 12 + crates/ninep/Cargo.toml | 9 +- crates/ninep/examples/client.rs | 2 +- crates/ninep/examples/local_proxy.rs | 29 ++ crates/ninep/examples/ramfs.rs | 25 ++ crates/ninep/src/fs/mod.rs | 11 +- crates/ninep/src/fs/simple.rs | 35 ++- crates/ninep/src/lib.rs | 2 + crates/ninep/src/util/hook.rs | 223 ++++++++++++++ crates/ninep/src/util/local_proxy.rs | 439 +++++++++++++++++++++++++++ crates/ninep/src/util/mod.rs | 6 + crates/ninep/src/util/ram.rs | 126 ++++++++ crates/ninep/src/util/read_only.rs | 110 +++++++ 13 files changed, 1015 insertions(+), 14 deletions(-) create mode 100644 crates/ninep/examples/local_proxy.rs create mode 100644 crates/ninep/examples/ramfs.rs create mode 100644 crates/ninep/src/util/hook.rs create mode 100644 crates/ninep/src/util/local_proxy.rs create mode 100644 crates/ninep/src/util/mod.rs create mode 100644 crates/ninep/src/util/ram.rs create mode 100644 crates/ninep/src/util/read_only.rs diff --git a/Cargo.lock b/Cargo.lock index fadade63..5c5af3c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -778,12 +778,14 @@ dependencies = [ name = "ninep" version = "0.6.0" dependencies = [ + "assert_fs", "bitflags 2.11.1", "jiff", "lexopt", "simple_coro", "simple_test_case", "tokio", + "uzers", ] [[package]] @@ -1421,6 +1423,16 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" +[[package]] +name = "uzers" +version = "0.12.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b8275fb1afee25b4111d2dc8b5c505dbbc4afd0b990cb96deb2d88bff8be18d" +dependencies = [ + "libc", + "log", +] + [[package]] name = "valuable" version = "0.1.1" diff --git a/crates/ninep/Cargo.toml b/crates/ninep/Cargo.toml index f1ebfbb7..ff1c0a71 100644 --- a/crates/ninep/Cargo.toml +++ b/crates/ninep/Cargo.toml @@ -14,16 +14,19 @@ include = [ ] [features] -default = ["tokio"] +default = ["tokio", "util_fs"] tokio = ["dep:tokio"] +util_fs = ["dep:uzers"] [dependencies] bitflags = "2" -tokio = { version = "1", features = ["macros", "net", "io-util", "rt", "sync"], optional = true } +jiff = "0.2" simple_coro = "0.1.5" -jiff = "0.2.24" +tokio = { version = "1", features = ["macros", "net", "io-util", "rt", "sync"], optional = true } +uzers = { version = "0.12", optional = true } [dev-dependencies] +assert_fs = "1.1" lexopt = "0.3.2" simple_test_case = "1" tokio = { version = "1", features = ["macros", "net", "io-util", "rt", "sync", "time", "rt-multi-thread", "net", "io-util"] } diff --git a/crates/ninep/examples/client.rs b/crates/ninep/examples/client.rs index 4d098416..4cc99097 100644 --- a/crates/ninep/examples/client.rs +++ b/crates/ninep/examples/client.rs @@ -145,7 +145,7 @@ impl Args { }; let (socket_path, path) = match path.split_once('/') { - Some((ns, path)) => (ns, path), + Some((socket_path, path)) => (socket_path, path), None => (path.as_str(), "/"), }; diff --git a/crates/ninep/examples/local_proxy.rs b/crates/ninep/examples/local_proxy.rs new file mode 100644 index 00000000..bb215901 --- /dev/null +++ b/crates/ninep/examples/local_proxy.rs @@ -0,0 +1,29 @@ +use ninep::{ + sync::server::Server, + util::{hook::HookFs, local_proxy::LocalProxyFs}, +}; +use std::env::{args, current_dir}; + +fn main() { + let path = args().nth(1).expect("must provide a path to bind to"); + let chatty = args().nth(2).as_deref() == Some("--chatty"); + + let fs = HookFs::new(LocalProxyFs::new(path).unwrap(), move |op| { + if chatty { + println!("{op:?}"); + } + + Ok(()) + }); + + let s = Server::new(fs); + let socket_path = current_dir().unwrap().join("proxy-fs"); + + println!( + "starting local-proxy-fs file server at {}", + socket_path.display() + ); + if s.serve_socket_with_custom_path(socket_path).join().is_err() { + eprintln!("server thread died"); + } +} diff --git a/crates/ninep/examples/ramfs.rs b/crates/ninep/examples/ramfs.rs new file mode 100644 index 00000000..53303e10 --- /dev/null +++ b/crates/ninep/examples/ramfs.rs @@ -0,0 +1,25 @@ +//! A simple in-memory filesystem that allows clients to read and write arbitrary file paths. +use ninep::{ + sync::server::Server, + util::{hook::HookFs, ram::RamFs}, +}; +use std::env::{args, current_dir}; + +fn main() { + let chatty = args().nth(1).as_deref() == Some("--chatty"); + let fs = HookFs::new(RamFs::new(env!("USER"), "group"), move |op| { + if chatty { + println!("{op:?}"); + } + + Ok(()) + }); + + let s = Server::new(fs); + let socket_path = current_dir().unwrap().join("ramfs"); + + println!("starting ram-fs file server at {}", socket_path.display()); + if s.serve_socket_with_custom_path(socket_path).join().is_err() { + eprintln!("server thread died"); + } +} diff --git a/crates/ninep/src/fs/mod.rs b/crates/ninep/src/fs/mod.rs index f8522287..4e49f803 100644 --- a/crates/ninep/src/fs/mod.rs +++ b/crates/ninep/src/fs/mod.rs @@ -204,7 +204,7 @@ impl Mode { } fn is_allowed(&self, read: bool, write: bool, exec: bool) -> bool { - let m = Mode::new(self.bits() & 0x03); // Mask off the additional truncate and remove bits + let m = self.base(); // Mask off the additional truncate and remove bits (m == Mode::READ && read) || (m == Mode::WRITE && write) @@ -213,12 +213,17 @@ impl Mode { || false } + /// The base [Mode] without [Mode::TRUNCATE] and [Mode::REMOVE_ON_CLOSE] bits. + pub fn base(&self) -> Mode { + Mode::new(self.bits() & 0x03) + } + pub(crate) fn allows_read(&self) -> bool { - *self == Mode::READ || *self == Mode::READ_WRITE + self.base() == Mode::READ || self.base() == Mode::READ_WRITE } pub(crate) fn allows_write(&self) -> bool { - *self == Mode::WRITE || *self == Mode::READ_WRITE + self.base() == Mode::WRITE || self.base() == Mode::READ_WRITE } } diff --git a/crates/ninep/src/fs/simple.rs b/crates/ninep/src/fs/simple.rs index 743e42a4..b426f89e 100644 --- a/crates/ninep/src/fs/simple.rs +++ b/crates/ninep/src/fs/simple.rs @@ -123,13 +123,23 @@ where /// A simple file implementation for use in a [FileTree]. #[derive(Debug, Clone)] pub struct File { - stat: Stat, + parent: Option, + /// The stat associated with this [File] node. + pub stat: Stat, /// User defined additional data per [File] node. pub aux: T, } impl File { - fn new(qid: Qid, name: &str, owner: &str, group: &str, perms: Perm, aux: T) -> Self { + fn new( + qid: Qid, + name: &str, + owner: &str, + group: &str, + perms: Perm, + aux: T, + parent: Option, + ) -> Self { File { stat: Stat { qid, @@ -143,12 +153,15 @@ impl File { last_modified_by: owner.into(), }, aux, + parent, } } - /// The [Stat] for this file. - pub fn stat(&self) -> &Stat { - &self.stat + /// The `qid` of the parent node for this file. + /// + /// Returns [None] for the root node. + pub fn parent(&self) -> Option { + self.parent } /// Attempt to apply a [WStat] to the [Stat] of this file. @@ -179,7 +192,7 @@ where T: Send + Sync + 'static, { fn new(owner: &str, group: &str, perms: Perm, aux: T) -> Self { - let root = File::new(Qid::dir(0), "/", owner, group, perms, aux); + let root = File::new(Qid::dir(0), "/", owner, group, perms, aux, None); Self { entries: BTreeMap::from_iter([(0, root)]), @@ -231,7 +244,15 @@ where self.entries.insert( qid_path, - File::new(qid, name, &pstat.owner, &pstat.group, perms, aux), + File::new( + qid, + name, + &pstat.owner, + &pstat.group, + perms, + aux, + Some(parent), + ), ); self.children .entry(pstat.qid.path) diff --git a/crates/ninep/src/lib.rs b/crates/ninep/src/lib.rs index 5f1ea62c..67a0c212 100644 --- a/crates/ninep/src/lib.rs +++ b/crates/ninep/src/lib.rs @@ -14,6 +14,8 @@ pub mod fs; pub mod sansio; pub mod sync; +#[cfg(feature = "util_fs")] +pub mod util; #[cfg(feature = "tokio")] pub mod tokio; diff --git a/crates/ninep/src/util/hook.rs b/crates/ninep/src/util/hook.rs new file mode 100644 index 00000000..60242c45 --- /dev/null +++ b/crates/ninep/src/util/hook.rs @@ -0,0 +1,223 @@ +//! A proxy filesystem that runs a user provided hook function before each operation. +use crate::{ + Result, + fs::{IoUnit, Mode, Perm, Qid, Stat, WStat}, + sync::server::{ClientId, ReadOutcome, Serve9p}, +}; +use std::{fmt, sync::Arc}; + +type Hook = Arc Fn(FsOp<'a>) -> Result<()> + Send + Sync>; + +/// A filesystem proxy that runs a user provided hook before each operation before deferring to an +/// inner filesystem implementation. +#[derive(Clone)] +pub struct HookFs +where + T: Serve9p, +{ + inner: T, + hook: Hook, +} + +impl HookFs +where + T: Serve9p, +{ + /// Create a new [HookFs] with the provided hook function. + pub fn new(inner: T, hook: F) -> Self + where + F: for<'a> Fn(FsOp<'a>) -> Result<()> + Send + Sync + 'static, + { + Self { + inner, + hook: Arc::new(hook), + } + } +} + +impl fmt::Debug for HookFs +where + T: Serve9p + fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("HookFs") + .field("inner", &self.inner) + .finish_non_exhaustive() + } +} + +impl Serve9p for HookFs +where + T: Serve9p, +{ + fn open(&self, cid: ClientId, qid: u64, mode: Mode, uname: &str) -> Result { + (self.hook)(FsOp::Open { + cid, + qid, + mode, + uname, + })?; + + self.inner.open(cid, qid, mode, uname) + } + + fn walk_one(&self, cid: ClientId, parent_qid: u64, child: &str, uname: &str) -> Result { + (self.hook)(FsOp::WalkOne { + cid, + parent_qid, + child, + uname, + })?; + + self.inner.walk_one(cid, parent_qid, child, uname) + } + + fn read( + &self, + cid: ClientId, + qid: u64, + offset: usize, + count: usize, + uname: &str, + ) -> Result { + (self.hook)(FsOp::Read { + cid, + qid, + offset, + count, + uname, + })?; + + self.inner.read(cid, qid, offset, count, uname) + } + + fn read_dir(&self, cid: ClientId, qid: u64, uname: &str) -> Result> { + (self.hook)(FsOp::ReadDir { cid, qid, uname })?; + + self.inner.read_dir(cid, qid, uname) + } + + fn write( + &self, + cid: ClientId, + qid: u64, + offset: usize, + data: Vec, + uname: &str, + ) -> Result { + (self.hook)(FsOp::Write { + cid, + qid, + offset, + n_bytes: data.len(), + uname, + })?; + + self.inner.write(cid, qid, offset, data, uname) + } + + fn stat(&self, cid: ClientId, qid: u64, uname: &str) -> Result { + (self.hook)(FsOp::Stat { cid, qid, uname })?; + + self.inner.stat(cid, qid, uname) + } + + fn write_stat(&self, cid: ClientId, qid: u64, wstat: WStat, uname: &str) -> Result<()> { + (self.hook)(FsOp::WriteStat { + cid, + qid, + wstat: &wstat, + uname, + })?; + + self.inner.write_stat(cid, qid, wstat, uname) + } + + fn remove(&self, cid: ClientId, qid: u64, uname: &str) -> Result<()> { + (self.hook)(FsOp::Remove { cid, qid, uname })?; + + self.inner.remove(cid, qid, uname) + } + + fn create( + &self, + cid: ClientId, + parent: u64, + name: &str, + perm: Perm, + mode: Mode, + uname: &str, + ) -> Result<(Qid, IoUnit)> { + (self.hook)(FsOp::Create { + cid, + parent, + name, + perm, + mode, + uname, + })?; + + self.inner.create(cid, parent, name, perm, mode, uname) + } +} + +/// Events emitted for each filesystem operation. +#[expect(missing_docs)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FsOp<'a> { + WalkOne { + cid: ClientId, + parent_qid: u64, + child: &'a str, + uname: &'a str, + }, + Open { + cid: ClientId, + qid: u64, + mode: Mode, + uname: &'a str, + }, + Read { + cid: ClientId, + qid: u64, + offset: usize, + count: usize, + uname: &'a str, + }, + ReadDir { + cid: ClientId, + qid: u64, + uname: &'a str, + }, + Write { + cid: ClientId, + qid: u64, + offset: usize, + n_bytes: usize, + uname: &'a str, + }, + Stat { + cid: ClientId, + qid: u64, + uname: &'a str, + }, + WriteStat { + cid: ClientId, + qid: u64, + wstat: &'a WStat, + uname: &'a str, + }, + Remove { + cid: ClientId, + qid: u64, + uname: &'a str, + }, + Create { + cid: ClientId, + parent: u64, + name: &'a str, + perm: Perm, + mode: Mode, + uname: &'a str, + }, +} diff --git a/crates/ninep/src/util/local_proxy.rs b/crates/ninep/src/util/local_proxy.rs new file mode 100644 index 00000000..59a4fe6b --- /dev/null +++ b/crates/ninep/src/util/local_proxy.rs @@ -0,0 +1,439 @@ +//! A 9p filesystem proxy over a local directory. +use crate::{ + Result, + fs::{FileTree, FileType, IoUnit, Mode, Perm, Qid, Stat, Timestamp, WStat}, + sansio::server::{E_PERMISSION_DENIED, E_UNKNOWN_FILE}, + sync::server::{ClientId, ReadOutcome, Serve9p}, +}; +use std::{ + collections::BTreeMap, + fs::{self, Metadata, OpenOptions}, + io::{self, Read, Seek, SeekFrom, Write}, + os::unix::fs::{MetadataExt, PermissionsExt}, + path::{Path, PathBuf}, +}; +use uzers::{get_group_by_gid, get_user_by_uid}; + +const DEFAULT_IOUNIT: IoUnit = 8168; + +/// The metadata we store per-node in the file tree +#[derive(Debug, Clone)] +struct PathMeta { + path: PathBuf, + meta: Metadata, +} + +/// A filesystem proxy that exposes a local directory over 9p. +#[derive(Debug)] +pub struct LocalProxyFs { + root: PathBuf, + ft: FileTree, + iounit: IoUnit, +} + +impl LocalProxyFs { + /// Construct a new [ProxyFs] pointed at the provided root directory + pub fn new(root: impl Into) -> io::Result { + let root = fs::canonicalize(root.into())?; + let root_meta = fs::metadata(&root)?; + if !root_meta.is_dir() { + return Err(io::Error::other("proxy root must be a directory")); + } + + let (owner, group) = owner_and_group_from_meta(&root_meta); + let perms = perms_from_meta(&root_meta); + let pm = PathMeta { + path: root.clone(), + meta: root_meta, + }; + + let ft = FileTree::new(&owner, &group, perms, pm); + + let fs = Self { + root, + ft, + iounit: DEFAULT_IOUNIT, + }; + + fs.try_sync_dir(0).map_err(io::Error::other)?; + + Ok(fs) + } + + /// On-disk path and metadata for `qid`. + /// + /// Panics if called for an unknown qid + fn meta_for_qid(&self, qid: u64) -> PathMeta { + self.ft.with_file(qid, |f| f.aux.clone()).unwrap() + } + + /// On-disk path for `qid`. + /// + /// Panics if called for an unknown qid + fn path_for_qid(&self, qid: u64) -> PathBuf { + self.ft.with_file(qid, |f| f.aux.path.clone()).unwrap() + } + + /// Parent qid for `qid`. + /// + /// Panics if called for an unknown qid or the root node. + fn parent_qid(&self, qid: u64) -> u64 { + self.ft.with_file(qid, |f| f.parent().unwrap()).unwrap() + } + + /// Check to see if the given node has been modified since we cached metadata for it. + /// If we are unable to read metadata from disk, prune this node. + fn modified_since_cache_or_prune(&self, qid: u64) -> Result { + let modified_since_cache = |qid: u64| -> io::Result { + let cached = self.meta_for_qid(qid); + let t_cached = cached.meta.modified()?; + let current = fs::metadata(&cached.path)?; + let t_current = current.modified()?; + + Ok(t_current != t_cached) + }; + + match modified_since_cache(qid) { + Ok(opt) => Ok(opt), + Err(e) => { + self.ft.remove(qid); + Err(e.to_string()) + } + } + } + + // TODO: re-run perm checks if perms are now different + fn try_sync_dir(&self, qid: u64) -> Result<()> { + let path = self.meta_for_qid(qid).path; + let mut on_disk = self.on_disk_entries_for(&path).map_err(|e| e.to_string())?; + + // Update known cache entries and prune entries that are no longer present on disk + for stat in self.ft.read_dir(qid)?.iter() { + let qid = stat.qid.path; + + match on_disk.remove(&stat.name) { + Some(pm) => { + _ = self.ft.with_file_mut(qid, |f| { + let (owner, group) = owner_and_group_from_meta(&pm.meta); + f.stat.owner = owner; + f.stat.group = group; + f.stat.n_bytes = pm.meta.len(); + f.stat.perms = perms_from_meta(&pm.meta); + f.stat.last_accessed = Timestamp::from_second(pm.meta.atime()).unwrap(); + f.stat.last_modified = Timestamp::from_second(pm.meta.mtime()).unwrap(); + f.aux = pm; + }) + } + None => self.ft.remove(qid), + } + } + + // Anything remaining in on_disk is something new so insert it + for (name, pm) in on_disk.into_iter() { + let ty = if pm.meta.is_dir() { + FileType::DIRECTORY + } else { + FileType::FILE + }; + let perms = perms_from_meta(&pm.meta); + let meta = pm.meta.clone(); + let qid = self.ft.try_add_node(qid, &name, perms, ty, pm)?; + self.try_sync_file_with_meta(qid.path, meta)?; + } + + Ok(()) + } + + fn try_sync_file(&self, qid: u64) -> Result<()> { + let path = self.path_for_qid(qid); + let meta = self + .meta_if_under_root(&path) + .ok_or_else(|| E_UNKNOWN_FILE.to_string())?; + + self.try_sync_file_with_meta(qid, meta) + } + + // TODO: re-run perm checks if perms are now different (will require Mode) + fn try_sync_file_with_meta(&self, qid: u64, meta: Metadata) -> Result<()> { + let path = self.path_for_qid(qid); + let name = path + .file_name() + .ok_or_else(|| E_UNKNOWN_FILE.to_string())? + .to_string_lossy() + .to_string(); + + let (owner, group) = owner_and_group_from_meta(&meta); + + self.ft.with_file_mut(qid, |f| { + f.stat.name = name; + f.stat.owner = owner; + f.stat.group = group; + f.stat.n_bytes = meta.len(); + f.stat.perms = perms_from_meta(&meta); + f.stat.last_accessed = Timestamp::from_second(meta.atime()).unwrap(); + f.stat.last_modified = Timestamp::from_second(meta.mtime()).unwrap(); + f.aux = PathMeta { path, meta }; + }) + } + + fn on_disk_entries_for(&self, dir_path: &Path) -> io::Result> { + let mut m = BTreeMap::new(); + + for entry in fs::read_dir(dir_path)? { + let entry = entry?; + let name = entry.file_name().to_string_lossy().to_string(); + let path = entry.path(); + + // Only include entries that resolve to being under our root + if let Some(meta) = self.meta_if_under_root(&path) { + m.insert(name, PathMeta { path, meta }); + }; + } + + Ok(m) + } + + fn meta_if_under_root(&self, path: &Path) -> Option { + let canonicalized = fs::canonicalize(path).ok()?; + if !canonicalized.starts_with(&self.root) { + return None; + } + + fs::metadata(path).ok() + } +} + +impl Serve9p for LocalProxyFs { + fn open(&self, _cid: ClientId, qid: u64, _mode: Mode, _uname: &str) -> Result { + if self.modified_since_cache_or_prune(qid)? { + self.try_sync_file(qid)?; + } + + Ok(self.iounit) + } + + fn walk_one(&self, _cid: ClientId, parent_qid: u64, child: &str, _uname: &str) -> Result { + if self.modified_since_cache_or_prune(parent_qid)? { + self.try_sync_dir(parent_qid)?; + } + + self.ft.walk_one(parent_qid, child) + } + + fn read( + &self, + _cid: ClientId, + qid: u64, + offset: usize, + count: usize, + _uname: &str, + ) -> Result { + if self.modified_since_cache_or_prune(qid)? { + self.try_sync_file(qid)?; + } + + let path = self.path_for_qid(qid); + + let mut f = OpenOptions::new() + .read(true) + .open(&path) + .map_err(|e| e.to_string())?; + f.seek(SeekFrom::Start(offset as u64)) + .map_err(|e| e.to_string())?; + + let mut buf = vec![0; count]; + let n = f.read(&mut buf).map_err(|e| e.to_string())?; + buf.truncate(n); + + Ok(ReadOutcome::Immediate(buf)) + } + + fn read_dir(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result> { + if self.modified_since_cache_or_prune(qid)? { + self.try_sync_dir(qid)?; + } + + self.ft.read_dir(qid) + } + + fn write( + &self, + _cid: ClientId, + qid: u64, + offset: usize, + data: Vec, + _uname: &str, + ) -> Result { + if self.modified_since_cache_or_prune(qid)? { + self.try_sync_file(qid)?; + } + + let path = self.path_for_qid(qid); + + let mut f = OpenOptions::new() + .write(true) + .open(&path) + .map_err(|e| e.to_string())?; + f.seek(SeekFrom::Start(offset as u64)) + .map_err(|e| e.to_string())?; + + f.write(data.as_slice()).map_err(|e| e.to_string()) + } + + fn stat(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result { + if self.modified_since_cache_or_prune(qid)? { + let pm = self.meta_for_qid(qid); + if pm.meta.is_dir() { + self.try_sync_dir(qid)?; + } else { + self.try_sync_file(qid)?; + } + } + + self.ft.stat(qid) + } + + fn write_stat(&self, _cid: ClientId, qid: u64, wstat: WStat, _uname: &str) -> Result<()> { + if wstat.last_accessed.is_some() + || wstat.last_modified.is_some() + || wstat.group.is_some() + || wstat.last_modified_by.is_some() + || qid == 0 + { + return Err(E_PERMISSION_DENIED.to_string()); + } + + let pm = self.meta_for_qid(qid); + if pm.meta.is_dir() { + self.try_sync_dir(qid)?; + } else { + self.try_sync_file(qid)?; + } + + if let Some(perms) = wstat.perms { + let mode = perms.bits() & 0o777; + let mut permissions = fs::metadata(&pm.path) + .map_err(|e| e.to_string())? + .permissions(); + permissions.set_mode(mode); + fs::set_permissions(&pm.path, permissions).map_err(|e| e.to_string())?; + } + + if let Some(size) = wstat.n_bytes { + if pm.meta.is_dir() { + return Err(E_PERMISSION_DENIED.to_string()); + } + + let f = OpenOptions::new() + .write(true) + .open(&pm.path) + .map_err(|e| e.to_string())?; + f.set_len(size).map_err(|e| e.to_string())?; + } + + if let Some(name) = wstat.name { + let parent = pm.path.parent().unwrap(); + let new_path = parent.join(&name); + fs::rename(&pm.path, &new_path).map_err(|e| e.to_string())?; + + self.try_sync_dir(self.parent_qid(qid))?; + } + + Ok(()) + } + + fn remove(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result<()> { + if qid == 0 { + return Err(E_PERMISSION_DENIED.to_string()); + } + + if self.modified_since_cache_or_prune(qid)? { + self.try_sync_file(qid)?; + } + + let is_dir = self.ft.with_file(qid, |f| f.aux.meta.is_dir())?; + let path = self.path_for_qid(qid); + + if is_dir { + fs::remove_dir(&path).map_err(|e| e.to_string())?; + } else { + fs::remove_file(&path).map_err(|e| e.to_string())?; + } + + self.ft.remove(qid); + + Ok(()) + } + + fn create( + &self, + _cid: ClientId, + parent: u64, + name: &str, + perm: Perm, + _mode: Mode, + _uname: &str, + ) -> Result<(Qid, IoUnit)> { + if self.modified_since_cache_or_prune(parent)? { + self.try_sync_dir(parent)?; + } + + let parent_path = self.path_for_qid(parent); + let path = parent_path.join(name); + if path.exists() { + return Err("file already exists".to_string()); + } + + let is_dir = perm.contains(Perm::DIRECTORY); + if is_dir { + fs::create_dir(&path).map_err(|e| e.to_string())?; + } else { + OpenOptions::new() + .create_new(true) + .write(true) + .open(&path) + .map_err(|e| e.to_string())?; + } + + let mode_bits = perm.bits() & 0o777; + let mut permissions = fs::metadata(&path) + .map_err(|e| e.to_string())? + .permissions(); + permissions.set_mode(mode_bits); + fs::set_permissions(&path, permissions).map_err(|e| e.to_string())?; + + // Sync to allow our normal logic to pick up the new file details and create the qid + self.try_sync_dir(parent)?; + + let qid = self.ft.walk_one(parent, name)?; + + Ok((qid, self.iounit)) + } +} + +fn perms_from_meta(meta: &Metadata) -> Perm { + let mut perms = Perm::new(meta.permissions().mode() & 0o777); + if meta.is_dir() { + perms |= Perm::DIRECTORY; + } + + perms +} + +fn owner_and_group_from_meta(meta: &Metadata) -> (String, String) { + let owner = get_user_by_uid(meta.uid()) + .map(|u| u.name().to_string_lossy().into_owned()) + .unwrap_or_else(|| "unknown".into()); + + let group = get_group_by_gid(meta.gid()) + .map(|g| g.name().to_string_lossy().into_owned()) + .unwrap_or_else(|| "unknown".into()); + + (owner, group) +} + +#[cfg(test)] +mod tests { + use super::*; + use assert_fs::TempDir; +} diff --git a/crates/ninep/src/util/mod.rs b/crates/ninep/src/util/mod.rs new file mode 100644 index 00000000..3645afa4 --- /dev/null +++ b/crates/ninep/src/util/mod.rs @@ -0,0 +1,6 @@ +//! General purpose utility filesystem implementations. + +pub mod hook; +pub mod local_proxy; +pub mod ram; +pub mod read_only; diff --git a/crates/ninep/src/util/ram.rs b/crates/ninep/src/util/ram.rs new file mode 100644 index 00000000..c5f3aff6 --- /dev/null +++ b/crates/ninep/src/util/ram.rs @@ -0,0 +1,126 @@ +//! A simple in-memory filesystem implementation. +use crate::{ + Result, + fs::{FileTree, FileType, IoUnit, Mode, Perm, Qid, Stat, WStat}, + sansio::server::E_PERMISSION_DENIED, + sync::server::{ClientId, ReadOutcome, Serve9p}, +}; +use std::cmp::min; + +const DEFAULT_IOUNIT: IoUnit = 8168; + +/// A simple in-memory file server implementation. +#[derive(Debug, Clone)] +pub struct RamFs { + ft: FileTree>, + iounit: IoUnit, +} + +impl RamFs { + /// Construct a [RamFs] with default [permissions][Perm] and [IoUnit]. + pub fn new(owner: &str, group: &str) -> Self { + Self::new_with_base_perms_and_iounit( + owner, + group, + Perm::any_read() | Perm::any_write() | Perm::any_exec(), + DEFAULT_IOUNIT, + ) + } + + /// Construct a [RamFs] with the provided [permissions][Perm] and [IoUnit]. + pub fn new_with_base_perms_and_iounit( + owner: &str, + group: &str, + perms: Perm, + iounit: IoUnit, + ) -> Self { + RamFs { + ft: FileTree::new(owner, group, perms, Vec::new()), + iounit, + } + } +} + +impl Serve9p for RamFs { + fn open(&self, _cid: ClientId, _qid: u64, _mode: Mode, _uname: &str) -> Result { + Ok(self.iounit) + } + + fn walk_one(&self, _cid: ClientId, parent_qid: u64, child: &str, _uname: &str) -> Result { + self.ft.walk_one(parent_qid, child) + } + + fn read( + &self, + _cid: ClientId, + qid: u64, + offset: usize, + count: usize, + _uname: &str, + ) -> Result { + let data: Vec = self.ft.with_file(qid, |f| { + if offset > f.aux.len() { + Vec::new() + } else { + let to = min(offset + count, f.aux.len()); + f.aux[offset..to].to_vec() + } + })?; + + Ok(ReadOutcome::Immediate(data)) + } + + fn read_dir(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result> { + self.ft.read_dir(qid) + } + + fn write( + &self, + _cid: ClientId, + qid: u64, + offset: usize, + data: Vec, + _uname: &str, + ) -> Result { + self.ft.with_file_mut(qid, |f| { + let n = data.len(); + if offset + data.len() > f.aux.len() { + f.aux.resize(offset + data.len(), 0); + } + f.aux[offset..offset + data.len()].copy_from_slice(data.as_slice()); + + n + }) + } + + fn stat(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result { + self.ft.stat(qid) + } + + fn write_stat(&self, _cid: ClientId, qid: u64, wstat: WStat, _uname: &str) -> Result<()> { + self.ft + .with_file_mut(qid, |f| f.try_apply_wstat(wstat))? + .map_err(|_| E_PERMISSION_DENIED.to_string()) + } + + fn remove(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result<()> { + self.ft.remove(qid); + + Ok(()) + } + + fn create( + &self, + _cid: ClientId, + parent: u64, + name: &str, + perm: Perm, + _mode: Mode, + _uname: &str, + ) -> Result<(Qid, IoUnit)> { + let ty = FileType::from(perm); + let qid = self.ft.try_add_node(parent, name, perm, ty, Vec::new())?; + + Ok((qid, self.iounit)) + } +} diff --git a/crates/ninep/src/util/read_only.rs b/crates/ninep/src/util/read_only.rs new file mode 100644 index 00000000..cb000938 --- /dev/null +++ b/crates/ninep/src/util/read_only.rs @@ -0,0 +1,110 @@ +//! A proxy filesystem that supports toggling on a read-only mode that auto-denies mutating +//! operations. +use crate::{ + Result, + fs::{IoUnit, Mode, Perm, Qid, Stat, WStat}, + sansio::server::E_PERMISSION_DENIED, + sync::server::{ClientId, ReadOutcome, Serve9p}, +}; + +/// A proxy filesystem that supports toggling on a read-only mode that auto-denies mutating +/// operations. +#[derive(Debug, Clone)] +pub struct ReadOnlyFs +where + T: Serve9p, +{ + inner: T, + read_only: bool, +} + +impl ReadOnlyFs +where + T: Serve9p, +{ + /// Create a new [ReadOnlyFs] that starts in read-only mode. + pub fn new(inner: T) -> Self { + Self { + inner, + read_only: true, + } + } + + /// Set read-only mode to be on or off. + pub fn read_only(&mut self, read_only: bool) { + self.read_only = read_only; + } +} + +impl Serve9p for ReadOnlyFs +where + T: Serve9p, +{ + fn open(&self, cid: ClientId, qid: u64, mode: Mode, uname: &str) -> Result { + self.inner.open(cid, qid, mode, uname) + } + + fn walk_one(&self, cid: ClientId, parent_qid: u64, child: &str, uname: &str) -> Result { + self.inner.walk_one(cid, parent_qid, child, uname) + } + + fn read( + &self, + cid: ClientId, + qid: u64, + offset: usize, + count: usize, + uname: &str, + ) -> Result { + self.inner.read(cid, qid, offset, count, uname) + } + + fn read_dir(&self, cid: ClientId, qid: u64, uname: &str) -> Result> { + self.inner.read_dir(cid, qid, uname) + } + + fn write( + &self, + cid: ClientId, + qid: u64, + offset: usize, + data: Vec, + uname: &str, + ) -> Result { + self.inner.write(cid, qid, offset, data, uname) + } + + fn stat(&self, cid: ClientId, qid: u64, uname: &str) -> Result { + let mut stat = self.inner.stat(cid, qid, uname)?; + if self.read_only { + stat.perms + .remove(Perm::OWNER_WRITE | Perm::GROUP_WRITE | Perm::OTHER_WRITE); + } + + Ok(stat) + } + + fn write_stat(&self, cid: ClientId, qid: u64, wstat: WStat, uname: &str) -> Result<()> { + self.inner.write_stat(cid, qid, wstat, uname) + } + + fn remove(&self, cid: ClientId, qid: u64, uname: &str) -> Result<()> { + self.inner.remove(cid, qid, uname) + } + + fn create( + &self, + cid: ClientId, + parent: u64, + name: &str, + perm: Perm, + mode: Mode, + uname: &str, + ) -> Result<(Qid, IoUnit)> { + if self.read_only && mode.allows_write() { + return Err(E_PERMISSION_DENIED.to_string()); + } + + self.inner.create(cid, parent, name, perm, mode, uname) + } +} From 12369d84c0497793a87fa027e4fcbf2a2fc8e288 Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Wed, 6 May 2026 13:55:55 +0100 Subject: [PATCH 2/7] Add tests for RamFs --- crates/ninep/src/fs/simple.rs | 23 +++ crates/ninep/src/util/ram.rs | 348 +++++++++++++++++++++++++++++++--- 2 files changed, 343 insertions(+), 28 deletions(-) diff --git a/crates/ninep/src/fs/simple.rs b/crates/ninep/src/fs/simple.rs index b426f89e..071b2f2c 100644 --- a/crates/ninep/src/fs/simple.rs +++ b/crates/ninep/src/fs/simple.rs @@ -64,6 +64,29 @@ where self.with_nodes_mut(|nodes| nodes.remove(qid)) } + /// Attempt to map a path within this file tree to a qid. + /// + /// Returns `Some(qid)` for a known path, otherwise `None`. + pub fn qid_for_path(&self, path: &str) -> Option { + if !path.starts_with('/') { + return None; + } + + let mut qid = 0; + + // Need to skip the empty string from the leading slash + for elem in path.split('/').skip(1) { + qid = self.walk_one(qid, elem).ok()?.path; + } + + Some(qid) + } + + /// Whether or not this file tree contains the given qid + pub fn contains_qid(&self, qid: u64) -> bool { + self.with_nodes(|nodes| nodes.entries.contains_key(&qid)) + } + /// Run a closure with access to the [File] associated with the given `qid`. pub fn with_file(&self, qid: u64, f: F) -> Result where diff --git a/crates/ninep/src/util/ram.rs b/crates/ninep/src/util/ram.rs index c5f3aff6..364c9e97 100644 --- a/crates/ninep/src/util/ram.rs +++ b/crates/ninep/src/util/ram.rs @@ -2,7 +2,7 @@ use crate::{ Result, fs::{FileTree, FileType, IoUnit, Mode, Perm, Qid, Stat, WStat}, - sansio::server::E_PERMISSION_DENIED, + sansio::server::{E_PERMISSION_DENIED, E_UNKNOWN_FILE}, sync::server::{ClientId, ReadOutcome, Serve9p}, }; use std::cmp::min; @@ -39,25 +39,30 @@ impl RamFs { iounit, } } + + /// Obtain a copy of the [FileTree] within this RamFs. + /// + /// [FileTree] internally stores its data within an `Arc`, so this can be used to interact with + /// the tree directly. + pub fn file_tree(&self) -> FileTree> { + self.ft.clone() + } } impl Serve9p for RamFs { - fn open(&self, _cid: ClientId, _qid: u64, _mode: Mode, _uname: &str) -> Result { - Ok(self.iounit) + fn open(&self, qid: u64, _mode: Mode, _cid: ClientId) -> Result { + if self.ft.contains_qid(qid) { + Ok(self.iounit) + } else { + Err(E_UNKNOWN_FILE.to_string()) + } } - fn walk_one(&self, _cid: ClientId, parent_qid: u64, child: &str, _uname: &str) -> Result { + fn walk_one(&self, parent_qid: u64, child: &str, _cid: ClientId) -> Result { self.ft.walk_one(parent_qid, child) } - fn read( - &self, - _cid: ClientId, - qid: u64, - offset: usize, - count: usize, - _uname: &str, - ) -> Result { + fn read(&self, qid: u64, offset: usize, count: usize, _cid: ClientId) -> Result { let data: Vec = self.ft.with_file(qid, |f| { if offset > f.aux.len() { Vec::new() @@ -70,40 +75,37 @@ impl Serve9p for RamFs { Ok(ReadOutcome::Immediate(data)) } - fn read_dir(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result> { + fn read_dir(&self, qid: u64, _cid: ClientId) -> Result> { self.ft.read_dir(qid) } - fn write( - &self, - _cid: ClientId, - qid: u64, - offset: usize, - data: Vec, - _uname: &str, - ) -> Result { + fn write(&self, qid: u64, offset: usize, data: Vec, _cid: ClientId) -> Result { self.ft.with_file_mut(qid, |f| { + if offset > f.aux.len() { + return Err("offset beyond end of file".to_string()); + } + let n = data.len(); if offset + data.len() > f.aux.len() { f.aux.resize(offset + data.len(), 0); } f.aux[offset..offset + data.len()].copy_from_slice(data.as_slice()); - n - }) + Ok(n) + })? } - fn stat(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result { + fn stat(&self, qid: u64, _cid: ClientId) -> Result { self.ft.stat(qid) } - fn write_stat(&self, _cid: ClientId, qid: u64, wstat: WStat, _uname: &str) -> Result<()> { + fn write_stat(&self, qid: u64, wstat: WStat, _cid: ClientId) -> Result<()> { self.ft .with_file_mut(qid, |f| f.try_apply_wstat(wstat))? .map_err(|_| E_PERMISSION_DENIED.to_string()) } - fn remove(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result<()> { + fn remove(&self, qid: u64, _cid: ClientId) -> Result<()> { self.ft.remove(qid); Ok(()) @@ -111,12 +113,11 @@ impl Serve9p for RamFs { fn create( &self, - _cid: ClientId, parent: u64, name: &str, perm: Perm, _mode: Mode, - _uname: &str, + _cid: ClientId, ) -> Result<(Qid, IoUnit)> { let ty = FileType::from(perm); let qid = self.ft.try_add_node(parent, name, perm, ty, Vec::new())?; @@ -124,3 +125,294 @@ impl Serve9p for RamFs { Ok((qid, self.iounit)) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::sansio::server::E_ILLEGAL_CREATE_NAME; + use simple_test_case::test_case; + + const CID: ClientId = ClientId(0); + + fn add_file(fs: &RamFs, parent: u64, name: &str, data: &[u8]) -> Qid { + fs.file_tree() + .try_add_node( + parent, + name, + Perm::any_read(), + FileType::FILE, + data.to_vec(), + ) + .unwrap() + } + + #[test] + fn open_works() { + let fs = RamFs::new("user", "group"); + let qid = add_file(&fs, 0, "test", b""); + + assert_eq!(fs.open(qid.path, Mode::READ, CID).unwrap(), DEFAULT_IOUNIT); + assert_eq!(fs.open(42, Mode::READ, CID).unwrap_err(), E_UNKNOWN_FILE); + } + + #[test] + fn walk_one_works() { + let fs = RamFs::new("user", "group"); + let qid = add_file(&fs, 0, "test", b""); + + assert_eq!(fs.walk_one(0, "test", CID).unwrap(), qid); + } + + #[test_case(99, "test"; "unknown parent")] + #[test_case(0, "missing"; "unknown child")] + #[test] + fn walk_one_unknown_returns_error(parent_qid: u64, child: &str) { + let fs = RamFs::new("user", "group"); + let _ = add_file(&fs, 0, "test", b""); + + assert_eq!( + fs.walk_one(parent_qid, child, CID).unwrap_err(), + E_UNKNOWN_FILE + ); + } + + #[test_case(0, 3, b"abc".to_vec(); "from start")] + #[test_case(2, 2, b"cd".to_vec(); "middle slice")] + #[test_case(6, 5, b"".to_vec(); "offset at end")] + #[test_case(100, 5, b"".to_vec(); "offset beyond end")] + #[test] + fn read_returns_expected_data(offset: usize, count: usize, expected: Vec) { + let fs = RamFs::new("user", "group"); + let qid = add_file(&fs, 0, "test", b"abcdef"); + + match fs.read(qid.path, offset, count, CID).unwrap() { + ReadOutcome::Immediate(data) => assert_eq!(data, expected), + ReadOutcome::Blocked(_) => panic!("RamFs should always return immediate read data"), + } + } + + #[test] + fn read_unknown_returns_error() { + let fs = RamFs::new("user", "group"); + + assert_eq!(fs.read(1, 0, 1, CID).unwrap_err(), E_UNKNOWN_FILE); + } + + #[test] + fn read_dir_returns_children_for_directory() { + let fs = RamFs::new("user", "group"); + let _ = add_file(&fs, 0, "root-file", b""); + let dir = fs + .file_tree() + .try_add_node(0, "dir", Perm::DIRECTORY, FileType::DIRECTORY, Vec::new()) + .unwrap(); + let _ = add_file(&fs, dir.path, "nested", b""); + + let mut root_children = fs + .read_dir(0, CID) + .unwrap() + .into_iter() + .map(|s| s.name) + .collect::>(); + root_children.sort(); + + assert_eq!( + root_children, + vec!["dir".to_string(), "root-file".to_string()] + ); + + let nested_children = fs.read_dir(dir.path, CID).unwrap(); + assert_eq!(nested_children.len(), 1); + assert_eq!(nested_children[0].name, "nested"); + } + + #[test] + fn read_dir_unknown_returns_error() { + let fs = RamFs::new("user", "group"); + + assert_eq!(fs.read_dir(1, CID).unwrap_err(), E_UNKNOWN_FILE); + } + + #[test_case(1, b"ZZ".to_vec(), b"aZZ".to_vec(); "overwrite existing bytes")] + #[test_case(3, b"X".to_vec(), b"abcX".to_vec(); "append at end")] + #[test_case(0, b"".to_vec(), b"abc".to_vec(); "empty write")] + #[test] + fn write_updates_content(offset: usize, payload: Vec, expected: Vec) { + let fs = RamFs::new("user", "group"); + let qid = add_file(&fs, 0, "test", b"abc"); + + let n = fs.write(qid.path, offset, payload.clone(), CID).unwrap(); + assert_eq!(n, payload.len()); + + let data = fs + .file_tree() + .with_file(qid.path, |f| f.aux.clone()) + .unwrap(); + assert_eq!(data, expected); + } + + #[test] + fn write_offset_past_end_returns_error() { + let fs = RamFs::new("user", "group"); + let qid = add_file(&fs, 0, "test", b"abc"); + + assert_eq!( + fs.write(qid.path, 5, b"X".to_vec(), CID).unwrap_err(), + "offset beyond end of file" + ); + + let data = fs + .file_tree() + .with_file(qid.path, |f| f.aux.clone()) + .unwrap(); + assert_eq!(data, b"abc".to_vec()); + } + + #[test] + fn write_unknown_returns_error() { + let fs = RamFs::new("user", "group"); + + assert_eq!(fs.write(1, 0, vec![1], CID).unwrap_err(), E_UNKNOWN_FILE); + } + + #[test] + fn stat_works() { + let fs = RamFs::new("user", "group"); + let qid = add_file(&fs, 0, "test", b""); + + let stat = fs.stat(qid.path, CID).unwrap(); + assert_eq!(stat.qid, qid); + assert_eq!(stat.name, "test"); + } + + #[test] + fn stat_unknown_returns_error() { + let fs = RamFs::new("user", "group"); + + assert_eq!(fs.stat(1, CID).unwrap_err(), E_UNKNOWN_FILE); + } + + #[test] + fn write_stat_updates_file_metadata() { + let fs = RamFs::new("user", "group"); + let qid = add_file(&fs, 0, "test", b""); + + let wstat = WStat { + qid, + name: Some("renamed".into()), + perms: Some(Perm::OWNER_READ), + n_bytes: Some(123), + ..Default::default() + }; + + fs.write_stat(qid.path, wstat, CID).unwrap(); + + let stat = fs.stat(qid.path, CID).unwrap(); + assert_eq!(stat.name, "renamed"); + assert_eq!(stat.perms, Perm::OWNER_READ); + assert_eq!(stat.n_bytes, 123); + } + + #[test] + fn write_stat_mismatched_qid_returns_permission_denied() { + let fs = RamFs::new("user", "group"); + let qid = add_file(&fs, 0, "test", b""); + + let wstat = WStat { + qid: Qid::file(qid.path + 1), + ..Default::default() + }; + + assert_eq!( + fs.write_stat(qid.path, wstat, CID).unwrap_err(), + E_PERMISSION_DENIED + ); + } + + #[test] + fn write_stat_unknown_returns_error() { + let fs = RamFs::new("user", "group"); + let wstat = WStat { + qid: Qid::file(1), + ..Default::default() + }; + + assert_eq!(fs.write_stat(1, wstat, CID).unwrap_err(), E_UNKNOWN_FILE); + } + + #[test] + fn remove_prunes_subtree() { + let fs = RamFs::new("user", "group"); + let dir = fs + .file_tree() + .try_add_node(0, "dir", Perm::DIRECTORY, FileType::DIRECTORY, Vec::new()) + .unwrap(); + let child = add_file(&fs, dir.path, "nested", b""); + + fs.remove(dir.path, CID).unwrap(); + + assert_eq!(fs.stat(dir.path, CID).unwrap_err(), E_UNKNOWN_FILE); + assert_eq!(fs.stat(child.path, CID).unwrap_err(), E_UNKNOWN_FILE); + } + + #[test] + fn remove_unknown_is_ok() { + let fs = RamFs::new("user", "group"); + + assert!(fs.remove(1, CID).is_ok()); + } + + #[test_case(Perm::OWNER_READ, FileType::FILE, "file"; "file create")] + #[test_case(Perm::DIRECTORY | Perm::OWNER_READ, FileType::DIRECTORY, "dir"; "directory create")] + #[test] + fn create_adds_new_node(perm: Perm, expected_ty: FileType, name: &str) { + let fs = RamFs::new_with_base_perms_and_iounit( + "user", + "group", + Perm::any_read() | Perm::any_write() | Perm::any_exec(), + 999, + ); + + let (qid, iounit) = fs.create(0, name, perm, Mode::READ, CID).unwrap(); + assert_eq!(iounit, 999); + assert_eq!(qid.ty, expected_ty); + assert_eq!(fs.walk_one(0, name, CID).unwrap().path, qid.path); + } + + #[test] + fn create_unknown_parent_returns_error() { + let fs = RamFs::new("user", "group"); + + assert_eq!( + fs.create(999, "test", Perm::OWNER_READ, Mode::READ, CID) + .unwrap_err(), + E_UNKNOWN_FILE + ); + } + + #[test_case("."; "single dot")] + #[test_case(".."; "double dot")] + #[test] + fn create_rejects_illegal_names(name: &str) { + let fs = RamFs::new("user", "group"); + + assert_eq!( + fs.create(0, name, Perm::OWNER_READ, Mode::READ, CID) + .unwrap_err(), + E_ILLEGAL_CREATE_NAME + ); + } + + #[test] + fn create_rejects_duplicate_names() { + let fs = RamFs::new("user", "group"); + fs.create(0, "dup", Perm::OWNER_READ, Mode::READ, CID) + .unwrap(); + + assert_eq!( + fs.create(0, "dup", Perm::OWNER_READ, Mode::READ, CID) + .unwrap_err(), + "file already exists" + ); + } +} From b34533a89c6f732b2d9e8b10975d86ddd3a44e9e Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Wed, 6 May 2026 13:56:06 +0100 Subject: [PATCH 3/7] Fix method signatures for util filesystems --- crates/ninep/src/util/hook.rs | 78 ++++++++-------------------- crates/ninep/src/util/local_proxy.rs | 49 ++++++----------- crates/ninep/src/util/read_only.rs | 51 +++++++----------- 3 files changed, 57 insertions(+), 121 deletions(-) diff --git a/crates/ninep/src/util/hook.rs b/crates/ninep/src/util/hook.rs index 60242c45..dafb6eca 100644 --- a/crates/ninep/src/util/hook.rs +++ b/crates/ninep/src/util/hook.rs @@ -50,103 +50,79 @@ impl Serve9p for HookFs where T: Serve9p, { - fn open(&self, cid: ClientId, qid: u64, mode: Mode, uname: &str) -> Result { - (self.hook)(FsOp::Open { - cid, - qid, - mode, - uname, - })?; + fn open(&self, qid: u64, mode: Mode, cid: ClientId) -> Result { + (self.hook)(FsOp::Open { cid, qid, mode })?; - self.inner.open(cid, qid, mode, uname) + self.inner.open(qid, mode, cid) } - fn walk_one(&self, cid: ClientId, parent_qid: u64, child: &str, uname: &str) -> Result { + fn walk_one(&self, parent_qid: u64, child: &str, cid: ClientId) -> Result { (self.hook)(FsOp::WalkOne { cid, parent_qid, child, - uname, })?; - self.inner.walk_one(cid, parent_qid, child, uname) + self.inner.walk_one(parent_qid, child, cid) } - fn read( - &self, - cid: ClientId, - qid: u64, - offset: usize, - count: usize, - uname: &str, - ) -> Result { + fn read(&self, qid: u64, offset: usize, count: usize, cid: ClientId) -> Result { (self.hook)(FsOp::Read { cid, qid, offset, count, - uname, })?; - self.inner.read(cid, qid, offset, count, uname) + self.inner.read(qid, offset, count, cid) } - fn read_dir(&self, cid: ClientId, qid: u64, uname: &str) -> Result> { - (self.hook)(FsOp::ReadDir { cid, qid, uname })?; + fn read_dir(&self, qid: u64, cid: ClientId) -> Result> { + (self.hook)(FsOp::ReadDir { cid, qid })?; - self.inner.read_dir(cid, qid, uname) + self.inner.read_dir(qid, cid) } - fn write( - &self, - cid: ClientId, - qid: u64, - offset: usize, - data: Vec, - uname: &str, - ) -> Result { + fn write(&self, qid: u64, offset: usize, data: Vec, cid: ClientId) -> Result { (self.hook)(FsOp::Write { cid, qid, offset, n_bytes: data.len(), - uname, })?; - self.inner.write(cid, qid, offset, data, uname) + self.inner.write(qid, offset, data, cid) } - fn stat(&self, cid: ClientId, qid: u64, uname: &str) -> Result { - (self.hook)(FsOp::Stat { cid, qid, uname })?; + fn stat(&self, qid: u64, cid: ClientId) -> Result { + (self.hook)(FsOp::Stat { cid, qid })?; - self.inner.stat(cid, qid, uname) + self.inner.stat(qid, cid) } - fn write_stat(&self, cid: ClientId, qid: u64, wstat: WStat, uname: &str) -> Result<()> { + fn write_stat(&self, qid: u64, wstat: WStat, cid: ClientId) -> Result<()> { (self.hook)(FsOp::WriteStat { cid, qid, wstat: &wstat, - uname, })?; - self.inner.write_stat(cid, qid, wstat, uname) + self.inner.write_stat(qid, wstat, cid) } - fn remove(&self, cid: ClientId, qid: u64, uname: &str) -> Result<()> { - (self.hook)(FsOp::Remove { cid, qid, uname })?; + fn remove(&self, qid: u64, cid: ClientId) -> Result<()> { + (self.hook)(FsOp::Remove { cid, qid })?; - self.inner.remove(cid, qid, uname) + self.inner.remove(qid, cid) } fn create( &self, - cid: ClientId, parent: u64, name: &str, perm: Perm, mode: Mode, - uname: &str, + cid: ClientId, ) -> Result<(Qid, IoUnit)> { (self.hook)(FsOp::Create { cid, @@ -154,10 +130,9 @@ where name, perm, mode, - uname, })?; - self.inner.create(cid, parent, name, perm, mode, uname) + self.inner.create(parent, name, perm, mode, cid) } } @@ -169,48 +144,40 @@ pub enum FsOp<'a> { cid: ClientId, parent_qid: u64, child: &'a str, - uname: &'a str, }, Open { cid: ClientId, qid: u64, mode: Mode, - uname: &'a str, }, Read { cid: ClientId, qid: u64, offset: usize, count: usize, - uname: &'a str, }, ReadDir { cid: ClientId, qid: u64, - uname: &'a str, }, Write { cid: ClientId, qid: u64, offset: usize, n_bytes: usize, - uname: &'a str, }, Stat { cid: ClientId, qid: u64, - uname: &'a str, }, WriteStat { cid: ClientId, qid: u64, wstat: &'a WStat, - uname: &'a str, }, Remove { cid: ClientId, qid: u64, - uname: &'a str, }, Create { cid: ClientId, @@ -218,6 +185,5 @@ pub enum FsOp<'a> { name: &'a str, perm: Perm, mode: Mode, - uname: &'a str, }, } diff --git a/crates/ninep/src/util/local_proxy.rs b/crates/ninep/src/util/local_proxy.rs index 59a4fe6b..fe11d335 100644 --- a/crates/ninep/src/util/local_proxy.rs +++ b/crates/ninep/src/util/local_proxy.rs @@ -32,7 +32,7 @@ pub struct LocalProxyFs { } impl LocalProxyFs { - /// Construct a new [ProxyFs] pointed at the provided root directory + /// Construct a new [LocalProxyFs] pointed at the provided root directory. pub fn new(root: impl Into) -> io::Result { let root = fs::canonicalize(root.into())?; let root_meta = fs::metadata(&root)?; @@ -138,7 +138,13 @@ impl LocalProxyFs { let perms = perms_from_meta(&pm.meta); let meta = pm.meta.clone(); let qid = self.ft.try_add_node(qid, &name, perms, ty, pm)?; - self.try_sync_file_with_meta(qid.path, meta)?; + + // FIXME: this is too eager: we don't want to sync the full tree! + if meta.is_dir() { + self.try_sync_dir(qid.path)? + } else { + self.try_sync_file_with_meta(qid.path, meta)?; + } } Ok(()) @@ -204,7 +210,7 @@ impl LocalProxyFs { } impl Serve9p for LocalProxyFs { - fn open(&self, _cid: ClientId, qid: u64, _mode: Mode, _uname: &str) -> Result { + fn open(&self, qid: u64, _mode: Mode, _cid: ClientId) -> Result { if self.modified_since_cache_or_prune(qid)? { self.try_sync_file(qid)?; } @@ -212,7 +218,7 @@ impl Serve9p for LocalProxyFs { Ok(self.iounit) } - fn walk_one(&self, _cid: ClientId, parent_qid: u64, child: &str, _uname: &str) -> Result { + fn walk_one(&self, parent_qid: u64, child: &str, _cid: ClientId) -> Result { if self.modified_since_cache_or_prune(parent_qid)? { self.try_sync_dir(parent_qid)?; } @@ -220,14 +226,7 @@ impl Serve9p for LocalProxyFs { self.ft.walk_one(parent_qid, child) } - fn read( - &self, - _cid: ClientId, - qid: u64, - offset: usize, - count: usize, - _uname: &str, - ) -> Result { + fn read(&self, qid: u64, offset: usize, count: usize, _cid: ClientId) -> Result { if self.modified_since_cache_or_prune(qid)? { self.try_sync_file(qid)?; } @@ -248,7 +247,7 @@ impl Serve9p for LocalProxyFs { Ok(ReadOutcome::Immediate(buf)) } - fn read_dir(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result> { + fn read_dir(&self, qid: u64, _cid: ClientId) -> Result> { if self.modified_since_cache_or_prune(qid)? { self.try_sync_dir(qid)?; } @@ -256,14 +255,7 @@ impl Serve9p for LocalProxyFs { self.ft.read_dir(qid) } - fn write( - &self, - _cid: ClientId, - qid: u64, - offset: usize, - data: Vec, - _uname: &str, - ) -> Result { + fn write(&self, qid: u64, offset: usize, data: Vec, _cid: ClientId) -> Result { if self.modified_since_cache_or_prune(qid)? { self.try_sync_file(qid)?; } @@ -280,7 +272,7 @@ impl Serve9p for LocalProxyFs { f.write(data.as_slice()).map_err(|e| e.to_string()) } - fn stat(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result { + fn stat(&self, qid: u64, _cid: ClientId) -> Result { if self.modified_since_cache_or_prune(qid)? { let pm = self.meta_for_qid(qid); if pm.meta.is_dir() { @@ -293,7 +285,7 @@ impl Serve9p for LocalProxyFs { self.ft.stat(qid) } - fn write_stat(&self, _cid: ClientId, qid: u64, wstat: WStat, _uname: &str) -> Result<()> { + fn write_stat(&self, qid: u64, wstat: WStat, _cid: ClientId) -> Result<()> { if wstat.last_accessed.is_some() || wstat.last_modified.is_some() || wstat.group.is_some() @@ -342,7 +334,7 @@ impl Serve9p for LocalProxyFs { Ok(()) } - fn remove(&self, _cid: ClientId, qid: u64, _uname: &str) -> Result<()> { + fn remove(&self, qid: u64, _cid: ClientId) -> Result<()> { if qid == 0 { return Err(E_PERMISSION_DENIED.to_string()); } @@ -367,12 +359,11 @@ impl Serve9p for LocalProxyFs { fn create( &self, - _cid: ClientId, parent: u64, name: &str, perm: Perm, _mode: Mode, - _uname: &str, + _cid: ClientId, ) -> Result<(Qid, IoUnit)> { if self.modified_since_cache_or_prune(parent)? { self.try_sync_dir(parent)?; @@ -431,9 +422,3 @@ fn owner_and_group_from_meta(meta: &Metadata) -> (String, String) { (owner, group) } - -#[cfg(test)] -mod tests { - use super::*; - use assert_fs::TempDir; -} diff --git a/crates/ninep/src/util/read_only.rs b/crates/ninep/src/util/read_only.rs index cb000938..90f4a6fa 100644 --- a/crates/ninep/src/util/read_only.rs +++ b/crates/ninep/src/util/read_only.rs @@ -40,42 +40,28 @@ impl Serve9p for ReadOnlyFs where T: Serve9p, { - fn open(&self, cid: ClientId, qid: u64, mode: Mode, uname: &str) -> Result { - self.inner.open(cid, qid, mode, uname) + fn open(&self, qid: u64, mode: Mode, cid: ClientId) -> Result { + self.inner.open(qid, mode, cid) } - fn walk_one(&self, cid: ClientId, parent_qid: u64, child: &str, uname: &str) -> Result { - self.inner.walk_one(cid, parent_qid, child, uname) + fn walk_one(&self, parent_qid: u64, child: &str, cid: ClientId) -> Result { + self.inner.walk_one(parent_qid, child, cid) } - fn read( - &self, - cid: ClientId, - qid: u64, - offset: usize, - count: usize, - uname: &str, - ) -> Result { - self.inner.read(cid, qid, offset, count, uname) + fn read(&self, qid: u64, offset: usize, count: usize, cid: ClientId) -> Result { + self.inner.read(qid, offset, count, cid) } - fn read_dir(&self, cid: ClientId, qid: u64, uname: &str) -> Result> { - self.inner.read_dir(cid, qid, uname) + fn read_dir(&self, qid: u64, cid: ClientId) -> Result> { + self.inner.read_dir(qid, cid) } - fn write( - &self, - cid: ClientId, - qid: u64, - offset: usize, - data: Vec, - uname: &str, - ) -> Result { - self.inner.write(cid, qid, offset, data, uname) + fn write(&self, qid: u64, offset: usize, data: Vec, cid: ClientId) -> Result { + self.inner.write(qid, offset, data, cid) } - fn stat(&self, cid: ClientId, qid: u64, uname: &str) -> Result { - let mut stat = self.inner.stat(cid, qid, uname)?; + fn stat(&self, qid: u64, cid: ClientId) -> Result { + let mut stat = self.inner.stat(qid, cid)?; if self.read_only { stat.perms .remove(Perm::OWNER_WRITE | Perm::GROUP_WRITE | Perm::OTHER_WRITE); @@ -84,27 +70,26 @@ where Ok(stat) } - fn write_stat(&self, cid: ClientId, qid: u64, wstat: WStat, uname: &str) -> Result<()> { - self.inner.write_stat(cid, qid, wstat, uname) + fn write_stat(&self, qid: u64, wstat: WStat, cid: ClientId) -> Result<()> { + self.inner.write_stat(qid, wstat, cid) } - fn remove(&self, cid: ClientId, qid: u64, uname: &str) -> Result<()> { - self.inner.remove(cid, qid, uname) + fn remove(&self, qid: u64, cid: ClientId) -> Result<()> { + self.inner.remove(qid, cid) } fn create( &self, - cid: ClientId, parent: u64, name: &str, perm: Perm, mode: Mode, - uname: &str, + cid: ClientId, ) -> Result<(Qid, IoUnit)> { if self.read_only && mode.allows_write() { return Err(E_PERMISSION_DENIED.to_string()); } - self.inner.create(cid, parent, name, perm, mode, uname) + self.inner.create(parent, name, perm, mode, cid) } } From 390cf8de7299eaa124354235418b9a93e78e4995 Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Wed, 6 May 2026 14:11:58 +0100 Subject: [PATCH 4/7] Add tests for HookFs --- crates/ninep/src/util/hook.rs | 343 ++++++++++++++++++++++++++++++---- 1 file changed, 310 insertions(+), 33 deletions(-) diff --git a/crates/ninep/src/util/hook.rs b/crates/ninep/src/util/hook.rs index dafb6eca..28590e7a 100644 --- a/crates/ninep/src/util/hook.rs +++ b/crates/ninep/src/util/hook.rs @@ -51,67 +51,49 @@ where T: Serve9p, { fn open(&self, qid: u64, mode: Mode, cid: ClientId) -> Result { - (self.hook)(FsOp::Open { cid, qid, mode })?; + (self.hook)(FsOp::open(cid, qid, mode))?; self.inner.open(qid, mode, cid) } fn walk_one(&self, parent_qid: u64, child: &str, cid: ClientId) -> Result { - (self.hook)(FsOp::WalkOne { - cid, - parent_qid, - child, - })?; + (self.hook)(FsOp::walk_one(cid, parent_qid, child))?; self.inner.walk_one(parent_qid, child, cid) } fn read(&self, qid: u64, offset: usize, count: usize, cid: ClientId) -> Result { - (self.hook)(FsOp::Read { - cid, - qid, - offset, - count, - })?; + (self.hook)(FsOp::read(cid, qid, offset, count))?; self.inner.read(qid, offset, count, cid) } fn read_dir(&self, qid: u64, cid: ClientId) -> Result> { - (self.hook)(FsOp::ReadDir { cid, qid })?; + (self.hook)(FsOp::read_dir(cid, qid))?; self.inner.read_dir(qid, cid) } fn write(&self, qid: u64, offset: usize, data: Vec, cid: ClientId) -> Result { - (self.hook)(FsOp::Write { - cid, - qid, - offset, - n_bytes: data.len(), - })?; + (self.hook)(FsOp::write(cid, qid, offset, data.len()))?; self.inner.write(qid, offset, data, cid) } fn stat(&self, qid: u64, cid: ClientId) -> Result { - (self.hook)(FsOp::Stat { cid, qid })?; + (self.hook)(FsOp::stat(cid, qid))?; self.inner.stat(qid, cid) } fn write_stat(&self, qid: u64, wstat: WStat, cid: ClientId) -> Result<()> { - (self.hook)(FsOp::WriteStat { - cid, - qid, - wstat: &wstat, - })?; + (self.hook)(FsOp::write_stat(cid, qid, &wstat))?; self.inner.write_stat(qid, wstat, cid) } fn remove(&self, qid: u64, cid: ClientId) -> Result<()> { - (self.hook)(FsOp::Remove { cid, qid })?; + (self.hook)(FsOp::remove(cid, qid))?; self.inner.remove(qid, cid) } @@ -124,13 +106,7 @@ where mode: Mode, cid: ClientId, ) -> Result<(Qid, IoUnit)> { - (self.hook)(FsOp::Create { - cid, - parent, - name, - perm, - mode, - })?; + (self.hook)(FsOp::create(cid, parent, name, perm, mode))?; self.inner.create(parent, name, perm, mode, cid) } @@ -187,3 +163,304 @@ pub enum FsOp<'a> { mode: Mode, }, } + +impl<'a> FsOp<'a> { + fn walk_one(cid: ClientId, parent_qid: u64, child: &'a str) -> Self { + Self::WalkOne { + cid, + parent_qid, + child, + } + } + + fn open(cid: ClientId, qid: u64, mode: Mode) -> Self { + Self::Open { cid, qid, mode } + } + + fn read(cid: ClientId, qid: u64, offset: usize, count: usize) -> Self { + Self::Read { + cid, + qid, + offset, + count, + } + } + + fn read_dir(cid: ClientId, qid: u64) -> Self { + Self::ReadDir { cid, qid } + } + + fn write(cid: ClientId, qid: u64, offset: usize, n_bytes: usize) -> Self { + Self::Write { + cid, + qid, + offset, + n_bytes, + } + } + + fn stat(cid: ClientId, qid: u64) -> Self { + Self::Stat { cid, qid } + } + + fn write_stat(cid: ClientId, qid: u64, wstat: &'a WStat) -> Self { + Self::WriteStat { cid, qid, wstat } + } + + fn remove(cid: ClientId, qid: u64) -> Self { + Self::Remove { cid, qid } + } + + fn create(cid: ClientId, parent: u64, name: &'a str, perm: Perm, mode: Mode) -> Self { + Self::Create { + cid, + parent, + name, + perm, + mode, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{fs::FileType, sansio::server::E_UNKNOWN_FILE, util::ram::RamFs}; + use simple_test_case::test_case; + + const CID: ClientId = ClientId(0); + const HOOK_ERR: &str = "hook failed"; + + fn hook_result(is_ok: bool) -> Result<()> { + if is_ok { + Ok(()) + } else { + Err(HOOK_ERR.to_string()) + } + } + + fn read_all(fs: &RamFs, qid: u64) -> Vec { + match fs.read(qid, 0, 1024, CID).unwrap() { + ReadOutcome::Immediate(data) => data, + ReadOutcome::Blocked(_) => panic!("RamFs should return immediate read data"), + } + } + + fn add_file(fs: &RamFs, parent: u64, name: &str, data: &[u8]) -> Qid { + fs.file_tree() + .try_add_node( + parent, + name, + Perm::any_read() | Perm::any_write(), + FileType::FILE, + data.to_vec(), + ) + .unwrap() + } + + #[test_case(true; "hook ok")] + #[test_case(false; "hook err")] + #[test] + fn open_hook_controls_execution(hook_ok: bool) { + let inner = RamFs::new("user", "group"); + let qid = add_file(&inner, 0, "f", b""); + let target_qid = if hook_ok { qid.path } else { 99 }; + + let fs = HookFs::new(inner, move |op| { + assert_eq!(op, FsOp::open(CID, target_qid, Mode::READ)); + hook_result(hook_ok) + }); + + let res = fs.open(target_qid, Mode::READ, CID); + if hook_ok { + assert!(res.is_ok()); + } else { + assert_eq!(res.unwrap_err(), HOOK_ERR); + } + } + + #[test_case(true; "hook ok")] + #[test_case(false; "hook err")] + #[test] + fn walk_one_hook_controls_execution(hook_ok: bool) { + let inner = RamFs::new("user", "group"); + let qid = add_file(&inner, 0, "child", b""); + let parent_qid = if hook_ok { 0 } else { 99 }; + + let fs = HookFs::new(inner, move |op| { + assert_eq!(op, FsOp::walk_one(CID, parent_qid, "child")); + hook_result(hook_ok) + }); + + let res = fs.walk_one(parent_qid, "child", CID); + if hook_ok { + assert_eq!(res.unwrap(), qid); + } else { + assert_eq!(res.unwrap_err(), HOOK_ERR); + } + } + + #[test_case(true; "hook ok")] + #[test_case(false; "hook err")] + #[test] + fn read_hook_controls_execution(hook_ok: bool) { + let inner = RamFs::new("user", "group"); + let qid = add_file(&inner, 0, "f", b"abcdef"); + let target_qid = if hook_ok { qid.path } else { 99 }; + + let fs = HookFs::new(inner, move |op| { + assert_eq!(op, FsOp::read(CID, target_qid, 1, 3)); + hook_result(hook_ok) + }); + + let res = fs.read(target_qid, 1, 3, CID); + if hook_ok { + match res.unwrap() { + ReadOutcome::Immediate(data) => assert_eq!(data, b"bcd".to_vec()), + ReadOutcome::Blocked(_) => panic!("RamFs should return immediate read data"), + } + } else { + assert_eq!(res.unwrap_err(), HOOK_ERR); + } + } + + #[test_case(true; "hook ok")] + #[test_case(false; "hook err")] + #[test] + fn read_dir_hook_controls_execution(hook_ok: bool) { + let inner = RamFs::new("user", "group"); + let _ = add_file(&inner, 0, "entry", b""); + let target_qid = if hook_ok { 0 } else { 99 }; + + let fs = HookFs::new(inner, move |op| { + assert_eq!(op, FsOp::read_dir(CID, target_qid)); + hook_result(hook_ok) + }); + + let res = fs.read_dir(target_qid, CID); + if hook_ok { + let entries = res.unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].name, "entry"); + } else { + assert_eq!(res.unwrap_err(), HOOK_ERR); + } + } + + #[test_case(true; "hook ok")] + #[test_case(false; "hook err")] + #[test] + fn write_hook_controls_execution(hook_ok: bool) { + let inner = RamFs::new("user", "group"); + let qid = add_file(&inner, 0, "f", b"abc"); + let fs = HookFs::new(inner.clone(), move |op| { + assert_eq!(op, FsOp::write(CID, qid.path, 1, 2)); + hook_result(hook_ok) + }); + + let res = fs.write(qid.path, 1, b"ZZ".to_vec(), CID); + if hook_ok { + assert_eq!(res.unwrap(), 2); + assert_eq!(read_all(&inner, qid.path), b"aZZ".to_vec()); + } else { + assert_eq!(res.unwrap_err(), HOOK_ERR); + assert_eq!(read_all(&inner, qid.path), b"abc".to_vec()); + } + } + + #[test_case(true; "hook ok")] + #[test_case(false; "hook err")] + #[test] + fn stat_hook_controls_execution(hook_ok: bool) { + let inner = RamFs::new("user", "group"); + let qid = add_file(&inner, 0, "f", b""); + let target_qid = if hook_ok { qid.path } else { 99 }; + + let fs = HookFs::new(inner, move |op| { + assert_eq!(op, FsOp::stat(CID, target_qid)); + hook_result(hook_ok) + }); + + let res = fs.stat(target_qid, CID); + if hook_ok { + assert_eq!(res.unwrap().name, "f"); + } else { + assert_eq!(res.unwrap_err(), HOOK_ERR); + } + } + + #[test_case(true; "hook ok")] + #[test_case(false; "hook err")] + #[test] + fn write_stat_hook_controls_execution(hook_ok: bool) { + let inner = RamFs::new("user", "group"); + let qid = add_file(&inner, 0, "f", b""); + let wstat = WStat { + qid, + name: Some("renamed".into()), + ..Default::default() + }; + let expected_wstat = wstat.clone(); + + let fs = HookFs::new(inner.clone(), move |op| { + assert_eq!(op, FsOp::write_stat(CID, qid.path, &expected_wstat)); + hook_result(hook_ok) + }); + + let res = fs.write_stat(qid.path, wstat, CID); + if hook_ok { + assert!(res.is_ok()); + assert_eq!(inner.stat(qid.path, CID).unwrap().name, "renamed"); + } else { + assert_eq!(res.unwrap_err(), HOOK_ERR); + assert_eq!(inner.stat(qid.path, CID).unwrap().name, "f"); + } + } + + #[test_case(true; "hook ok")] + #[test_case(false; "hook err")] + #[test] + fn remove_hook_controls_execution(hook_ok: bool) { + let inner = RamFs::new("user", "group"); + let qid = add_file(&inner, 0, "f", b""); + let fs = HookFs::new(inner.clone(), move |op| { + assert_eq!(op, FsOp::remove(CID, qid.path)); + hook_result(hook_ok) + }); + + let res = fs.remove(qid.path, CID); + if hook_ok { + assert!(res.is_ok()); + assert_eq!(inner.stat(qid.path, CID).unwrap_err(), E_UNKNOWN_FILE); + } else { + assert_eq!(res.unwrap_err(), HOOK_ERR); + assert_eq!(inner.stat(qid.path, CID).unwrap().name, "f"); + } + } + + #[test_case(true; "hook ok")] + #[test_case(false; "hook err")] + #[test] + fn create_hook_controls_execution(hook_ok: bool) { + let inner = RamFs::new("user", "group"); + let fs = HookFs::new(inner.clone(), move |op| { + assert_eq!( + op, + FsOp::create(CID, 0, "new-file", Perm::OWNER_READ, Mode::READ) + ); + hook_result(hook_ok) + }); + + let res = fs.create(0, "new-file", Perm::OWNER_READ, Mode::READ, CID); + if hook_ok { + let (qid, _) = res.unwrap(); + assert_eq!(inner.walk_one(0, "new-file", CID).unwrap().path, qid.path); + } else { + assert_eq!(res.unwrap_err(), HOOK_ERR); + assert_eq!( + inner.walk_one(0, "new-file", CID).unwrap_err(), + E_UNKNOWN_FILE + ); + } + } +} From 5bb4c4e85a567e6f5e3d5630f477ab8cc4785303 Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Wed, 6 May 2026 17:16:40 +0100 Subject: [PATCH 5/7] Sleep briefly when opening a buffer and trying to find the ID --- crates/ad_client/Cargo.toml | 2 +- crates/ad_client/src/sync/mod.rs | 4 ++++ crates/ad_client/src/tokio/mod.rs | 5 +++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/ad_client/Cargo.toml b/crates/ad_client/Cargo.toml index 47841361..61af601f 100644 --- a/crates/ad_client/Cargo.toml +++ b/crates/ad_client/Cargo.toml @@ -23,7 +23,7 @@ tokio = ["dep:tokio", "ninep/tokio"] [dependencies] ad_event = { version = "0.4", path = "../ad_event" } ninep = { version = "0.6", path = "../ninep", default-features = false } -tokio = { version = "1", features = ["macros", "net", "io-util", "rt", "sync"], optional = true } +tokio = { version = "1", features = ["macros", "net", "io-util", "rt", "sync", "time"], optional = true } [dev-dependencies] ad-editor = { path = "../../" } diff --git a/crates/ad_client/src/sync/mod.rs b/crates/ad_client/src/sync/mod.rs index 53d1ce2a..29b6afba 100644 --- a/crates/ad_client/src/sync/mod.rs +++ b/crates/ad_client/src/sync/mod.rs @@ -7,6 +7,8 @@ use std::{ os::unix::net::UnixStream, path::Path, str::FromStr, + thread::sleep, + time::Duration, }; mod event; @@ -221,6 +223,8 @@ impl Client { } fn _id_for_path(&mut self, path: &str) -> Result { + sleep(Duration::from_millis(5)); + for BufferMeta { id, filename } in self.open_buffers()?.into_iter() { if filename.ends_with(path) { return Ok(id); diff --git a/crates/ad_client/src/tokio/mod.rs b/crates/ad_client/src/tokio/mod.rs index a2de4b15..a83f34a7 100644 --- a/crates/ad_client/src/tokio/mod.rs +++ b/crates/ad_client/src/tokio/mod.rs @@ -1,8 +1,8 @@ //! An asynchronous client implementation. use crate::{BufferMeta, LogEvent, MiniBufferSelection, SessionMeta, parse_bufid}; use ninep::tokio::client::{Error, ReadLineStream, Result, UnixClient}; -use std::{env, io, path::Path, str::FromStr}; -use tokio::net::UnixStream; +use std::{env, io, path::Path, str::FromStr, time::Duration}; +use tokio::{net::UnixStream, time::sleep}; mod event; @@ -232,6 +232,7 @@ impl Client { } async fn _id_for_path(&mut self, path: &str) -> Result { + sleep(Duration::from_millis(5)).await; for BufferMeta { id, filename } in self.open_buffers().await?.into_iter() { if filename.ends_with(path) { return Ok(id); From f6034603c2cd2f60b659e08d00c27da04103ec4f Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Thu, 7 May 2026 07:30:47 +0100 Subject: [PATCH 6/7] Tidy up impl for LocalProxyFs --- crates/ninep/src/util/local_proxy.rs | 233 +++++++++++++++------------ 1 file changed, 126 insertions(+), 107 deletions(-) diff --git a/crates/ninep/src/util/local_proxy.rs b/crates/ninep/src/util/local_proxy.rs index fe11d335..6c302a00 100644 --- a/crates/ninep/src/util/local_proxy.rs +++ b/crates/ninep/src/util/local_proxy.rs @@ -1,7 +1,7 @@ //! A 9p filesystem proxy over a local directory. use crate::{ Result, - fs::{FileTree, FileType, IoUnit, Mode, Perm, Qid, Stat, Timestamp, WStat}, + fs::{FileTree, FileType, IoUnit, Mode, Perm, QID_ROOT, Qid, Stat, Timestamp, WStat}, sansio::server::{E_PERMISSION_DENIED, E_UNKNOWN_FILE}, sync::server::{ClientId, ReadOutcome, Serve9p}, }; @@ -16,13 +16,6 @@ use uzers::{get_group_by_gid, get_user_by_uid}; const DEFAULT_IOUNIT: IoUnit = 8168; -/// The metadata we store per-node in the file tree -#[derive(Debug, Clone)] -struct PathMeta { - path: PathBuf, - meta: Metadata, -} - /// A filesystem proxy that exposes a local directory over 9p. #[derive(Debug)] pub struct LocalProxyFs { @@ -35,27 +28,26 @@ impl LocalProxyFs { /// Construct a new [LocalProxyFs] pointed at the provided root directory. pub fn new(root: impl Into) -> io::Result { let root = fs::canonicalize(root.into())?; - let root_meta = fs::metadata(&root)?; - if !root_meta.is_dir() { + let meta = fs::metadata(&root)?; + if !meta.is_dir() { return Err(io::Error::other("proxy root must be a directory")); } - let (owner, group) = owner_and_group_from_meta(&root_meta); - let perms = perms_from_meta(&root_meta); + let (owner, group) = owner_and_group_from_meta(&meta); + let perms = perms_from_meta(&meta); let pm = PathMeta { path: root.clone(), - meta: root_meta, + meta, + needs_sync: false, }; - let ft = FileTree::new(&owner, &group, perms, pm); - let fs = Self { root, - ft, + ft: FileTree::new(&owner, &group, perms, pm), iounit: DEFAULT_IOUNIT, }; - fs.try_sync_dir(0).map_err(io::Error::other)?; + fs.try_sync_dir(QID_ROOT).map_err(io::Error::other)?; Ok(fs) } @@ -81,6 +73,14 @@ impl LocalProxyFs { self.ft.with_file(qid, |f| f.parent().unwrap()).unwrap() } + fn needs_sync(&self, qid: u64) -> bool { + self.ft.with_file(qid, |f| f.aux.needs_sync).unwrap() + } + + fn is_dir(&self, qid: u64) -> bool { + self.ft.with_file(qid, |f| f.aux.meta.is_dir()).unwrap() + } + /// Check to see if the given node has been modified since we cached metadata for it. /// If we are unable to read metadata from disk, prune this node. fn modified_since_cache_or_prune(&self, qid: u64) -> Result { @@ -102,7 +102,6 @@ impl LocalProxyFs { } } - // TODO: re-run perm checks if perms are now different fn try_sync_dir(&self, qid: u64) -> Result<()> { let path = self.meta_for_qid(qid).path; let mut on_disk = self.on_disk_entries_for(&path).map_err(|e| e.to_string())?; @@ -121,7 +120,8 @@ impl LocalProxyFs { f.stat.perms = perms_from_meta(&pm.meta); f.stat.last_accessed = Timestamp::from_second(pm.meta.atime()).unwrap(); f.stat.last_modified = Timestamp::from_second(pm.meta.mtime()).unwrap(); - f.aux = pm; + f.aux.path = pm.path; + f.aux.meta = pm.meta; }) } None => self.ft.remove(qid), @@ -138,15 +138,11 @@ impl LocalProxyFs { let perms = perms_from_meta(&pm.meta); let meta = pm.meta.clone(); let qid = self.ft.try_add_node(qid, &name, perms, ty, pm)?; - - // FIXME: this is too eager: we don't want to sync the full tree! - if meta.is_dir() { - self.try_sync_dir(qid.path)? - } else { - self.try_sync_file_with_meta(qid.path, meta)?; - } + self.try_sync_file_with_meta(qid.path, meta)?; } + _ = self.ft.with_file_mut(qid, |f| f.aux.needs_sync = false); + Ok(()) } @@ -159,7 +155,6 @@ impl LocalProxyFs { self.try_sync_file_with_meta(qid, meta) } - // TODO: re-run perm checks if perms are now different (will require Mode) fn try_sync_file_with_meta(&self, qid: u64, meta: Metadata) -> Result<()> { let path = self.path_for_qid(qid); let name = path @@ -178,7 +173,7 @@ impl LocalProxyFs { f.stat.perms = perms_from_meta(&meta); f.stat.last_accessed = Timestamp::from_second(meta.atime()).unwrap(); f.stat.last_modified = Timestamp::from_second(meta.mtime()).unwrap(); - f.aux = PathMeta { path, meta }; + f.aux = PathMeta::new(path, meta); }) } @@ -192,7 +187,7 @@ impl LocalProxyFs { // Only include entries that resolve to being under our root if let Some(meta) = self.meta_if_under_root(&path) { - m.insert(name, PathMeta { path, meta }); + m.insert(name, PathMeta::new(path, meta)); }; } @@ -219,7 +214,7 @@ impl Serve9p for LocalProxyFs { } fn walk_one(&self, parent_qid: u64, child: &str, _cid: ClientId) -> Result { - if self.modified_since_cache_or_prune(parent_qid)? { + if self.needs_sync(parent_qid) || self.modified_since_cache_or_prune(parent_qid)? { self.try_sync_dir(parent_qid)?; } @@ -231,24 +226,14 @@ impl Serve9p for LocalProxyFs { self.try_sync_file(qid)?; } - let path = self.path_for_qid(qid); - - let mut f = OpenOptions::new() - .read(true) - .open(&path) - .map_err(|e| e.to_string())?; - f.seek(SeekFrom::Start(offset as u64)) - .map_err(|e| e.to_string())?; - - let mut buf = vec![0; count]; - let n = f.read(&mut buf).map_err(|e| e.to_string())?; - buf.truncate(n); - - Ok(ReadOutcome::Immediate(buf)) + match io_read(&self.path_for_qid(qid), offset, count) { + Ok(data) => Ok(ReadOutcome::Immediate(data)), + Err(e) => Err(e.to_string()), + } } fn read_dir(&self, qid: u64, _cid: ClientId) -> Result> { - if self.modified_since_cache_or_prune(qid)? { + if self.needs_sync(qid) || self.modified_since_cache_or_prune(qid)? { self.try_sync_dir(qid)?; } @@ -260,22 +245,15 @@ impl Serve9p for LocalProxyFs { self.try_sync_file(qid)?; } - let path = self.path_for_qid(qid); - - let mut f = OpenOptions::new() - .write(true) - .open(&path) - .map_err(|e| e.to_string())?; - f.seek(SeekFrom::Start(offset as u64)) - .map_err(|e| e.to_string())?; - - f.write(data.as_slice()).map_err(|e| e.to_string()) + match io_write(&self.path_for_qid(qid), offset, &data) { + Ok(count) => Ok(count), + Err(e) => Err(e.to_string()), + } } fn stat(&self, qid: u64, _cid: ClientId) -> Result { if self.modified_since_cache_or_prune(qid)? { - let pm = self.meta_for_qid(qid); - if pm.meta.is_dir() { + if self.is_dir(qid) { self.try_sync_dir(qid)?; } else { self.try_sync_file(qid)?; @@ -290,7 +268,7 @@ impl Serve9p for LocalProxyFs { || wstat.last_modified.is_some() || wstat.group.is_some() || wstat.last_modified_by.is_some() - || qid == 0 + || qid == QID_ROOT { return Err(E_PERMISSION_DENIED.to_string()); } @@ -303,31 +281,15 @@ impl Serve9p for LocalProxyFs { } if let Some(perms) = wstat.perms { - let mode = perms.bits() & 0o777; - let mut permissions = fs::metadata(&pm.path) - .map_err(|e| e.to_string())? - .permissions(); - permissions.set_mode(mode); - fs::set_permissions(&pm.path, permissions).map_err(|e| e.to_string())?; + io_set_perms(&pm.path, perms).map_err(|e| e.to_string())?; } - if let Some(size) = wstat.n_bytes { - if pm.meta.is_dir() { - return Err(E_PERMISSION_DENIED.to_string()); - } - - let f = OpenOptions::new() - .write(true) - .open(&pm.path) - .map_err(|e| e.to_string())?; - f.set_len(size).map_err(|e| e.to_string())?; + if let Some(n_bytes) = wstat.n_bytes { + io_set_len(&pm.path, n_bytes).map_err(|e| e.to_string())?; } if let Some(name) = wstat.name { - let parent = pm.path.parent().unwrap(); - let new_path = parent.join(&name); - fs::rename(&pm.path, &new_path).map_err(|e| e.to_string())?; - + io_rename(&pm.path, &name).map_err(|e| e.to_string())?; self.try_sync_dir(self.parent_qid(qid))?; } @@ -343,15 +305,15 @@ impl Serve9p for LocalProxyFs { self.try_sync_file(qid)?; } - let is_dir = self.ft.with_file(qid, |f| f.aux.meta.is_dir())?; let path = self.path_for_qid(qid); - if is_dir { - fs::remove_dir(&path).map_err(|e| e.to_string())?; + let res = if self.is_dir(qid) { + fs::remove_dir(&path) } else { - fs::remove_file(&path).map_err(|e| e.to_string())?; - } + fs::remove_file(&path) + }; + res.map_err(|e| e.to_string())?; self.ft.remove(qid); Ok(()) @@ -369,39 +331,37 @@ impl Serve9p for LocalProxyFs { self.try_sync_dir(parent)?; } - let parent_path = self.path_for_qid(parent); - let path = parent_path.join(name); - if path.exists() { - return Err("file already exists".to_string()); - } - - let is_dir = perm.contains(Perm::DIRECTORY); - if is_dir { - fs::create_dir(&path).map_err(|e| e.to_string())?; - } else { - OpenOptions::new() - .create_new(true) - .write(true) - .open(&path) - .map_err(|e| e.to_string())?; - } - - let mode_bits = perm.bits() & 0o777; - let mut permissions = fs::metadata(&path) - .map_err(|e| e.to_string())? - .permissions(); - permissions.set_mode(mode_bits); - fs::set_permissions(&path, permissions).map_err(|e| e.to_string())?; + let path = self.path_for_qid(parent).join(name); + io_create(&path, perm).map_err(|e| e.to_string())?; // Sync to allow our normal logic to pick up the new file details and create the qid self.try_sync_dir(parent)?; - let qid = self.ft.walk_one(parent, name)?; Ok((qid, self.iounit)) } } +/// The metadata we store per-node in the file tree +#[derive(Debug, Clone)] +struct PathMeta { + path: PathBuf, + meta: Metadata, + needs_sync: bool, +} + +impl PathMeta { + fn new(path: PathBuf, meta: Metadata) -> Self { + let needs_sync = meta.is_dir(); + + Self { + path, + meta, + needs_sync, + } + } +} + fn perms_from_meta(meta: &Metadata) -> Perm { let mut perms = Perm::new(meta.permissions().mode() & 0o777); if meta.is_dir() { @@ -422,3 +382,62 @@ fn owner_and_group_from_meta(meta: &Metadata) -> (String, String) { (owner, group) } + +// io::Result returning functions for use in the Serve9p methods above. + +fn io_read(path: &Path, offset: usize, count: usize) -> io::Result> { + let mut f = OpenOptions::new().read(true).open(path)?; + f.seek(SeekFrom::Start(offset as u64))?; + + let mut buf = vec![0; count]; + let n = f.read(&mut buf)?; + buf.truncate(n); + + Ok(buf) +} + +fn io_write(path: &Path, offset: usize, data: &[u8]) -> io::Result { + let mut f = OpenOptions::new().write(true).open(path)?; + f.seek(SeekFrom::Start(offset as u64))?; + + f.write(data) +} + +fn io_set_perms(path: &Path, perms: Perm) -> io::Result<()> { + let mut permissions = fs::metadata(path)?.permissions(); + permissions.set_mode(perms.bits() & 0o777); + + fs::set_permissions(path, permissions) +} + +fn io_set_len(path: &Path, n_bytes: u64) -> io::Result<()> { + OpenOptions::new().write(true).open(path)?.set_len(n_bytes) +} + +fn io_rename(path: &Path, name: &str) -> io::Result<()> { + let parent = path + .parent() + .ok_or_else(|| io::Error::new(io::ErrorKind::NotFound, "no parent"))?; + + fs::rename(path, parent.join(name)) +} + +fn io_create(path: &Path, perm: Perm) -> io::Result<()> { + if path.exists() { + return Err(io::Error::new( + io::ErrorKind::AlreadyExists, + "file already exists", + )); + } + + if perm.contains(Perm::DIRECTORY) { + fs::create_dir(path)?; + } else { + fs::File::create_new(path)?; + } + + let mut permissions = fs::metadata(path)?.permissions(); + permissions.set_mode(perm.bits() & 0o777); + + fs::set_permissions(path, permissions) +} From ff0daf441b3dfe6e0f83b5a7236c1ea526db7720 Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Thu, 7 May 2026 07:51:27 +0100 Subject: [PATCH 7/7] Add tests for LocalProxyFs --- crates/ninep/src/util/local_proxy.rs | 352 ++++++++++++++++++++++++++- crates/ninep/src/util/ram.rs | 113 --------- 2 files changed, 345 insertions(+), 120 deletions(-) diff --git a/crates/ninep/src/util/local_proxy.rs b/crates/ninep/src/util/local_proxy.rs index 6c302a00..3c8107b1 100644 --- a/crates/ninep/src/util/local_proxy.rs +++ b/crates/ninep/src/util/local_proxy.rs @@ -55,8 +55,8 @@ impl LocalProxyFs { /// On-disk path and metadata for `qid`. /// /// Panics if called for an unknown qid - fn meta_for_qid(&self, qid: u64) -> PathMeta { - self.ft.with_file(qid, |f| f.aux.clone()).unwrap() + fn try_meta_for_qid(&self, qid: u64) -> Result { + self.ft.with_file(qid, |f| f.aux.clone()) } /// On-disk path for `qid`. @@ -84,8 +84,7 @@ impl LocalProxyFs { /// Check to see if the given node has been modified since we cached metadata for it. /// If we are unable to read metadata from disk, prune this node. fn modified_since_cache_or_prune(&self, qid: u64) -> Result { - let modified_since_cache = |qid: u64| -> io::Result { - let cached = self.meta_for_qid(qid); + let modified_since_cache = |cached: PathMeta| -> io::Result { let t_cached = cached.meta.modified()?; let current = fs::metadata(&cached.path)?; let t_current = current.modified()?; @@ -93,7 +92,8 @@ impl LocalProxyFs { Ok(t_current != t_cached) }; - match modified_since_cache(qid) { + let cached = self.try_meta_for_qid(qid)?; + match modified_since_cache(cached) { Ok(opt) => Ok(opt), Err(e) => { self.ft.remove(qid); @@ -103,7 +103,7 @@ impl LocalProxyFs { } fn try_sync_dir(&self, qid: u64) -> Result<()> { - let path = self.meta_for_qid(qid).path; + let path = self.try_meta_for_qid(qid)?.path; let mut on_disk = self.on_disk_entries_for(&path).map_err(|e| e.to_string())?; // Update known cache entries and prune entries that are no longer present on disk @@ -273,7 +273,7 @@ impl Serve9p for LocalProxyFs { return Err(E_PERMISSION_DENIED.to_string()); } - let pm = self.meta_for_qid(qid); + let pm = self.try_meta_for_qid(qid)?; if pm.meta.is_dir() { self.try_sync_dir(qid)?; } else { @@ -441,3 +441,341 @@ fn io_create(path: &Path, perm: Perm) -> io::Result<()> { fs::set_permissions(path, permissions) } + +#[cfg(test)] +mod tests { + use super::*; + use assert_fs::{ + TempDir, + prelude::{FileWriteStr, PathChild}, + }; + use simple_test_case::test_case; + use std::{ + fs, + os::unix::fs::{PermissionsExt, symlink}, + path::{Path, PathBuf}, + time::SystemTime, + }; + + const CID: ClientId = ClientId(0); + const ROOT_PATH: &str = "root"; + const HELLO_PATH: &str = "hello"; + const SUBDIR_PATH: &str = "subdir"; + const NESTED_PATH: &str = "subdir/nested"; + + struct FsWithTempDir { + tmp: TempDir, + root: PathBuf, + proxy_fs: LocalProxyFs, + } + + impl FsWithTempDir { + fn new() -> Self { + let tmp = TempDir::new().unwrap(); + let root = tmp.path().join(ROOT_PATH); + + fs::create_dir(&root).unwrap(); + fs::write(root.join(HELLO_PATH), b"hello world").unwrap(); + fs::create_dir(root.join(SUBDIR_PATH)).unwrap(); + fs::write(root.join(NESTED_PATH), b"nested").unwrap(); + + let proxy_fs = LocalProxyFs::new(&root).unwrap(); + + Self { + tmp, + root, + proxy_fs, + } + } + + fn chmod(&self, child_path: impl AsRef, mode: u32) -> io::Result<()> { + let path = self.root.join(child_path); + let mut perms = fs::metadata(&path)?.permissions(); + perms.set_mode(mode); + + fs::set_permissions(path, perms) + } + + fn symlink_in_root( + &self, + link_rel: impl AsRef, + target: impl AsRef, + ) -> io::Result<()> { + symlink(target, self.root.join(link_rel)) + } + + fn qid_for_path(&self, path: &str) -> u64 { + self.proxy_fs.ft.qid_for_path(&format!("/{path}")).unwrap() + } + } + + #[test] + fn new_rejects_non_directory_root() { + let tmp = TempDir::new().unwrap(); + let child = tmp.child("not-a-dir"); + child.write_str("content").unwrap(); + fs::write(&child, b"content").unwrap(); + + let err = LocalProxyFs::new(child.path()).unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::Other); + assert!(err.to_string().contains("proxy root must be a directory")); + } + + #[test] + fn new_populates_root_and_marks_root_as_synced() { + let FsWithTempDir { + tmp: _tmp, + proxy_fs, + .. + } = FsWithTempDir::new(); + + let ft = &proxy_fs.ft; + assert!(ft.qid_for_path(&format!("/{HELLO_PATH}")).is_some()); + let subdir_qid = ft.qid_for_path(&format!("/{SUBDIR_PATH}")).unwrap(); + + assert!(!proxy_fs.needs_sync(QID_ROOT)); + assert!(proxy_fs.needs_sync(subdir_qid)); + } + + #[test] + fn meta_if_under_root_handles_is_some_for_path_under_root() { + let t = FsWithTempDir::new(); + + assert!( + t.proxy_fs + .meta_if_under_root(&t.root.join(HELLO_PATH)) + .is_some() + ); + assert!( + t.proxy_fs + .meta_if_under_root(&t.root.join(SUBDIR_PATH)) + .is_some() + ); + } + + #[test] + fn meta_if_under_root_is_none_for_path_out_of_root() { + let t = FsWithTempDir::new(); + let outside = t.tmp.path().join("outside-file"); + fs::write(&outside, b"outside").unwrap(); + + assert!(t.proxy_fs.meta_if_under_root(&outside).is_none()); + } + + #[test] + fn meta_if_under_root_is_none_for_symlink_to_path_out_of_root() { + let t = FsWithTempDir::new(); + let outside = t.tmp.path().join("outside-file"); + fs::write(&outside, b"outside").unwrap(); + + t.symlink_in_root("escape", &outside).unwrap(); + assert!( + t.proxy_fs + .meta_if_under_root(&t.root.join("escape")) + .is_none() + ); + } + + #[test] + fn on_disk_entries_for_filters_symlinks_out_of_root() { + let t = FsWithTempDir::new(); + let outside = t.tmp.path().join("outside-file"); + fs::write(&outside, b"outside").unwrap(); + t.symlink_in_root("escape", &outside).unwrap(); + + let entries = t.proxy_fs.on_disk_entries_for(&t.root).unwrap(); + + assert!(entries.contains_key(HELLO_PATH)); + assert!(entries.contains_key(SUBDIR_PATH)); + assert!(!entries.contains_key("escape")); + } + + #[test] + fn modified_since_cache_or_prune_reports_changes() { + let FsWithTempDir { + tmp: _tmp, + root, + proxy_fs, + } = FsWithTempDir::new(); + let qid = proxy_fs.ft.qid_for_path(&format!("/{HELLO_PATH}")).unwrap(); + + let modified = proxy_fs.modified_since_cache_or_prune(qid); + assert_eq!(modified, Ok(false)); + + fs::File::open(root.join(HELLO_PATH)) + .unwrap() + .set_modified(SystemTime::UNIX_EPOCH) + .unwrap(); + + let modified = proxy_fs.modified_since_cache_or_prune(qid); + assert_eq!(modified, Ok(true)); + } + + #[test] + fn modified_since_cache_or_prune_prunes_when_entry_missing() { + let t = FsWithTempDir::new(); + let qid = t.qid_for_path(HELLO_PATH); + + fs::remove_file(t.root.join(HELLO_PATH)).unwrap(); + + assert!(t.proxy_fs.modified_since_cache_or_prune(qid).is_err()); + assert_eq!(t.proxy_fs.ft.stat(qid).unwrap_err(), E_UNKNOWN_FILE); + } + + #[test] + fn try_sync_file_updates_cached_metadata() { + let t = FsWithTempDir::new(); + let qid = t.qid_for_path(HELLO_PATH); + + fs::write(t.root.join(HELLO_PATH), b"hi").unwrap(); + t.chmod(HELLO_PATH, 0o600).unwrap(); + + t.proxy_fs.try_sync_file(qid).unwrap(); + + let stat = t.proxy_fs.ft.stat(qid).unwrap(); + assert_eq!(stat.n_bytes, 2); + assert_eq!(stat.perms.bits() & 0o777, 0o600); + } + + #[test] + fn try_sync_file_returns_unknown_file_for_missing_path() { + let t = FsWithTempDir::new(); + let qid = t.qid_for_path(HELLO_PATH); + + fs::remove_file(t.root.join(HELLO_PATH)).unwrap(); + + assert_eq!(t.proxy_fs.try_sync_file(qid).unwrap_err(), E_UNKNOWN_FILE); + } + + #[test] + fn try_sync_dir_prunes_missing_entries() { + let t = FsWithTempDir::new(); + let qid = t.qid_for_path(HELLO_PATH); + + fs::remove_file(t.root.join(HELLO_PATH)).unwrap(); + + t.proxy_fs.try_sync_dir(QID_ROOT).unwrap(); + + assert_eq!(t.proxy_fs.ft.stat(qid).unwrap_err(), E_UNKNOWN_FILE); + assert!(!t.proxy_fs.needs_sync(QID_ROOT)); + } + + #[test] + fn try_sync_dir_adds_entries_and_updates_existing_metadata() { + let t = FsWithTempDir::new(); + let hello_qid = t.qid_for_path(HELLO_PATH); + + t.chmod(HELLO_PATH, 0o600).unwrap(); + fs::write(t.root.join("new-file"), b"new").unwrap(); + fs::create_dir(t.root.join("new-dir")).unwrap(); + + t.proxy_fs.try_sync_dir(QID_ROOT).unwrap(); + + let hello_perms = t.proxy_fs.ft.stat(hello_qid).unwrap().perms; + assert_eq!(hello_perms.bits() & 0o777, 0o600); + + let new_file_qid = t.qid_for_path("new-file"); + let new_dir_qid = t.qid_for_path("new-dir"); + + assert!(!t.proxy_fs.needs_sync(QID_ROOT)); + assert!(!t.proxy_fs.needs_sync(new_file_qid)); + assert!(t.proxy_fs.needs_sync(new_dir_qid)); + } + + #[test] + fn open_works() { + let t = FsWithTempDir::new(); + let qid = t.qid_for_path(HELLO_PATH); + + assert_eq!( + t.proxy_fs.open(qid, Mode::READ, CID).unwrap(), + DEFAULT_IOUNIT + ); + assert_eq!( + t.proxy_fs.open(42, Mode::READ, CID).unwrap_err(), + E_UNKNOWN_FILE + ); + } + + #[test] + fn walk_one_works() { + let t = FsWithTempDir::new(); + let qid = t.qid_for_path(HELLO_PATH); + + assert_eq!( + t.proxy_fs.walk_one(QID_ROOT, HELLO_PATH, CID).unwrap().path, + qid + ); + } + + #[test_case(0, 3, b"hel"; "from start")] // typos:ignore + #[test_case(2, 5, b"llo w"; "middle slice")] + #[test_case(11, 5, b""; "offset at end")] + #[test_case(100, 5, b""; "offset beyond end")] + #[test] + fn read_returns_expected_data(offset: usize, count: usize, expected: &[u8]) { + let t = FsWithTempDir::new(); + let qid = t.qid_for_path(HELLO_PATH); + + match t.proxy_fs.read(qid, offset, count, CID).unwrap() { + ReadOutcome::Immediate(data) => assert_eq!(&data, expected), + ReadOutcome::Blocked(_) => { + panic!("LocalProxyFs should always return immediate read data") + } + } + } + + #[test] + fn read_dir_returns_children_for_directory() { + let t = FsWithTempDir::new(); + + let mut root_children = t + .proxy_fs + .read_dir(QID_ROOT, CID) + .unwrap() + .into_iter() + .map(|s| s.name) + .collect::>(); + root_children.sort(); + + assert_eq!( + root_children, + vec![HELLO_PATH.to_string(), SUBDIR_PATH.to_string()] + ); + + let qid = t.qid_for_path(SUBDIR_PATH); + let nested_children = t.proxy_fs.read_dir(qid, CID).unwrap(); + assert_eq!(nested_children.len(), 1); + assert_eq!(nested_children[0].name, "nested"); + } + + #[test_case(1, b"ZZ", b"hZZlo world"; "overwrite existing bytes")] + #[test_case(11, b"X", b"hello worldX"; "append at end")] + #[test_case(0, b"", b"hello world"; "empty write")] + #[test_case(12, b"X", b"hello world\0X"; "past current end")] + #[test] + fn write_updates_content(offset: usize, payload: &[u8], expected: &[u8]) { + let t = FsWithTempDir::new(); + let qid = t.qid_for_path(HELLO_PATH); + + let n = t + .proxy_fs + .write(qid, offset, payload.to_vec(), CID) + .unwrap(); + assert_eq!(n, payload.len()); + + let path = t.proxy_fs.try_meta_for_qid(qid).unwrap().path; + let data = fs::read(path).unwrap(); + assert_eq!(&data, expected); + } + + #[test] + fn stat_works() { + let t = FsWithTempDir::new(); + let qid = t.qid_for_path(HELLO_PATH); + + let stat = t.proxy_fs.stat(qid, CID).unwrap(); + assert_eq!(stat.qid.path, qid); + assert_eq!(stat.name, HELLO_PATH); + } +} diff --git a/crates/ninep/src/util/ram.rs b/crates/ninep/src/util/ram.rs index 364c9e97..9cea78a3 100644 --- a/crates/ninep/src/util/ram.rs +++ b/crates/ninep/src/util/ram.rs @@ -129,7 +129,6 @@ impl Serve9p for RamFs { #[cfg(test)] mod tests { use super::*; - use crate::sansio::server::E_ILLEGAL_CREATE_NAME; use simple_test_case::test_case; const CID: ClientId = ClientId(0); @@ -163,19 +162,6 @@ mod tests { assert_eq!(fs.walk_one(0, "test", CID).unwrap(), qid); } - #[test_case(99, "test"; "unknown parent")] - #[test_case(0, "missing"; "unknown child")] - #[test] - fn walk_one_unknown_returns_error(parent_qid: u64, child: &str) { - let fs = RamFs::new("user", "group"); - let _ = add_file(&fs, 0, "test", b""); - - assert_eq!( - fs.walk_one(parent_qid, child, CID).unwrap_err(), - E_UNKNOWN_FILE - ); - } - #[test_case(0, 3, b"abc".to_vec(); "from start")] #[test_case(2, 2, b"cd".to_vec(); "middle slice")] #[test_case(6, 5, b"".to_vec(); "offset at end")] @@ -191,13 +177,6 @@ mod tests { } } - #[test] - fn read_unknown_returns_error() { - let fs = RamFs::new("user", "group"); - - assert_eq!(fs.read(1, 0, 1, CID).unwrap_err(), E_UNKNOWN_FILE); - } - #[test] fn read_dir_returns_children_for_directory() { let fs = RamFs::new("user", "group"); @@ -226,13 +205,6 @@ mod tests { assert_eq!(nested_children[0].name, "nested"); } - #[test] - fn read_dir_unknown_returns_error() { - let fs = RamFs::new("user", "group"); - - assert_eq!(fs.read_dir(1, CID).unwrap_err(), E_UNKNOWN_FILE); - } - #[test_case(1, b"ZZ".to_vec(), b"aZZ".to_vec(); "overwrite existing bytes")] #[test_case(3, b"X".to_vec(), b"abcX".to_vec(); "append at end")] #[test_case(0, b"".to_vec(), b"abc".to_vec(); "empty write")] @@ -268,13 +240,6 @@ mod tests { assert_eq!(data, b"abc".to_vec()); } - #[test] - fn write_unknown_returns_error() { - let fs = RamFs::new("user", "group"); - - assert_eq!(fs.write(1, 0, vec![1], CID).unwrap_err(), E_UNKNOWN_FILE); - } - #[test] fn stat_works() { let fs = RamFs::new("user", "group"); @@ -285,13 +250,6 @@ mod tests { assert_eq!(stat.name, "test"); } - #[test] - fn stat_unknown_returns_error() { - let fs = RamFs::new("user", "group"); - - assert_eq!(fs.stat(1, CID).unwrap_err(), E_UNKNOWN_FILE); - } - #[test] fn write_stat_updates_file_metadata() { let fs = RamFs::new("user", "group"); @@ -313,33 +271,6 @@ mod tests { assert_eq!(stat.n_bytes, 123); } - #[test] - fn write_stat_mismatched_qid_returns_permission_denied() { - let fs = RamFs::new("user", "group"); - let qid = add_file(&fs, 0, "test", b""); - - let wstat = WStat { - qid: Qid::file(qid.path + 1), - ..Default::default() - }; - - assert_eq!( - fs.write_stat(qid.path, wstat, CID).unwrap_err(), - E_PERMISSION_DENIED - ); - } - - #[test] - fn write_stat_unknown_returns_error() { - let fs = RamFs::new("user", "group"); - let wstat = WStat { - qid: Qid::file(1), - ..Default::default() - }; - - assert_eq!(fs.write_stat(1, wstat, CID).unwrap_err(), E_UNKNOWN_FILE); - } - #[test] fn remove_prunes_subtree() { let fs = RamFs::new("user", "group"); @@ -355,13 +286,6 @@ mod tests { assert_eq!(fs.stat(child.path, CID).unwrap_err(), E_UNKNOWN_FILE); } - #[test] - fn remove_unknown_is_ok() { - let fs = RamFs::new("user", "group"); - - assert!(fs.remove(1, CID).is_ok()); - } - #[test_case(Perm::OWNER_READ, FileType::FILE, "file"; "file create")] #[test_case(Perm::DIRECTORY | Perm::OWNER_READ, FileType::DIRECTORY, "dir"; "directory create")] #[test] @@ -378,41 +302,4 @@ mod tests { assert_eq!(qid.ty, expected_ty); assert_eq!(fs.walk_one(0, name, CID).unwrap().path, qid.path); } - - #[test] - fn create_unknown_parent_returns_error() { - let fs = RamFs::new("user", "group"); - - assert_eq!( - fs.create(999, "test", Perm::OWNER_READ, Mode::READ, CID) - .unwrap_err(), - E_UNKNOWN_FILE - ); - } - - #[test_case("."; "single dot")] - #[test_case(".."; "double dot")] - #[test] - fn create_rejects_illegal_names(name: &str) { - let fs = RamFs::new("user", "group"); - - assert_eq!( - fs.create(0, name, Perm::OWNER_READ, Mode::READ, CID) - .unwrap_err(), - E_ILLEGAL_CREATE_NAME - ); - } - - #[test] - fn create_rejects_duplicate_names() { - let fs = RamFs::new("user", "group"); - fs.create(0, "dup", Perm::OWNER_READ, Mode::READ, CID) - .unwrap(); - - assert_eq!( - fs.create(0, "dup", Perm::OWNER_READ, Mode::READ, CID) - .unwrap_err(), - "file already exists" - ); - } }