diff --git a/src/filetransfer/params.rs b/src/filetransfer/params.rs index 7f96018..a397b58 100644 --- a/src/filetransfer/params.rs +++ b/src/filetransfer/params.rs @@ -25,10 +25,10 @@ pub enum HostBridgeParams { } impl HostBridgeParams { - pub fn unwrap_protocol_params(&self) -> &ProtocolParams { + pub fn protocol_params(&self) -> Option<&ProtocolParams> { match self { - HostBridgeParams::Localhost(_) => panic!("Localhost has no protocol params"), - HostBridgeParams::Remote(_, params) => params, + HostBridgeParams::Localhost(_) => None, + HostBridgeParams::Remote(_, params) => Some(params), } } @@ -278,6 +278,19 @@ mod test { use super::*; + #[test] + fn test_protocol_params_on_localhost_returns_none() { + let params = HostBridgeParams::Localhost(PathBuf::from("/tmp")); + assert!(params.protocol_params().is_none()); + } + + #[test] + fn test_protocol_params_on_remote_returns_some() { + let params = + HostBridgeParams::Remote(FileTransferProtocol::Sftp, ProtocolParams::default()); + assert!(params.protocol_params().is_some()); + } + #[test] fn test_filetransfer_params() { let params: FileTransferParams = diff --git a/src/filetransfer/remotefs_builder.rs b/src/filetransfer/remotefs_builder.rs index d5ff51a..54cfa88 100644 --- a/src/filetransfer/remotefs_builder.rs +++ b/src/filetransfer/remotefs_builder.rs @@ -44,13 +44,13 @@ impl RemoteFsBuilder { ) -> Result, String> { match (protocol, params) { (FileTransferProtocol::AwsS3, ProtocolParams::AwsS3(params)) => { - Ok(Box::new(Self::aws_s3_client(params))) + Ok(Box::new(Self::aws_s3_client(params)?)) } (FileTransferProtocol::Ftp(secure), ProtocolParams::Generic(params)) => { Ok(Box::new(Self::ftp_client(params, secure))) } (FileTransferProtocol::Kube, ProtocolParams::Kube(params)) => { - Ok(Box::new(Self::kube_client(params))) + Ok(Box::new(Self::kube_client(params)?)) } (FileTransferProtocol::Scp, ProtocolParams::Generic(params)) => { Ok(Box::new(Self::scp_client(params, config_client))) @@ -60,7 +60,7 @@ impl RemoteFsBuilder { } #[cfg(smb)] (FileTransferProtocol::Smb, ProtocolParams::Smb(params)) => { - Ok(Box::new(Self::smb_client(params))) + Ok(Box::new(Self::smb_client(params)?)) } (FileTransferProtocol::WebDAV, ProtocolParams::WebDAV(params)) => { Ok(Box::new(Self::webdav_client(params))) @@ -75,13 +75,13 @@ impl RemoteFsBuilder { } /// Build aws s3 client from parameters - fn aws_s3_client(params: AwsS3Params) -> AwsS3Fs { + fn aws_s3_client(params: AwsS3Params) -> Result { let rt = Arc::new( tokio::runtime::Builder::new_current_thread() .worker_threads(1) .enable_all() .build() - .expect("Unable to create tokio runtime"), + .map_err(|e| format!("Unable to create tokio runtime: {e}"))?, ); let mut client = AwsS3Fs::new(params.bucket_name, &rt).new_path_style(params.new_path_style); @@ -106,7 +106,7 @@ impl RemoteFsBuilder { if let Some(session_token) = params.session_token { client = client.session_token(session_token); } - client + Ok(client) } /// Build ftp client from parameters @@ -125,19 +125,19 @@ impl RemoteFsBuilder { } /// Build kube client - fn kube_client(params: KubeProtocolParams) -> KubeFs { + fn kube_client(params: KubeProtocolParams) -> Result { let rt = Arc::new( tokio::runtime::Builder::new_current_thread() .worker_threads(1) .enable_all() .build() - .expect("Unable to create tokio runtime"), + .map_err(|e| format!("Unable to create tokio runtime: {e}"))?, ); let kube_fs = KubeFs::new(&rt); if let Some(config) = params.config() { - kube_fs.config(config) + Ok(kube_fs.config(config)) } else { - kube_fs + Ok(kube_fs) } } @@ -158,7 +158,7 @@ impl RemoteFsBuilder { } #[cfg(smb_unix)] - fn smb_client(params: SmbParams) -> SmbFs { + fn smb_client(params: SmbParams) -> Result { let mut credentials = SmbCredentials::default() .server(format!("smb://{}:{}", params.address, params.port)) .share(params.share); @@ -179,14 +179,14 @@ impl RemoteFsBuilder { .one_share_per_server(true) .case_sensitive(false), ) - .unwrap_or_else(|e| { + .map_err(|e| { error!("Invalid params for protocol SMB: {e}"); - panic!("Invalid params for protocol SMB: {e}") + format!("Invalid params for protocol SMB: {e}") }) } #[cfg(smb_windows)] - fn smb_client(params: SmbParams) -> SmbFs { + fn smb_client(params: SmbParams) -> Result { let mut credentials = SmbCredentials::new(params.address, params.share); if let Some(username) = params.username { @@ -196,7 +196,7 @@ impl RemoteFsBuilder { credentials = credentials.password(password); } - SmbFs::new(credentials) + Ok(SmbFs::new(credentials)) } fn webdav_client(params: WebDAVProtocolParams) -> WebDAVFs { diff --git a/src/host/localhost.rs b/src/host/localhost.rs index ce5ba2a..a6c38fa 100644 --- a/src/host/localhost.rs +++ b/src/host/localhost.rs @@ -395,7 +395,17 @@ impl HostBridge for Localhost { fn exec(&mut self, cmd: &str) -> HostResult { // Make command let args: Vec<&str> = cmd.split(' ').collect(); - let cmd: &str = args.first().unwrap(); + let cmd: &str = match args.first() { + Some(cmd) => cmd, + None => { + error!("Empty command provided to exec"); + return Err(HostError::new( + HostErrorType::ExecutionFailed, + None, + self.wrkdir.as_path(), + )); + } + }; let argv: &[&str] = &args[1..]; info!("Executing command: {} {:?}", cmd, argv); match std::process::Command::new(cmd).args(argv).output() { @@ -993,6 +1003,14 @@ mod tests { assert!(host.exec("echo 5").ok().unwrap().as_str().contains("5")); } + #[test] + fn test_host_exec_empty_command() { + let tmpdir: tempfile::TempDir = tempfile::TempDir::new().unwrap(); + let mut host: Localhost = Localhost::new(PathBuf::from(tmpdir.path())).ok().unwrap(); + // Execute empty command should return error + assert!(host.exec("").is_err()); + } + #[cfg(posix)] #[test] fn should_create_symlink() { diff --git a/src/system/bookmarks_client.rs b/src/system/bookmarks_client.rs index 9fb290f..57d4712 100644 --- a/src/system/bookmarks_client.rs +++ b/src/system/bookmarks_client.rs @@ -190,8 +190,8 @@ impl BookmarksClient { ) { let name: String = name.as_ref().to_string(); if name.is_empty() { - error!("Fatal error; bookmark name is empty"); - panic!("Bookmark name can't be empty"); + error!("Bookmark name is empty; ignoring add_bookmark request"); + return; } // Make bookmark info!("Added bookmark {}", name); @@ -557,15 +557,13 @@ mod tests { } #[test] - #[should_panic] - fn test_system_bookmarks_bad_bookmark_name() { let tmp_dir: tempfile::TempDir = TempDir::new().ok().unwrap(); let (cfg_path, key_path): (PathBuf, PathBuf) = get_paths(tmp_dir.path()); // Initialize a new bookmarks client let mut client: BookmarksClient = BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap(); - // Add bookmark + // Add bookmark with empty name should be silently ignored client.add_bookmark( "", make_generic_ftparams( @@ -577,6 +575,8 @@ mod tests { ), true, ); + // No bookmark should have been added + assert_eq!(client.iter_bookmarks().count(), 0); } #[test] @@ -738,14 +738,13 @@ mod tests { } #[test] - #[should_panic] fn test_system_bookmarks_add_bookmark_empty() { let tmp_dir: tempfile::TempDir = TempDir::new().ok().unwrap(); let (cfg_path, key_path): (PathBuf, PathBuf) = get_paths(tmp_dir.path()); // Initialize a new bookmarks client let mut client: BookmarksClient = BookmarksClient::new(cfg_path.as_path(), key_path.as_path(), 16, true).unwrap(); - // Add bookmark + // Add bookmark with empty name should be silently ignored client.add_bookmark( "", make_generic_ftparams( @@ -757,6 +756,8 @@ mod tests { ), true, ); + // No bookmark should have been added + assert_eq!(client.iter_bookmarks().count(), 0); } #[test] diff --git a/src/system/config_client.rs b/src/system/config_client.rs index 90fe0aa..3b88ce4 100644 --- a/src/system/config_client.rs +++ b/src/system/config_client.rs @@ -309,7 +309,7 @@ impl ConfigClient { None => None, Some(key_path) => { // Get host and username - let (host, username): (String, String) = Self::get_ssh_tokens(mkey); + let (host, username) = Self::get_ssh_tokens(mkey)?; // Return key Some((host, username, PathBuf::from(key_path))) } @@ -389,12 +389,18 @@ impl ConfigClient { } /// Get ssh tokens starting from ssh host key - /// Panics if key has invalid syntax - /// Returns: (host, username) - fn get_ssh_tokens(host_key: &str) -> (String, String) { + /// Returns: (host, username) or None if key has invalid syntax + fn get_ssh_tokens(host_key: &str) -> Option<(String, String)> { let tokens: Vec<&str> = host_key.split('@').collect(); - assert!(tokens.len() >= 2); - (String::from(tokens[1]), String::from(tokens[0])) + if tokens.len() >= 2 { + Some((String::from(tokens[1]), String::from(tokens[0]))) + } else { + error!( + "Invalid SSH host key format: '{}' (expected 'username@host')", + host_key + ); + None + } } /// Make serializer error from `std::io::Error` @@ -711,10 +717,16 @@ mod tests { ); assert_eq!( ConfigClient::get_ssh_tokens("pi@192.168.1.31"), - (String::from("192.168.1.31"), String::from("pi")) + Some((String::from("192.168.1.31"), String::from("pi"))) ); } + #[test] + fn test_system_config_get_ssh_tokens_invalid() { + assert!(ConfigClient::get_ssh_tokens("invalid").is_none()); + assert!(ConfigClient::get_ssh_tokens("").is_none()); + } + #[test] fn test_system_config_make_io_err() { let err: SerializerError = diff --git a/src/system/keys/filestorage.rs b/src/system/keys/filestorage.rs index cf99445..4d758d1 100644 --- a/src/system/keys/filestorage.rs +++ b/src/system/keys/filestorage.rs @@ -71,7 +71,10 @@ impl KeyStorage for FileStorage { return Err(KeyStorageError::ProviderError); } // Set file to readonly - let mut permissions: Permissions = file.metadata().unwrap().permissions(); + let mut permissions: Permissions = file + .metadata() + .map_err(|_| KeyStorageError::ProviderError)? + .permissions(); permissions.set_readonly(true); let _ = file.set_permissions(permissions); Ok(()) diff --git a/src/ui/activities/filetransfer/mod.rs b/src/ui/activities/filetransfer/mod.rs index 463ce1e..90f0c31 100644 --- a/src/ui/activities/filetransfer/mod.rs +++ b/src/ui/activities/filetransfer/mod.rs @@ -478,9 +478,10 @@ impl Activity for FileTransferActivity { && !self.browser.local_pane().fs.is_localhost() { let host_bridge_params = self.context().host_bridge_params().unwrap(); - let ft_params = host_bridge_params.unwrap_protocol_params(); - // print params - let msg: String = Self::get_connection_msg(ft_params); + let msg: String = match host_bridge_params.protocol_params() { + Some(ft_params) => Self::get_connection_msg(ft_params), + None => String::from("Connecting..."), + }; // Set init state to connecting popup self.mount_blocking_wait(msg.as_str()); // Connect to remote