fix: stabilize core error handling

Remove production panic and unwrap paths from core modules.

Propagate bookmark encryption failures, harden file watcher and temp mapped file handling, and clean up dead code in shared utilities.

src/activity_manager.rs

@@ -94,7 +94,9 @@ impl ActivityManager {
                 // local dir is remote_args.local_dir if set, otherwise current dir
                 let local_dir = remote_args
                     .local_dir
-                    .unwrap_or_else(|| env::current_dir().unwrap());
+                    .map(Ok)
+                    .unwrap_or_else(env::current_dir)
+                    .map_err(|err| format!("Could not resolve current directory: {err}"))?;
                 debug!("host bridge is None, setting local dir to {:?}", local_dir,);
 
                 self.set_host_params(
@@ -143,27 +145,26 @@ impl ActivityManager {
 
         match host {
             HostParams::HostBridge(HostBridgeParams::Localhost(path)) => {
-                self.context
+                self.context_mut()?
-                    .as_mut()
-                    .unwrap()
                     .set_host_bridge_params(HostBridgeParams::Localhost(path));
             }
             HostParams::HostBridge(HostBridgeParams::Remote(_, _)) => {
-                let (protocol, params) = remote_params.unwrap();
+                let (protocol, params) = remote_params.ok_or_else(|| {
-                self.context
+                    String::from("Missing remote parameters for host bridge configuration")
-                    .as_mut()
+                })?;
-                    .unwrap()
+                self.context_mut()?
                     .set_host_bridge_params(HostBridgeParams::Remote(protocol, params));
             }
             HostParams::Remote(_) => {
-                let (protocol, params) = remote_params.unwrap();
+                let (protocol, params) = remote_params
+                    .ok_or_else(|| String::from("Missing remote parameters for remote host"))?;
                 let params = FileTransferParams {
                     local_path: remote_local_path,
                     remote_path: remote_remote_path,
                     protocol,
                     params,
                 };
-                self.context.as_mut().unwrap().set_remote_params(params);
+                self.context_mut()?.set_remote_params(params);
             }
         }
         Ok(())
@@ -185,8 +186,10 @@ impl ActivityManager {
             ) && params.generic_params().is_some()
             {
                 // * if protocol is SCP or SFTP check whether a SSH key is registered for this remote, in case not ask password
-                let storage = SshKeyStorage::from(self.context.as_ref().unwrap().config());
+                let storage = SshKeyStorage::from(self.context_ref()?.config());
-                let generic_params = params.generic_params().unwrap();
+                let generic_params = params.generic_params().ok_or_else(|| {
+                    String::from("Missing generic parameters for SSH password resolution")
+                })?;
                 let username = generic_params
                     .username
                     .clone()
@@ -219,7 +222,7 @@ impl ActivityManager {
 
     /// Prompt user for password to set into params.
     fn prompt_password(&mut self, params: &mut ProtocolParams) -> Result<(), String> {
-        let ctx = self.context.as_mut().unwrap();
+        let ctx = self.context_mut()?;
         let prompt = format!("Password for {}: ", params.host_name());
 
         match tty::read_secret_from_tty(ctx.terminal(), prompt) {
@@ -244,7 +247,7 @@ impl ActivityManager {
         bookmark_name: &str,
         password: Option<&str>,
     ) -> Result<(), String> {
-        if let Some(bookmarks_client) = self.context.as_mut().unwrap().bookmarks_client_mut() {
+        if let Some(bookmarks_client) = self.context_mut()?.bookmarks_client_mut() {
             let params = match bookmarks_client.get_bookmark(bookmark_name) {
                 None => {
                     return Err(format!(
@@ -269,6 +272,18 @@ impl ActivityManager {
         }
     }
 
+    fn context_mut(&mut self) -> Result<&mut Context, String> {
+        self.context
+            .as_mut()
+            .ok_or_else(|| String::from("Activity manager context is not initialized"))
+    }
+
+    fn context_ref(&self) -> Result<&Context, String> {
+        self.context
+            .as_ref()
+            .ok_or_else(|| String::from("Activity manager context is not initialized"))
+    }
+
     ///
     /// Loop for activity manager. You need to provide the activity to start with
     /// Returns the exitcode

src/cli/remote.rs

@@ -73,10 +73,16 @@ impl TryFrom<&Args> for RemoteArgs {
 
         // set args based on hosts len
         if hosts.len() == 1 {
-            remote_args.remote = hosts.pop().unwrap();
+            remote_args.remote = hosts
+                .pop()
+                .ok_or_else(|| String::from("Missing remote host configuration"))?;
         } else if hosts.len() == 2 {
-            remote_args.host_bridge = hosts.pop().unwrap();
+            remote_args.host_bridge = hosts
-            remote_args.remote = hosts.pop().unwrap();
+                .pop()
+                .ok_or_else(|| String::from("Missing host-bridge configuration"))?;
+            remote_args.remote = hosts
+                .pop()
+                .ok_or_else(|| String::from("Missing remote host configuration"))?;
         }
 
         Ok(remote_args)

src/explorer/builder.rs

@@ -24,7 +24,7 @@ impl FileExplorerBuilder {
 
     /// Take FileExplorer out of builder
     pub fn build(&mut self) -> FileExplorer {
-        self.explorer.take().unwrap()
+        self.explorer.take().unwrap_or_default()
     }
 
     /// Enable HIDDEN_FILES option

src/explorer/formatter.rs

@@ -108,7 +108,7 @@ impl CallChainBlock {
             None => {
                 self.next_block = Some(Box::new(CallChainBlock::new(
                     func, prefix, fmt_len, fmt_extra,
-                )))
+                )));
             }
             Some(block) => block.push(func, prefix, fmt_len, fmt_extra),
         }
@@ -375,23 +375,20 @@ impl Formatter {
             // Add to cur str, prefix and the key value
             //format!("{cur_str}{prefix}{size:10}", size = size.display().si())
         } else if fsentry.metadata().symlink.is_some() {
-            let size = ByteSize(
+            match fsentry.metadata().symlink.as_ref() {
-                fsentry
+                Some(symlink) => {
-                    .metadata()
+                    let size = ByteSize(symlink.to_string_lossy().len() as u64);
-                    .symlink
+                    let mut fmt = size.display().si().to_string();
-                    .as_ref()
+                    // pad with up to len 10
-                    .unwrap()
+                    let pad = 10usize.saturating_sub(fmt.len());
-                    .to_string_lossy()
+                    for _ in 0..pad {
-                    .len() as u64,
+                        fmt.push(' ');
-            );
+                    }
-            let mut fmt = size.display().si().to_string();
-            // pad with up to len 10
-            let pad = 10usize.saturating_sub(fmt.len());
-            for _ in 0..pad {
-                fmt.push(' ');
-            }
 
-            format!("{cur_str}{prefix}{fmt}")
+                    format!("{cur_str}{prefix}{fmt}")
+                }
+                None => format!("{cur_str}{prefix}          "),
+            }
         } else {
             // Add to cur str, prefix and the key value
             format!("{cur_str}{prefix}          ")
@@ -476,12 +473,14 @@ impl Formatter {
         let mut last_index: usize = 0;
         // Match fmt str against regex
         for regex_match in FMT_KEY_REGEX.captures_iter(fmt_str) {
-            // Get match index (unwrap is safe, since always exists)
+            let Some(full_match) = regex_match.get(0) else {
-            let index: usize = fmt_str.find(&regex_match[0]).unwrap();
+                continue;
+            };
+            let index: usize = full_match.start();
             // Get prefix
             let prefix: String = String::from(&fmt_str[last_index..index]);
             // Increment last index (sum prefix lenght and the length of the key)
-            last_index += prefix.len() + regex_match[0].len();
+            last_index += prefix.len() + full_match.as_str().len();
             // Match attributes
             match FMT_ATTR_REGEX.captures(&regex_match[1]) {
                 Some(regex_match) => {
@@ -516,7 +515,7 @@ impl Formatter {
                     match callchain.as_mut() {
                         None => {
                             callchain =
-                                Some(CallChainBlock::new(callback, prefix, fmt_len, fmt_extra))
+                                Some(CallChainBlock::new(callback, prefix, fmt_len, fmt_extra));
                         }
                         Some(chain_block) => chain_block.push(callback, prefix, fmt_len, fmt_extra),
                     }

src/host.rs

@@ -19,7 +19,6 @@ pub type HostResult<T> = Result<T, HostError>;
 
 /// HostErrorType provides an overview of the specific host error
 #[derive(Error, Debug)]
-#[allow(dead_code)]
 pub enum HostErrorType {
     #[error("No such file or directory")]
     NoSuchFileOrDirectory,
@@ -37,6 +36,7 @@ pub enum HostErrorType {
     ExecutionFailed,
     #[error("Could not delete file")]
     DeleteFailed,
+    #[cfg(win)]
     #[error("Not implemented")]
     NotImplemented,
     #[error("remote fs error: {0}")]

src/host/remote_bridged/temp_mapped_file.rs

@@ -16,23 +16,20 @@ pub struct TempMappedFile {
 
 impl Write for TempMappedFile {
     fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
-        let rc = self.write_hnd()?;
+        let mut handle = self.write_hnd()?;
-        let mut ref_mut = rc.lock().unwrap();
+        handle.write(buf)
-        ref_mut.as_mut().unwrap().write(buf)
     }
 
     fn flush(&mut self) -> std::io::Result<()> {
-        let rc = self.write_hnd()?;
+        let mut handle = self.write_hnd()?;
-        let mut ref_mut = rc.lock().unwrap();
+        handle.flush()
-        ref_mut.as_mut().unwrap().flush()
     }
 }
 
 impl Read for TempMappedFile {
     fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
-        let rc = self.read_hnd()?;
+        let mut handle = self.read_hnd()?;
-        let mut ref_mut = rc.lock().unwrap();
+        handle.read(buf)
-        ref_mut.as_mut().unwrap().read(buf)
     }
 }
 
@@ -57,7 +54,13 @@ impl TempMappedFile {
     /// Must be called
     pub fn sync(&mut self) -> HostResult<()> {
         {
-            let mut lock = self.handle.lock().unwrap();
+            let mut lock = self.lock_handle().map_err(|e| {
+                HostError::new(
+                    HostErrorType::FileNotAccessible,
+                    Some(e),
+                    self.tempfile.path(),
+                )
+            })?;
 
             if let Some(hnd) = lock.take() {
                 hnd.sync_all().map_err(|e| {
@@ -73,28 +76,62 @@ impl TempMappedFile {
         Ok(())
     }
 
-    fn write_hnd(&mut self) -> io::Result<Arc<Mutex<Option<File>>>> {
+    fn write_hnd(&mut self) -> io::Result<FileHandle<'_>> {
-        {
+        let mut lock = self.lock_handle()?;
-            let mut lock = self.handle.lock().unwrap();
+        if lock.is_none() {
-            if lock.is_none() {
+            let hnd = File::create(self.tempfile.path())?;
-                let hnd = File::create(self.tempfile.path())?;
+            lock.replace(hnd);
-                lock.replace(hnd);
-            }
         }
 
-        Ok(self.handle.clone())
+        Ok(FileHandle::new(lock))
     }
 
-    fn read_hnd(&mut self) -> io::Result<Arc<Mutex<Option<File>>>> {
+    fn read_hnd(&mut self) -> io::Result<FileHandle<'_>> {
-        {
+        let mut lock = self.lock_handle()?;
-            let mut lock = self.handle.lock().unwrap();
+        if lock.is_none() {
-            if lock.is_none() {
+            let hnd = File::open(self.tempfile.path())?;
-                let hnd = File::open(self.tempfile.path())?;
+            lock.replace(hnd);
-                lock.replace(hnd);
-            }
         }
 
-        Ok(self.handle.clone())
+        Ok(FileHandle::new(lock))
+    }
+
+    fn lock_handle(&self) -> io::Result<std::sync::MutexGuard<'_, Option<File>>> {
+        self.handle
+            .lock()
+            .map_err(|_| io::Error::other("temporary file handle lock poisoned"))
+    }
+}
+
+struct FileHandle<'a> {
+    guard: std::sync::MutexGuard<'a, Option<File>>,
+}
+
+impl<'a> FileHandle<'a> {
+    fn new(guard: std::sync::MutexGuard<'a, Option<File>>) -> Self {
+        Self { guard }
+    }
+
+    fn file_mut(&mut self) -> io::Result<&mut File> {
+        self.guard
+            .as_mut()
+            .ok_or_else(|| io::Error::other("temporary file handle is not initialized"))
+    }
+}
+
+impl Write for FileHandle<'_> {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+        self.file_mut()?.write(buf)
+    }
+
+    fn flush(&mut self) -> io::Result<()> {
+        self.file_mut()?.flush()
+    }
+}
+
+impl Read for FileHandle<'_> {
+    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+        self.file_mut()?.read(buf)
     }
 }
 

src/support/import_ssh_hosts.rs

@@ -52,7 +52,9 @@ pub fn import_ssh_hosts(ssh_config: Option<PathBuf>, keyring: bool) -> Result<()
         .flat_map(host_to_params)
         .for_each(|(name, params, identity_file_params)| {
             debug!("Adding bookmark for host: {name} with params: {params:?}");
-            bookmarks_client.add_bookmark(name, params, false);
+            if let Err(err) = bookmarks_client.add_bookmark(name, params, false) {
+                error!("Could not add imported bookmark: {err}");
+            }
 
             // add ssh key if any
             if let Some(identity_file_params) = identity_file_params {
@@ -255,8 +257,7 @@ mod tests {
 
     struct SshTestConfig {
         config: NamedTempFile,
-        #[allow(dead_code)]
+        _identity_file: NamedTempFile,
-        identity_file: NamedTempFile,
     }
 
     fn ssh_test_config() -> SshTestConfig {
@@ -320,7 +321,7 @@ Host test3
 
         SshTestConfig {
             config: file,
-            identity_file,
+            _identity_file: identity_file,
         }
     }
 }

src/system/bookmarks_client.rs

@@ -187,15 +187,15 @@ impl BookmarksClient {
         name: S,
         params: FileTransferParams,
         save_password: bool,
-    ) {
+    ) -> Result<(), SerializerError> {
         let name: String = name.as_ref().to_string();
         if name.is_empty() {
             error!("Bookmark name is empty; ignoring add_bookmark request");
-            return;
+            return Ok(());
         }
         // Make bookmark
         info!("Added bookmark {}", name);
-        let mut host: Bookmark = self.make_bookmark(params);
+        let mut host: Bookmark = self.make_bookmark(params)?;
         // If not save_password, set secrets to `None`
         if !save_password {
             host.password = None;
@@ -205,6 +205,7 @@ impl BookmarksClient {
             }
         }
         self.hosts.bookmarks.insert(name, host);
+        Ok(())
     }
 
     /// Delete entry from bookmarks
@@ -226,9 +227,9 @@ impl BookmarksClient {
     }
 
     /// Add a new recent to bookmarks
-    pub fn add_recent(&mut self, params: FileTransferParams) {
+    pub fn add_recent(&mut self, params: FileTransferParams) -> Result<(), SerializerError> {
         // Make bookmark
-        let mut host: Bookmark = self.make_bookmark(params);
+        let mut host: Bookmark = self.make_bookmark(params)?;
         // Null password for recents
         host.password = None;
         if let Some(s3) = host.s3.as_mut() {
@@ -240,7 +241,7 @@ impl BookmarksClient {
             if *value == host {
                 debug!("Discarding recent since duplicated ({})", key);
                 // Don't save duplicates
-                return;
+                return Ok(());
             }
         }
         // If hosts size is bigger than self.recents_size; pop last
@@ -265,6 +266,7 @@ impl BookmarksClient {
         let name: String = fmt_time(SystemTime::now(), "ISO%Y%m%dT%H%M%S");
         info!("Saved recent host {}", name);
         self.hosts.recents.insert(name, host);
+        Ok(())
     }
 
     /// Delete entry from recents
@@ -329,27 +331,29 @@ impl BookmarksClient {
     }
 
     /// Make bookmark from credentials
-    fn make_bookmark(&self, params: FileTransferParams) -> Bookmark {
+    fn make_bookmark(&self, params: FileTransferParams) -> Result<Bookmark, SerializerError> {
         let mut bookmark: Bookmark = Bookmark::from(params);
         // Encrypt password
         if let Some(pwd) = bookmark.password {
-            bookmark.password = Some(self.encrypt_str(pwd.as_str()));
+            bookmark.password = Some(self.encrypt_str(pwd.as_str())?);
         }
         // Encrypt aws s3 params
         if let Some(s3) = bookmark.s3.as_mut() {
             if let Some(access_key) = s3.access_key.as_mut() {
-                *access_key = self.encrypt_str(access_key.as_str());
+                *access_key = self.encrypt_str(access_key.as_str())?;
             }
             if let Some(secret_access_key) = s3.secret_access_key.as_mut() {
-                *secret_access_key = self.encrypt_str(secret_access_key.as_str());
+                *secret_access_key = self.encrypt_str(secret_access_key.as_str())?;
             }
         }
-        bookmark
+        Ok(bookmark)
     }
 
     /// Encrypt provided string using AES-128. Encrypted buffer is then converted to BASE64
-    fn encrypt_str(&self, txt: &str) -> String {
+    fn encrypt_str(&self, txt: &str) -> Result<String, SerializerError> {
-        crypto::aes128_b64_crypt(self.key.as_str(), txt)
+        crypto::aes128_b64_crypt(self.key.as_str(), txt).map_err(|err| {
+            SerializerError::new_ex(SerializerErrorKind::Serialization, err.to_string())
+        })
     }
 
     /// Decrypt provided string using AES-128
@@ -403,24 +407,32 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add some bookmarks
-        client.add_bookmark(
+        assert!(
-            "raspberry",
+            client
-            make_generic_ftparams(
+                .add_bookmark(
-                FileTransferProtocol::Sftp,
+                    "raspberry",
-                "192.168.1.31",
+                    make_generic_ftparams(
-                22,
+                        FileTransferProtocol::Sftp,
-                "pi",
+                        "192.168.1.31",
-                Some("mypassword"),
+                        22,
-            ),
+                        "pi",
-            true,
+                        Some("mypassword"),
+                    ),
+                    true,
+                )
+                .is_ok()
+        );
+        assert!(
+            client
+                .add_recent(make_generic_ftparams(
+                    FileTransferProtocol::Sftp,
+                    "192.168.1.31",
+                    22,
+                    "pi",
+                    Some("mypassword"),
+                ))
+                .is_ok()
         );
-        client.add_recent(make_generic_ftparams(
-            FileTransferProtocol::Sftp,
-            "192.168.1.31",
-            22,
-            "pi",
-            Some("mypassword"),
-        ));
         let recent_key: String = String::from(client.iter_recents().next().unwrap());
         assert!(client.write_bookmarks().is_ok());
         let key: String = client.key.clone();
@@ -451,7 +463,11 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add s3 bookmark
-        client.add_bookmark("my-bucket", make_s3_ftparams(), true);
+        assert!(
+            client
+                .add_bookmark("my-bucket", make_s3_ftparams(), true)
+                .is_ok()
+        );
         // Verify bookmark
         let bookmark = client.get_bookmark("my-bucket").unwrap();
         assert_eq!(bookmark.protocol, FileTransferProtocol::AwsS3);
@@ -471,7 +487,11 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add s3 bookmark
-        client.add_bookmark("my-bucket", make_s3_ftparams(), false);
+        assert!(
+            client
+                .add_bookmark("my-bucket", make_s3_ftparams(), false)
+                .is_ok()
+        );
         // Verify bookmark
         let bookmark = client.get_bookmark("my-bucket").unwrap();
         assert_eq!(bookmark.protocol, FileTransferProtocol::AwsS3);
@@ -492,7 +512,7 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add s3 bookmark
-        client.add_recent(make_s3_ftparams());
+        assert!(client.add_recent(make_s3_ftparams()).is_ok());
         // Verify bookmark
         let bookmark = client.iter_recents().next().unwrap();
         let bookmark = client.get_recent(bookmark).unwrap();
@@ -515,27 +535,35 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add bookmark
-        client.add_bookmark(
+        assert!(
-            "raspberry",
+            client
-            make_generic_ftparams(
+                .add_bookmark(
-                FileTransferProtocol::Sftp,
+                    "raspberry",
-                "192.168.1.31",
+                    make_generic_ftparams(
-                22,
+                        FileTransferProtocol::Sftp,
-                "pi",
+                        "192.168.1.31",
-                Some("mypassword"),
+                        22,
-            ),
+                        "pi",
-            true,
+                        Some("mypassword"),
+                    ),
+                    true,
+                )
+                .is_ok()
         );
-        client.add_bookmark(
+        assert!(
-            "raspberry2",
+            client
-            make_generic_ftparams(
+                .add_bookmark(
-                FileTransferProtocol::Sftp,
+                    "raspberry2",
-                "192.168.1.31",
+                    make_generic_ftparams(
-                22,
+                        FileTransferProtocol::Sftp,
-                "pi",
+                        "192.168.1.31",
-                Some("mypassword2"),
+                        22,
-            ),
+                        "pi",
-            true,
+                        Some("mypassword2"),
+                    ),
+                    true,
+                )
+                .is_ok()
         );
         // Iter
         assert_eq!(client.iter_bookmarks().count(), 2);
@@ -564,16 +592,20 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add bookmark with empty name should be silently ignored
-        client.add_bookmark(
+        assert!(
-            "",
+            client
-            make_generic_ftparams(
+                .add_bookmark(
-                FileTransferProtocol::Sftp,
+                    "",
-                "192.168.1.31",
+                    make_generic_ftparams(
-                22,
+                        FileTransferProtocol::Sftp,
-                "pi",
+                        "192.168.1.31",
-                Some("mypassword"),
+                        22,
-            ),
+                        "pi",
-            true,
+                        Some("mypassword"),
+                    ),
+                    true,
+                )
+                .is_ok()
         );
         // No bookmark should have been added
         assert_eq!(client.iter_bookmarks().count(), 0);
@@ -587,16 +619,20 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add bookmark
-        client.add_bookmark(
+        assert!(
-            "raspberry",
+            client
-            make_generic_ftparams(
+                .add_bookmark(
-                FileTransferProtocol::Sftp,
+                    "raspberry",
-                "192.168.1.31",
+                    make_generic_ftparams(
-                22,
+                        FileTransferProtocol::Sftp,
-                "pi",
+                        "192.168.1.31",
-                Some("mypassword"),
+                        22,
-            ),
+                        "pi",
-            false,
+                        Some("mypassword"),
+                    ),
+                    false,
+                )
+                .is_ok()
         );
         let bookmark = ftparams_to_tup(client.get_bookmark(&String::from("raspberry")).unwrap());
         assert_eq!(bookmark.0, String::from("192.168.1.31"));
@@ -615,13 +651,17 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add bookmark
-        client.add_recent(make_generic_ftparams(
+        assert!(
-            FileTransferProtocol::Sftp,
+            client
-            "192.168.1.31",
+                .add_recent(make_generic_ftparams(
-            22,
+                    FileTransferProtocol::Sftp,
-            "pi",
+                    "192.168.1.31",
-            Some("mypassword"),
+                    22,
-        ));
+                    "pi",
+                    Some("mypassword"),
+                ))
+                .is_ok()
+        );
         // Iter
         assert_eq!(client.iter_recents().count(), 1);
         let key: String = String::from(client.iter_recents().next().unwrap());
@@ -651,20 +691,28 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add bookmark
-        client.add_recent(make_generic_ftparams(
+        assert!(
-            FileTransferProtocol::Sftp,
+            client
-            "192.168.1.31",
+                .add_recent(make_generic_ftparams(
-            22,
+                    FileTransferProtocol::Sftp,
-            "pi",
+                    "192.168.1.31",
-            Some("mypassword"),
+                    22,
-        ));
+                    "pi",
-        client.add_recent(make_generic_ftparams(
+                    Some("mypassword"),
-            FileTransferProtocol::Sftp,
+                ))
-            "192.168.1.31",
+                .is_ok()
-            22,
+        );
-            "pi",
+        assert!(
-            Some("mypassword"),
+            client
-        ));
+                .add_recent(make_generic_ftparams(
+                    FileTransferProtocol::Sftp,
+                    "192.168.1.31",
+                    22,
+                    "pi",
+                    Some("mypassword"),
+                ))
+                .is_ok()
+        );
         // There should be only one recent
         assert_eq!(client.iter_recents().count(), 1);
     }
@@ -679,31 +727,43 @@ mod tests {
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 2, true).unwrap();
         // Add recent, wait 1 second for each one (cause the name depends on time)
         // 1
-        client.add_recent(make_generic_ftparams(
+        assert!(
-            FileTransferProtocol::Sftp,
+            client
-            "192.168.1.1",
+                .add_recent(make_generic_ftparams(
-            22,
+                    FileTransferProtocol::Sftp,
-            "pi",
+                    "192.168.1.1",
-            Some("mypassword"),
+                    22,
-        ));
+                    "pi",
+                    Some("mypassword"),
+                ))
+                .is_ok()
+        );
         sleep(Duration::from_secs(1));
         // 2
-        client.add_recent(make_generic_ftparams(
+        assert!(
-            FileTransferProtocol::Sftp,
+            client
-            "192.168.1.2",
+                .add_recent(make_generic_ftparams(
-            22,
+                    FileTransferProtocol::Sftp,
-            "pi",
+                    "192.168.1.2",
-            Some("mypassword"),
+                    22,
-        ));
+                    "pi",
+                    Some("mypassword"),
+                ))
+                .is_ok()
+        );
         sleep(Duration::from_secs(1));
         // 3
-        client.add_recent(make_generic_ftparams(
+        assert!(
-            FileTransferProtocol::Sftp,
+            client
-            "192.168.1.3",
+                .add_recent(make_generic_ftparams(
-            22,
+                    FileTransferProtocol::Sftp,
-            "pi",
+                    "192.168.1.3",
-            Some("mypassword"),
+                    22,
-        ));
+                    "pi",
+                    Some("mypassword"),
+                ))
+                .is_ok()
+        );
         // Limit is 2
         assert_eq!(client.iter_recents().count(), 2);
         // Check that 192.168.1.1 has been removed
@@ -745,16 +805,20 @@ mod tests {
         let mut client: BookmarksClient =
             BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap();
         // Add bookmark with empty name should be silently ignored
-        client.add_bookmark(
+        assert!(
-            "",
+            client
-            make_generic_ftparams(
+                .add_bookmark(
-                FileTransferProtocol::Sftp,
+                    "",
-                "192.168.1.31",
+                    make_generic_ftparams(
-                22,
+                        FileTransferProtocol::Sftp,
-                "pi",
+                        "192.168.1.31",
-                Some("mypassword"),
+                        22,
-            ),
+                        "pi",
-            true,
+                        Some("mypassword"),
+                    ),
+                    true,
+                )
+                .is_ok()
         );
         // No bookmark should have been added
         assert_eq!(client.iter_bookmarks().count(), 0);

src/system/watcher.rs

@@ -27,6 +27,8 @@ pub enum FsWatcherError {
     PathNotWatched,
     #[error("unable to watch path, since it's already watched")]
     PathAlreadyWatched,
+    #[error("watcher event channel disconnected")]
+    Disconnected,
     #[error("unknown event: {0}")]
     UnknownEvent(&'static str),
     #[error("worker error: {0}")]
@@ -114,7 +116,7 @@ impl FsWatcher {
         let res = match self.receiver.recv_timeout(Duration::from_millis(1)) {
             Ok(res) => res,
             Err(RecvTimeoutError::Timeout) => return Ok(None),
-            Err(RecvTimeoutError::Disconnected) => panic!("File watcher died"),
+            Err(RecvTimeoutError::Disconnected) => return Err(FsWatcherError::Disconnected),
         };
 
         // convert event to FsChange

src/ui/activities/auth/bookmarks.rs

@@ -71,7 +71,10 @@ impl AuthActivity {
         };
 
         if let Some(bookmarks_cli) = self.bookmarks_client_mut() {
-            bookmarks_cli.add_bookmark(name.clone(), params, save_password);
+            if let Err(err) = bookmarks_cli.add_bookmark(name.clone(), params, save_password) {
+                self.mount_error(format!("Could not save bookmark: {err}"));
+                return;
+            }
             // Save bookmarks
             self.write_bookmarks();
             // Remove `name` from bookmarks if exists
@@ -121,7 +124,10 @@ impl AuthActivity {
             }
         };
         if let Some(bookmarks_cli) = self.bookmarks_client_mut() {
-            bookmarks_cli.add_recent(params);
+            if let Err(err) = bookmarks_cli.add_recent(params) {
+                self.mount_error(format!("Could not save recent host: {err}"));
+                return;
+            }
             // Save bookmarks
             self.write_bookmarks();
         }

src/ui/store.rs

@@ -9,14 +9,10 @@ use std::collections::HashMap;
 // -- store state
 
 /// Store state describes a value in the store
-#[allow(dead_code)]
 enum StoreState {
-    Str(String),     // String
+    Str(String),   // String
-    Signed(isize),   // Signed number
+    Boolean(bool), // Boolean value
-    Unsigned(usize), // Unsigned number
+    Flag,          // Empty value; used to work as a Flag (set unset)
-    Float(f64),      // Floating point number
-    Boolean(bool),   // Boolean value
-    Flag,            // Empty value; used to work as a Flag (set unset)
 }
 
 // -- store
@@ -28,7 +24,6 @@ pub(crate) struct Store {
     store: HashMap<String, StoreState>,
 }
 
-#[allow(dead_code)]
 impl Store {
     /// Initialize a new Store
     pub fn init() -> Self {
@@ -47,30 +42,6 @@ impl Store {
         }
     }
 
-    /// Get signed from store
-    pub fn get_signed(&self, key: &str) -> Option<isize> {
-        match self.store.get(key) {
-            Some(StoreState::Signed(i)) => Some(*i),
-            _ => None,
-        }
-    }
-
-    /// Get unsigned from store
-    pub fn get_unsigned(&self, key: &str) -> Option<usize> {
-        match self.store.get(key) {
-            Some(StoreState::Unsigned(u)) => Some(*u),
-            _ => None,
-        }
-    }
-
-    /// get float from store
-    pub fn get_float(&self, key: &str) -> Option<f64> {
-        match self.store.get(key) {
-            Some(StoreState::Float(f)) => Some(*f),
-            _ => None,
-        }
-    }
-
     /// get boolean from store
     pub fn get_boolean(&self, key: &str) -> Option<bool> {
         match self.store.get(key) {
@@ -91,22 +62,6 @@ impl Store {
         self.store.insert(key.to_string(), StoreState::Str(val));
     }
 
-    /// Set signed number
-    pub fn set_signed(&mut self, key: &str, val: isize) {
-        self.store.insert(key.to_string(), StoreState::Signed(val));
-    }
-
-    /// Set unsigned number
-    pub fn set_unsigned(&mut self, key: &str, val: usize) {
-        self.store
-            .insert(key.to_string(), StoreState::Unsigned(val));
-    }
-
-    /// Set floating point number
-    pub fn set_float(&mut self, key: &str, val: f64) {
-        self.store.insert(key.to_string(), StoreState::Float(val));
-    }
-
     /// Set boolean
     pub fn set_boolean(&mut self, key: &str, val: bool) {
         self.store.insert(key.to_string(), StoreState::Boolean(val));
@@ -118,46 +73,6 @@ impl Store {
     }
 
     // -- Consumers
-
-    /// Take string from store
-    pub fn take_string(&mut self, key: &str) -> Option<String> {
-        match self.store.remove(key) {
-            Some(StoreState::Str(s)) => Some(s),
-            _ => None,
-        }
-    }
-
-    /// Take signed from store
-    pub fn take_signed(&mut self, key: &str) -> Option<isize> {
-        match self.store.remove(key) {
-            Some(StoreState::Signed(i)) => Some(i),
-            _ => None,
-        }
-    }
-
-    /// Take unsigned from store
-    pub fn take_unsigned(&mut self, key: &str) -> Option<usize> {
-        match self.store.remove(key) {
-            Some(StoreState::Unsigned(u)) => Some(u),
-            _ => None,
-        }
-    }
-
-    /// Take float from store
-    pub fn take_float(&mut self, key: &str) -> Option<f64> {
-        match self.store.remove(key) {
-            Some(StoreState::Float(f)) => Some(f),
-            _ => None,
-        }
-    }
-
-    /// Take boolean from store
-    pub fn take_boolean(&mut self, key: &str) -> Option<bool> {
-        match self.store.remove(key) {
-            Some(StoreState::Boolean(b)) => Some(b),
-            _ => None,
-        }
-    }
 }
 
 #[cfg(test)]
@@ -174,39 +89,14 @@ mod tests {
         // Test string
         store.set_string("test", String::from("hello"));
         assert_eq!(*store.get_string("test").as_ref().unwrap(), "hello");
-        assert_eq!(store.take_string("test").unwrap(), "hello".to_string());
-        assert_eq!(store.take_string("test"), None);
-        // Test isize
-        store.set_signed("number", 3005);
-        assert_eq!(store.get_signed("number").unwrap(), 3005);
-        assert_eq!(store.take_signed("number").unwrap(), 3005);
-        assert_eq!(store.take_signed("number"), None);
-        store.set_signed("number", -123);
-        assert_eq!(store.get_signed("number").unwrap(), -123);
-        // Test usize
-        store.set_unsigned("unumber", 1024);
-        assert_eq!(store.get_unsigned("unumber").unwrap(), 1024);
-        assert_eq!(store.take_unsigned("unumber").unwrap(), 1024);
-        assert_eq!(store.take_unsigned("unumber"), None);
-        // Test float
-        store.set_float("float", 3.33);
-        assert_eq!(store.get_float("float").unwrap(), 3.33);
-        assert_eq!(store.take_float("float").unwrap(), 3.33);
-        assert_eq!(store.take_float("float"), None);
         // Test boolean
         store.set_boolean("bool", true);
         assert_eq!(store.get_boolean("bool").unwrap(), true);
-        assert_eq!(store.take_boolean("bool").unwrap(), true);
-        assert_eq!(store.take_boolean("bool"), None);
         // Test flag
         store.set("myflag");
         assert_eq!(store.isset("myflag"), true);
         // Test unexisting
         assert!(store.get_boolean("unexisting-key").is_none());
-        assert!(store.get_float("unexisting-key").is_none());
-        assert!(store.get_signed("unexisting-key").is_none());
-        assert!(store.get_signed("unexisting-key").is_none());
         assert!(store.get_string("unexisting-key").is_none());
-        assert!(store.get_unsigned("unexisting-key").is_none());
     }
 }

src/utils.rs

@@ -15,5 +15,4 @@ pub mod tty;
 pub mod ui;
 
 #[cfg(test)]
-#[allow(dead_code)]
 pub mod test_helpers;

src/utils/crypto.rs

@@ -17,19 +17,19 @@ const GCM_NONCE_LEN: usize = 12;
 
 /// Encrypt a string with AES-128-GCM. The output is `nonce || ciphertext`,
 /// encoded as standard Base64.
-pub fn aes128_b64_crypt(key: &str, input: &str) -> String {
+pub fn aes128_b64_crypt(key: &str, input: &str) -> Result<String, CryptoError> {
     let derived = derive_gcm_key(key);
     let cipher = Aes128Gcm::new(&derived.into());
     let nonce_bytes = Aes128Gcm::generate_nonce(&mut aes_gcm::aead::OsRng);
     let ciphertext = cipher
         .encrypt(&nonce_bytes, input.as_bytes())
-        .expect("AES-GCM encryption must not fail for valid inputs");
+        .map_err(|_| CryptoError::AesGcm)?;
 
     let mut combined = Vec::with_capacity(GCM_NONCE_LEN + ciphertext.len());
     combined.extend_from_slice(&nonce_bytes);
     combined.extend_from_slice(&ciphertext);
 
-    B64.encode(combined)
+    Ok(B64.encode(combined))
 }
 
 /// Decrypt a Base64-encoded string. Tries AES-128-GCM first; on failure falls
@@ -104,7 +104,7 @@ mod tests {
     fn test_encrypt_decrypt_roundtrip() {
         let key = "MYSUPERSECRETKEY";
         let input = "Hello world!";
-        let secret = aes128_b64_crypt(key, input);
+        let secret = aes128_b64_crypt(key, input).unwrap();
         let decrypted = aes128_b64_decrypt(key, &secret).unwrap();
         assert_eq!(decrypted, input);
     }
@@ -123,8 +123,8 @@ mod tests {
     fn test_different_encryptions_produce_different_ciphertexts() {
         let key = "MYSUPERSECRETKEY";
         let input = "Hello world!";
-        let secret1 = aes128_b64_crypt(key, input);
+        let secret1 = aes128_b64_crypt(key, input).unwrap();
-        let secret2 = aes128_b64_crypt(key, input);
+        let secret2 = aes128_b64_crypt(key, input).unwrap();
         // Due to random nonces, ciphertexts must differ
         assert_ne!(secret1, secret2);
         // But both decrypt to the same plaintext
@@ -134,7 +134,7 @@ mod tests {
 
     #[test]
     fn test_wrong_key_fails() {
-        let secret = aes128_b64_crypt("correct-key", "sensitive data");
+        let secret = aes128_b64_crypt("correct-key", "sensitive data").unwrap();
         assert!(aes128_b64_decrypt("wrong-key", &secret).is_err());
     }
 

src/utils/fmt.rs

@@ -61,8 +61,10 @@ pub fn fmt_path_elide_ex(p: &Path, width: usize, extra_len: usize) -> String {
             let mut ancestors = p.ancestors();
             let mut elided_path: PathBuf = PathBuf::new();
             // If ancestors_len's size is bigger than 2, push count - 2
-            if ancestors_len > 2 {
+            if ancestors_len > 2
-                elided_path.push(ancestors.nth(ancestors_len - 2).unwrap());
+                && let Some(ancestor) = ancestors.nth(ancestors_len - 2)
+            {
+                elided_path.push(ancestor);
             }
             // If ancestors_len is bigger than 3, push '…' and parent too
             if ancestors_len > 3 {

src/utils/parser.rs

@@ -111,6 +111,17 @@ static SEMVER_REGEX: Lazy<Regex> = lazy_regex!(r"v?((0|[1-9]\d*)\.(0|[1-9]\d*)\.
  */
 static BYTESIZE_REGEX: Lazy<Regex> = lazy_regex!(r"(:?([0-9])+)( )*(:?[KMGTP])?B$");
 
+fn capture_group_to_string(
+    groups: &regex::Captures<'_>,
+    index: usize,
+    field_name: &str,
+) -> Result<String, String> {
+    groups
+        .get(index)
+        .map(|group| group.as_str().to_string())
+        .ok_or_else(|| format!("Missing {field_name}"))
+}
+
 // -- remote opts
 
 /// Parse remote option string. Returns in case of success a RemoteOptions struct
@@ -265,9 +276,9 @@ fn parse_generic_remote_opt(
 fn parse_webdav_remote_opt(s: &str, prefix: &str) -> Result<FileTransferParams, String> {
     match REMOTE_WEBDAV_OPT_REGEX.captures(s) {
         Some(groups) => {
-            let username = groups.get(1).map(|x| x.as_str().to_string()).unwrap();
+            let username = capture_group_to_string(&groups, 1, "username")?;
-            let password = groups.get(2).map(|x| x.as_str().to_string()).unwrap();
+            let password = capture_group_to_string(&groups, 2, "password")?;
-            let uri = groups.get(3).map(|x| x.as_str().to_string()).unwrap();
+            let uri = capture_group_to_string(&groups, 3, "server URI")?;
             let remote_path: Option<PathBuf> =
                 groups.get(4).map(|group| PathBuf::from(group.as_str()));
 
@@ -474,7 +485,7 @@ pub fn parse_bytesize<S: AsRef<str>>(bytes: S) -> Option<ByteSize> {
                 .map(|x| x.as_str().parse::<u64>().unwrap_or(0))?;
             let unit = groups.get(4).map(|x| x.as_str().to_string());
             let unit = format!("{}B", unit.unwrap_or_default());
-            let unit = ByteUnit::from_str(unit.as_str()).unwrap();
+            let unit = ByteUnit::from_str(unit.as_str()).ok()?;
             Some(match unit {
                 ByteUnit::Byte => ByteSize::b(amount),
                 ByteUnit::Gigabyte => ByteSize::gib(amount),
@@ -681,6 +692,13 @@ mod tests {
         assert_eq!(params.password.as_str(), "password");
     }
 
+    #[test]
+    fn should_reject_malformed_webdav_options() {
+        let result = parse_remote_opt("https://omar@myserver:4445/myshare");
+
+        assert!(result.is_err());
+    }
+
     #[test]
     fn parse_aws_s3_opt() {
         // Simple
@@ -873,6 +891,7 @@ mod tests {
         assert_eq!(parse_bytesize("2 GB").unwrap().as_u64(), 2147483648);
         assert_eq!(parse_bytesize("1 TB").unwrap().as_u64(), 1099511627776);
         assert!(parse_bytesize("1 XB").is_none());
+        assert!(parse_bytesize("10 YB").is_none());
         assert!(parse_bytesize("1 GB aaaaa").is_none());
         assert!(parse_bytesize("1 GBaaaaa").is_none());
         assert!(parse_bytesize("1MBaaaaa").is_none());