From 931a6db65c45f16ba3277cf06ebc938b065a75ee Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Wed, 28 Aug 2024 13:04:46 -0600 Subject: [PATCH] unftp-sbe-fs: don't panic during Filesystem::new Filesystem::new will now return an error if it can't open the root directory. However, ServerExt::with_fs will still panic because the ServerBuilder is required to be infalliable. --- .../unftp-sbe-fs/examples/cap-ftpd-worker.rs | 2 +- crates/unftp-sbe-fs/src/ext.rs | 5 ++++- crates/unftp-sbe-fs/src/lib.rs | 6 +++--- crates/unftp-sbe-fs/src/tests.rs | 18 +++++++++--------- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/crates/unftp-sbe-fs/examples/cap-ftpd-worker.rs b/crates/unftp-sbe-fs/examples/cap-ftpd-worker.rs index d78da87e..01bce7d3 100644 --- a/crates/unftp-sbe-fs/examples/cap-ftpd-worker.rs +++ b/crates/unftp-sbe-fs/examples/cap-ftpd-worker.rs @@ -241,7 +241,7 @@ async fn main() { let auth = Arc::new(JsonFileAuthenticator::from_file(args[1].clone()).unwrap()); // XXX This would be a lot easier if the libunftp API allowed creating the // storage just before calling service. - let storage = Mutex::new(Some(Filesystem::new(std::env::temp_dir()))); + let storage = Mutex::new(Some(Filesystem::new(std::env::temp_dir()).unwrap())); let sgen = Box::new(move || storage.lock().unwrap().take().unwrap()); let mut sb = libunftp::ServerBuilder::with_authenticator(sgen, auth); diff --git a/crates/unftp-sbe-fs/src/ext.rs b/crates/unftp-sbe-fs/src/ext.rs index c55feadd..ed10d94d 100644 --- a/crates/unftp-sbe-fs/src/ext.rs +++ b/crates/unftp-sbe-fs/src/ext.rs @@ -19,7 +19,10 @@ pub trait ServerExt { let p = path.into(); libunftp::ServerBuilder::new(Box::new(move || { let p = &p.clone(); - Filesystem::new(p) + match Filesystem::new(p) { + Ok(fs) => fs, + Err(e) => panic!("Cannot open file system root {}: {}", p.display(), e), + } })) } } diff --git a/crates/unftp-sbe-fs/src/lib.rs b/crates/unftp-sbe-fs/src/lib.rs index fb0096f4..33a36223 100644 --- a/crates/unftp-sbe-fs/src/lib.rs +++ b/crates/unftp-sbe-fs/src/lib.rs @@ -80,11 +80,11 @@ impl Filesystem { /// Create a new Filesystem backend, with the given root. No operations can take place outside /// of the root. For example, when the `Filesystem` root is set to `/srv/ftp`, and a client /// asks for `hello.txt`, the server will send it `/srv/ftp/hello.txt`. - pub fn new>(root: P) -> Self { + pub fn new>(root: P) -> io::Result { let path = root.into(); let aa = cap_std::ambient_authority(); - let root_fd = Arc::new(cap_std::fs::Dir::open_ambient_dir(&path, aa).unwrap()); - Filesystem { root_fd, root: path } + let root_fd = Arc::new(cap_std::fs::Dir::open_ambient_dir(&path, aa)?); + Ok(Filesystem { root_fd, root: path }) } } diff --git a/crates/unftp-sbe-fs/src/tests.rs b/crates/unftp-sbe-fs/src/tests.rs index 7e886405..e82ffb1a 100644 --- a/crates/unftp-sbe-fs/src/tests.rs +++ b/crates/unftp-sbe-fs/src/tests.rs @@ -23,7 +23,7 @@ fn fs_stat() { let meta = file.metadata().unwrap(); // Create a filesystem StorageBackend with the directory containing our temp file as root - let fs = Filesystem::new(&root); + let fs = Filesystem::new(&root).unwrap(); // Since the filesystem backend is based on futures, we need a runtime to run it let rt = tokio::runtime::Builder::new_current_thread().build().unwrap(); @@ -48,7 +48,7 @@ fn fs_list() { let meta = file.metadata().unwrap(); // Create a filesystem StorageBackend with our root dir - let fs = Filesystem::new(root.path()); + let fs = Filesystem::new(root.path()).unwrap(); // Since the filesystem backend is based on futures, we need a runtime to run it let rt = tokio::runtime::Builder::new_current_thread().build().unwrap(); @@ -74,7 +74,7 @@ fn fs_list_fmt() { let relpath = path.strip_prefix(root.path()).unwrap(); // Create a filesystem StorageBackend with our root dir - let fs = Filesystem::new(root.path()); + let fs = Filesystem::new(root.path()).unwrap(); let rt = tokio::runtime::Builder::new_current_thread().build().unwrap(); let my_list = rt.block_on(fs.list_fmt(&DefaultUser {}, "/")).unwrap(); @@ -96,7 +96,7 @@ fn fs_get() { file.write_all(data).unwrap(); let filename = path.file_name().unwrap(); - let fs = Filesystem::new(&root); + let fs = Filesystem::new(&root).unwrap(); // Since the filesystem backend is based on futures, we need a runtime to run it let rt = Runtime::new().unwrap(); @@ -117,7 +117,7 @@ fn fs_get() { fn fs_put() { let root = std::env::temp_dir(); let orig_content = b"hallo"; - let fs = Filesystem::new(&root); + let fs = Filesystem::new(&root).unwrap(); // Since the Filesystem StorageBackend is based on futures, we need a runtime to run them // to completion @@ -178,7 +178,7 @@ fn fileinfo_fmt() { #[test] fn fs_mkd() { let root = tempfile::TempDir::new().unwrap().into_path(); - let fs = Filesystem::new(&root); + let fs = Filesystem::new(&root).unwrap(); let new_dir_name = "bla"; // Since the Filesystem StorageBackend is based on futures, we need a runtime to run them @@ -203,7 +203,7 @@ fn fs_rename_file() { // to completion let rt = Runtime::new().unwrap(); - let fs = Filesystem::new(&root); + let fs = Filesystem::new(&root).unwrap(); let r = rt.block_on(fs.rename(&DefaultUser {}, &old_filename, &new_filename)); assert!(r.is_ok()); @@ -225,7 +225,7 @@ fn fs_rename_dir() { // to completion let rt = Runtime::new().unwrap(); - let fs = Filesystem::new(&root); + let fs = Filesystem::new(&root).unwrap(); let r = rt.block_on(fs.rename(&DefaultUser {}, &old_dir, &new_dir)); assert!(r.is_ok()); @@ -247,7 +247,7 @@ fn fs_md5() { let mut file = file.as_file(); // Create a filesystem StorageBackend with the directory containing our temp file as root - let fs = Filesystem::new(&root); + let fs = Filesystem::new(&root).unwrap(); file.write_all(DATA.as_bytes()).unwrap(); // Since the filesystem backend is based on futures, we need a runtime to run it