Skip to content

Commit

Permalink
unftp-sbe-fs: don't panic during Filesystem::new
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
asomers committed Aug 28, 2024
1 parent e0e7b33 commit 931a6db
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 14 deletions.
2 changes: 1 addition & 1 deletion crates/unftp-sbe-fs/examples/cap-ftpd-worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion crates/unftp-sbe-fs/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}))
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/unftp-sbe-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<P: Into<PathBuf>>(root: P) -> Self {
pub fn new<P: Into<PathBuf>>(root: P) -> io::Result<Self> {
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 })
}
}

Expand Down
18 changes: 9 additions & 9 deletions crates/unftp-sbe-fs/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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());

Expand All @@ -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());

Expand All @@ -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
Expand Down

0 comments on commit 931a6db

Please sign in to comment.