From ba4f786f2f2e29dcb42733c8e1538150acee588b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 05:35:25 +0000 Subject: [PATCH 01/12] Initial plan From e120cf4d85e4ffbf6cd5a282927a94e2dabfc459 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 05:46:36 +0000 Subject: [PATCH 02/12] fix: update russh to 0.58 to fix build issue Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index f83b8e7..17a0b26 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ pyo3 = { version = "0.26.0", features = ["generate-import-lib"] } shellexpand = "3.1.0" pyo3-async-runtimes = { version = "0.26.0", features = ["tokio-runtime"] } tokio = { version = "1", features = ["full"] } -russh = { version = "0.56", default-features = false, features = ["flate2", "async-trait", "ring", "rsa"] } +russh = { version = "0.58", default-features = false, features = ["flate2", "async-trait", "ring", "rsa"] } russh-sftp = "2.1" async-trait = "0.1" From 0c4dff11378215945cf8320602724a00b5d4a7e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 05:52:25 +0000 Subject: [PATCH 03/12] feat: add sftp_put_dir and sftp_get_dir for recursive directory transfers Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- README.md | 6 + docs/asynchronous.md | 26 +++ docs/synchronous.md | 29 +++ src/asynchronous.rs | 313 ++++++++++++++++++++++++++++++++- src/connection.rs | 282 +++++++++++++++++++++++++++++ tests/test_async_connection.py | 63 +++++++ tests/test_connection.py | 53 ++++++ 7 files changed, 770 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ed8804b..77cc656 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,12 @@ conn.sftp_write(local_path="/path/to/my/file", remote_path="/dest/path/file") # Read a remote file contents = conn.sftp_read(remote_path="/dest/path/file") + +# Upload an entire local directory recursively +files_copied, bytes_transferred = conn.sftp_put_dir("/local/build/", "/remote/app/") + +# Download an entire remote directory recursively +files_copied, bytes_transferred = conn.sftp_get_dir("/remote/logs/", "/local/logs/") ``` 📚 **For complete documentation including SCP, file tailing, interactive shells, and more, see [Synchronous Usage](docs/synchronous.md).** diff --git a/docs/asynchronous.md b/docs/asynchronous.md index 03921fd..d8cd457 100644 --- a/docs/asynchronous.md +++ b/docs/asynchronous.md @@ -207,6 +207,32 @@ async with AsyncConnection(host="my.test.server", password="pass") as conn: print(file) ``` +### Directory Transfers + +Upload or download entire directory trees with a single awaitable call. Both methods return a tuple of `(files_copied, bytes_transferred)`. + +```python +async with AsyncConnection(host="my.test.server", password="pass") as conn: + # Upload an entire local directory to the remote server + files_copied, bytes_transferred = await conn.sftp_put_dir( + local_path="/local/build/", + remote_path="/remote/app/", + ) + + # Download an entire remote directory to a local destination + files_copied, bytes_transferred = await conn.sftp_get_dir( + remote_path="/remote/logs/", + local_path="/local/logs/", + ) +``` + +Optional keyword arguments control symlink and permission behaviour: + +| Parameter | Default | Description | +|-----------|---------|-------------| +| `follow_symlinks` | `True` | Follow symlinks; set `False` to skip them | +| `preserve_permissions` | `True` | Mirror source permissions on the destination | + ### Concurrent File Operations ```python diff --git a/docs/synchronous.md b/docs/synchronous.md index 1e05b44..8b0d11a 100644 --- a/docs/synchronous.md +++ b/docs/synchronous.md @@ -123,6 +123,35 @@ contents = conn.sftp_read(remote_path="/dest/path/file") print(contents) ``` +### Directory Transfers + +Upload or download entire directory trees with a single call. Both methods return a tuple of `(files_copied, bytes_transferred)`. + +```python +# Upload an entire local directory to the remote server +files_copied, bytes_transferred = conn.sftp_put_dir( + local_path="/local/build/", + remote_path="/remote/app/", +) + +# Download an entire remote directory to a local destination +files_copied, bytes_transferred = conn.sftp_get_dir( + remote_path="/remote/logs/", + local_path="/local/logs/", +) +``` + +Optional keyword arguments control symlink and permission behaviour: + +| Parameter | Default | Description | +|-----------|---------|-------------| +| `follow_symlinks` | `True` | Follow symlinks; set `False` to skip them | +| `preserve_permissions` | `True` | Mirror source permissions on the destination | + +```python +conn.sftp_put_dir("/src/", "/dst/", follow_symlinks=False, preserve_permissions=False) +``` + ### Copy Files Between Connections Hussh provides a convenient way to copy files between two remote servers: diff --git a/src/asynchronous.rs b/src/asynchronous.rs index 808a97e..6a5d194 100644 --- a/src/asynchronous.rs +++ b/src/asynchronous.rs @@ -79,6 +79,12 @@ //! //! # List directory contents //! files = await conn.sftp_list("/remote/path") +//! +//! # Upload an entire local directory recursively +//! files_copied, bytes_transferred = await conn.sftp_put_dir("/local/build/", "/remote/app/") +//! +//! # Download an entire remote directory recursively +//! files_copied, bytes_transferred = await conn.sftp_get_dir("/remote/logs/", "/local/logs/") //! ``` //! //! For file tailing: @@ -222,6 +228,24 @@ impl Handler for ClientHandler { /// /// * `path`: The path to the remote directory to list. /// +/// ### `sftp_put_dir` +/// +/// Uploads a local directory recursively to a remote path over SFTP. Returns (files_copied, bytes_transferred). It takes the following parameters: +/// +/// * `local_path`: The local directory to upload. +/// * `remote_path`: The remote destination path. +/// * `follow_symlinks`: Whether to follow symlinks (default: true). +/// * `preserve_permissions`: Whether to preserve file permissions (default: true). +/// +/// ### `sftp_get_dir` +/// +/// Downloads a remote directory recursively to a local path over SFTP. Returns (files_copied, bytes_transferred). It takes the following parameters: +/// +/// * `remote_path`: The remote directory to download. +/// * `local_path`: The local destination path. +/// * `follow_symlinks`: Whether to follow symlinks (default: true). +/// * `preserve_permissions`: Whether to preserve file permissions (default: true). +/// /// ### `shell` /// /// Creates an `AsyncInteractiveShell` instance. It takes the following parameter: @@ -574,7 +598,256 @@ impl AsyncConnection { Ok(()) } - /// Create a new AsyncConnection - used by MultiConnection + /// Internal async SFTP put_dir implementation + pub(crate) async fn sftp_put_dir_async( + &self, + local_path: String, + remote_path: String, + follow_symlinks: bool, + preserve_permissions: bool, + ) -> PyResult<(u64, u64)> { + let sftp = + AsyncConnection::get_or_init_sftp(self.session.clone(), self.sftp_session.clone()) + .await?; + + let mut files_copied = 0u64; + let mut bytes_transferred = 0u64; + + // Ensure remote base directory exists + if !sftp.try_exists(&remote_path).await.unwrap_or(false) { + sftp.create_dir(&remote_path).await.map_err(|e| { + PyRuntimeError::new_err(format!( + "Failed to create remote directory '{}': {}", + remote_path, e + )) + })?; + } + + // Stack for depth-first traversal: (local_dir, remote_dir) + let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( + std::path::PathBuf::from(&local_path), + std::path::PathBuf::from(&remote_path), + )]; + + while let Some((local_dir, remote_dir)) = dirs_to_process.pop() { + let mut read_dir = tokio::fs::read_dir(&local_dir).await.map_err(|e| { + PyRuntimeError::new_err(format!( + "Failed to read local directory '{}': {}", + local_dir.display(), + e + )) + })?; + + while let Some(entry) = read_dir.next_entry().await.map_err(|e| { + PyRuntimeError::new_err(format!("Directory entry error: {}", e)) + })? { + let local_entry = entry.path(); + let remote_entry = remote_dir.join(entry.file_name()); + let remote_entry_str = remote_entry.to_string_lossy().to_string(); + + let metadata = if follow_symlinks { + tokio::fs::metadata(&local_entry).await + } else { + tokio::fs::symlink_metadata(&local_entry).await + } + .map_err(|e| { + PyRuntimeError::new_err(format!( + "Metadata error for '{}': {}", + local_entry.display(), + e + )) + })?; + + if metadata.is_dir() { + if !sftp.try_exists(&remote_entry_str).await.unwrap_or(false) { + sftp.create_dir(&remote_entry_str).await.map_err(|e| { + PyRuntimeError::new_err(format!( + "Failed to create remote directory '{}': {}", + remote_entry_str, e + )) + })?; + } + #[cfg(unix)] + if preserve_permissions { + use std::os::unix::fs::PermissionsExt; + let mode = metadata.permissions().mode(); + let mut attrs = russh_sftp::client::fs::Metadata::default(); + attrs.permissions = Some(mode); + let _ = sftp.set_metadata(&remote_entry_str, attrs).await; + } + dirs_to_process.push((local_entry, remote_entry)); + } else if metadata.is_file() { + let mut local_file = + tokio::fs::File::open(&local_entry).await.map_err(|e| { + PyRuntimeError::new_err(format!("File open error: {}", e)) + })?; + let mut remote_file = sftp.create(&remote_entry_str).await.map_err(|e| { + PyRuntimeError::new_err(format!( + "Remote file creation error '{}': {}", + remote_entry_str, e + )) + })?; + let mut buffer = vec![0u8; 65536]; + loop { + let n = local_file.read(&mut buffer).await.map_err(|e| { + PyRuntimeError::new_err(format!("File read error: {}", e)) + })?; + if n == 0 { + break; + } + remote_file.write_all(&buffer[..n]).await.map_err(|e| { + PyRuntimeError::new_err(format!("Remote write error: {}", e)) + })?; + bytes_transferred += n as u64; + } + #[cfg(unix)] + if preserve_permissions { + use std::os::unix::fs::PermissionsExt; + let mode = metadata.permissions().mode(); + let mut attrs = russh_sftp::client::fs::Metadata::default(); + attrs.permissions = Some(mode); + let _ = sftp.set_metadata(&remote_entry_str, attrs).await; + } + files_copied += 1; + } + // Symlinks with follow_symlinks=false are skipped + } + } + + Ok((files_copied, bytes_transferred)) + } + + /// Internal async SFTP get_dir implementation + pub(crate) async fn sftp_get_dir_async( + &self, + remote_path: String, + local_path: String, + follow_symlinks: bool, + preserve_permissions: bool, + ) -> PyResult<(u64, u64)> { + let sftp = + AsyncConnection::get_or_init_sftp(self.session.clone(), self.sftp_session.clone()) + .await?; + + let mut files_copied = 0u64; + let mut bytes_transferred = 0u64; + + // Ensure local base directory exists + tokio::fs::create_dir_all(&local_path).await.map_err(|e| { + PyRuntimeError::new_err(format!( + "Failed to create local directory '{}': {}", + local_path, e + )) + })?; + + // Stack for depth-first traversal: (remote_dir, local_dir) + let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( + std::path::PathBuf::from(&remote_path), + std::path::PathBuf::from(&local_path), + )]; + + while let Some((remote_dir, local_dir)) = dirs_to_process.pop() { + let remote_dir_str = remote_dir.to_string_lossy().to_string(); + let read_dir = sftp.read_dir(&remote_dir_str).await.map_err(|e| { + PyRuntimeError::new_err(format!( + "Failed to read remote directory '{}': {}", + remote_dir_str, e + )) + })?; + + for entry in read_dir { + let file_name = entry.file_name(); + let local_entry = local_dir.join(&file_name); + let remote_entry = remote_dir.join(&file_name); + let remote_entry_str = remote_entry.to_string_lossy().to_string(); + + // When follow_symlinks=true, resolve symlinks on remote via metadata() + let file_type = if follow_symlinks && entry.file_type().is_symlink() { + sftp.metadata(&remote_entry_str) + .await + .map(|m| m.file_type()) + .unwrap_or_else(|_| entry.file_type()) + } else { + entry.file_type() + }; + + if file_type.is_symlink() { + // follow_symlinks=false: skip symlinks + continue; + } else if file_type.is_dir() { + tokio::fs::create_dir_all(&local_entry).await.map_err(|e| { + PyRuntimeError::new_err(format!( + "Failed to create local directory '{}': {}", + local_entry.display(), + e + )) + })?; + #[cfg(unix)] + if preserve_permissions { + if let Some(perm) = sftp + .metadata(&remote_entry_str) + .await + .ok() + .and_then(|m| m.permissions) + { + use std::os::unix::fs::PermissionsExt; + let _ = tokio::fs::set_permissions( + &local_entry, + std::fs::Permissions::from_mode(perm & 0o7777), + ) + .await; + } + } + dirs_to_process.push((remote_entry, local_entry)); + } else if file_type.is_file() { + let mut remote_file = sftp.open(&remote_entry_str).await.map_err(|e| { + PyRuntimeError::new_err(format!("SFTP open error: {}", e)) + })?; + let mut local_file = + tokio::fs::File::create(&local_entry).await.map_err(|e| { + PyRuntimeError::new_err(format!("File create error: {}", e)) + })?; + let mut buffer = vec![0u8; 65536]; + loop { + let n = remote_file.read(&mut buffer).await.map_err(|e| { + PyRuntimeError::new_err(format!("File read error: {}", e)) + })?; + if n == 0 { + break; + } + local_file.write_all(&buffer[..n]).await.map_err(|e| { + PyRuntimeError::new_err(format!("File write error: {}", e)) + })?; + bytes_transferred += n as u64; + } + local_file.flush().await.map_err(|e| { + PyRuntimeError::new_err(format!("Flush error: {}", e)) + })?; + #[cfg(unix)] + if preserve_permissions { + if let Some(perm) = sftp + .metadata(&remote_entry_str) + .await + .ok() + .and_then(|m| m.permissions) + { + use std::os::unix::fs::PermissionsExt; + let _ = tokio::fs::set_permissions( + &local_entry, + std::fs::Permissions::from_mode(perm & 0o7777), + ) + .await; + } + } + files_copied += 1; + } + } + } + + Ok((files_copied, bytes_transferred)) + } + + pub(crate) fn create( host: String, username: Option, @@ -755,7 +1028,43 @@ impl AsyncConnection { }) } - #[pyo3(signature = (pty=None))] + /// Uploads a local directory recursively to a remote path over SFTP. + /// Returns a tuple of (files_copied, bytes_transferred). + #[pyo3(signature = (local_path, remote_path, follow_symlinks=true, preserve_permissions=true))] + fn sftp_put_dir<'p>( + &self, + py: Python<'p>, + local_path: String, + remote_path: String, + follow_symlinks: bool, + preserve_permissions: bool, + ) -> PyResult> { + let conn = self.clone(); + pyo3_async_runtimes::tokio::future_into_py(py, async move { + conn.sftp_put_dir_async(local_path, remote_path, follow_symlinks, preserve_permissions) + .await + }) + } + + /// Downloads a remote directory recursively to a local path over SFTP. + /// Returns a tuple of (files_copied, bytes_transferred). + #[pyo3(signature = (remote_path, local_path, follow_symlinks=true, preserve_permissions=true))] + fn sftp_get_dir<'p>( + &self, + py: Python<'p>, + remote_path: String, + local_path: String, + follow_symlinks: bool, + preserve_permissions: bool, + ) -> PyResult> { + let conn = self.clone(); + pyo3_async_runtimes::tokio::future_into_py(py, async move { + conn.sftp_get_dir_async(remote_path, local_path, follow_symlinks, preserve_permissions) + .await + }) + } + + fn shell<'p>(&self, py: Python<'p>, pty: Option) -> PyResult> { let session_arc = self.session.clone(); let pty = pty.unwrap_or(false); diff --git a/src/connection.rs b/src/connection.rs index e7f2bd5..af0a779 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -184,6 +184,24 @@ impl SSHResult { /// * `local_path`: The path to the file on the local system. /// * `remote_path`: The path to save the file on the remote system. /// +/// ### `sftp_put_dir` +/// +/// Uploads a local directory recursively to a remote path over SFTP. Returns (files_copied, bytes_transferred). It takes the following parameters: +/// +/// * `local_path`: The local directory to upload. +/// * `remote_path`: The remote destination path. +/// * `follow_symlinks`: Whether to follow symlinks (default: true). +/// * `preserve_permissions`: Whether to preserve file permissions (default: true). +/// +/// ### `sftp_get_dir` +/// +/// Downloads a remote directory recursively to a local path over SFTP. Returns (files_copied, bytes_transferred). It takes the following parameters: +/// +/// * `remote_path`: The remote directory to download. +/// * `local_path`: The local destination path. +/// * `follow_symlinks`: Whether to follow symlinks (default: true). +/// * `preserve_permissions`: Whether to preserve file permissions (default: true). +/// /// ### `shell` /// /// Creates an `InteractiveShell` instance. It takes the following parameter: @@ -561,6 +579,270 @@ impl Connection { Ok(()) } + /// Uploads a local directory recursively to a remote path over SFTP. + /// Returns a tuple of (files_copied, bytes_transferred). + #[pyo3(signature = (local_path, remote_path, follow_symlinks=true, preserve_permissions=true))] + fn sftp_put_dir( + &mut self, + local_path: String, + remote_path: String, + follow_symlinks: bool, + preserve_permissions: bool, + ) -> PyResult<(u64, u64)> { + let mut files_copied = 0u64; + let mut bytes_transferred = 0u64; + + // Ensure remote base directory exists + if self.sftp().stat(Path::new(&remote_path)).is_err() { + self.sftp() + .mkdir(Path::new(&remote_path), 0o755) + .map_err(|e| { + PyErr::new::(format!( + "Failed to create remote directory '{}': {}", + remote_path, e + )) + })?; + } + + // Stack for depth-first traversal: (local_dir, remote_dir) + let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( + std::path::PathBuf::from(&local_path), + std::path::PathBuf::from(&remote_path), + )]; + + while let Some((local_dir, remote_dir)) = dirs_to_process.pop() { + let entries = std::fs::read_dir(&local_dir).map_err(|e| { + PyErr::new::(format!( + "Failed to read local directory '{}': {}", + local_dir.display(), + e + )) + })?; + + for entry in entries { + let entry = entry.map_err(|e| { + PyErr::new::(format!("Directory entry error: {}", e)) + })?; + + let local_entry = entry.path(); + let remote_entry = remote_dir.join(entry.file_name()); + let remote_entry_str = remote_entry.to_string_lossy().to_string(); + + let metadata = if follow_symlinks { + std::fs::metadata(&local_entry) + } else { + std::fs::symlink_metadata(&local_entry) + } + .map_err(|e| { + PyErr::new::(format!( + "Metadata error for '{}': {}", + local_entry.display(), + e + )) + })?; + + if metadata.is_dir() { + #[cfg(unix)] + let mode = if preserve_permissions { + use std::os::unix::fs::PermissionsExt; + metadata.permissions().mode() as i32 + } else { + 0o755i32 + }; + #[cfg(not(unix))] + let mode = 0o755i32; + if self.sftp().stat(Path::new(&remote_entry_str)).is_err() { + self.sftp() + .mkdir(Path::new(&remote_entry_str), mode) + .map_err(|e| { + PyErr::new::(format!( + "Failed to create remote directory '{}': {}", + remote_entry_str, e + )) + })?; + } + dirs_to_process.push((local_entry, remote_entry)); + } else if metadata.is_file() { + let mut local_file = std::fs::File::open(&local_entry).map_err(|e| { + PyErr::new::(format!("File open error: {}", e)) + })?; + let file_size = metadata.len(); + let mut remote_file = self + .sftp() + .create(Path::new(&remote_entry_str)) + .map_err(|e| { + PyErr::new::(format!( + "Remote file creation error '{}': {}", + remote_entry_str, e + )) + })?; + let buf_size = (file_size as usize).min(MAX_BUFF_SIZE).max(1); + let mut buffer = vec![0u8; buf_size]; + loop { + let n = local_file.read(&mut buffer).map_err(|e| { + PyErr::new::(format!("File read error: {}", e)) + })?; + if n == 0 { + break; + } + remote_file.write_all(&buffer[..n]).map_err(|e| { + PyErr::new::(format!("Remote write error: {}", e)) + })?; + bytes_transferred += n as u64; + } + remote_file.close().map_err(|e| { + PyErr::new::(format!("Close error: {}", e)) + })?; + #[cfg(unix)] + if preserve_permissions { + use std::os::unix::fs::PermissionsExt; + let mode = metadata.permissions().mode(); + let _ = self.sftp().setstat( + Path::new(&remote_entry_str), + ssh2::FileStat { + perm: Some(mode), + size: None, + uid: None, + gid: None, + atime: None, + mtime: None, + }, + ); + } + files_copied += 1; + } + // Symlinks with follow_symlinks=false are skipped + } + } + + Ok((files_copied, bytes_transferred)) + } + + /// Downloads a remote directory recursively to a local path over SFTP. + /// Returns a tuple of (files_copied, bytes_transferred). + #[pyo3(signature = (remote_path, local_path, follow_symlinks=true, preserve_permissions=true))] + fn sftp_get_dir( + &mut self, + remote_path: String, + local_path: String, + follow_symlinks: bool, + preserve_permissions: bool, + ) -> PyResult<(u64, u64)> { + let mut files_copied = 0u64; + let mut bytes_transferred = 0u64; + + // Ensure local base directory exists + std::fs::create_dir_all(&local_path).map_err(|e| { + PyErr::new::(format!( + "Failed to create local directory '{}': {}", + local_path, e + )) + })?; + + // Stack for depth-first traversal: (remote_dir, local_dir) + let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( + std::path::PathBuf::from(&remote_path), + std::path::PathBuf::from(&local_path), + )]; + + while let Some((remote_dir, local_dir)) = dirs_to_process.pop() { + let remote_dir_str = remote_dir.to_string_lossy().to_string(); + let entries = self + .sftp() + .readdir(Path::new(&remote_dir_str)) + .map_err(|e| { + PyErr::new::(format!( + "Failed to read remote directory '{}': {}", + remote_dir_str, e + )) + })?; + + for (entry_name, stat) in entries { + let file_name = entry_name + .file_name() + .ok_or_else(|| PyErr::new::("Invalid entry path"))? + .to_os_string(); + let local_entry = local_dir.join(&file_name); + let remote_entry = remote_dir.join(&entry_name); + let remote_entry_str = remote_entry.to_string_lossy().to_string(); + + // When follow_symlinks=true, resolve symlinks on remote via stat() + let resolved_stat = if follow_symlinks && stat.file_type().is_symlink() { + self.sftp() + .stat(Path::new(&remote_entry_str)) + .unwrap_or(stat) + } else { + stat + }; + + if resolved_stat.file_type().is_symlink() { + // follow_symlinks=false: skip symlinks + continue; + } else if resolved_stat.is_dir() { + std::fs::create_dir_all(&local_entry).map_err(|e| { + PyErr::new::(format!( + "Failed to create local directory '{}': {}", + local_entry.display(), + e + )) + })?; + #[cfg(unix)] + if preserve_permissions { + if let Some(perm) = resolved_stat.perm { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions( + &local_entry, + std::fs::Permissions::from_mode(perm & 0o7777), + ); + } + } + dirs_to_process.push((remote_entry, local_entry)); + } else if resolved_stat.is_file() { + let mut remote_file = BufReader::new( + self.sftp() + .open(Path::new(&remote_entry_str)) + .map_err(|e| { + PyErr::new::(format!("SFTP open error: {}", e)) + })?, + ); + let local_file = std::fs::File::create(&local_entry).map_err(|e| { + PyErr::new::(format!("File create error: {}", e)) + })?; + let mut writer = BufWriter::new(local_file); + let mut buffer = vec![0u8; MAX_BUFF_SIZE]; + loop { + let n = remote_file.read(&mut buffer).map_err(|e| { + PyErr::new::(format!("File read error: {}", e)) + })?; + if n == 0 { + break; + } + writer.write_all(&buffer[..n]).map_err(|e| { + PyErr::new::(format!("File write error: {}", e)) + })?; + bytes_transferred += n as u64; + } + writer.flush().map_err(|e| { + PyErr::new::(format!("Flush error: {}", e)) + })?; + #[cfg(unix)] + if preserve_permissions { + if let Some(perm) = resolved_stat.perm { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions( + &local_entry, + std::fs::Permissions::from_mode(perm & 0o7777), + ); + } + } + files_copied += 1; + } + } + } + + Ok((files_copied, bytes_transferred)) + } + // Copy a file from this connection to another connection #[pyo3(signature = (source_path, dest_conn, dest_path=None))] fn remote_copy( diff --git a/tests/test_async_connection.py b/tests/test_async_connection.py index 957b85c..2b70fb8 100644 --- a/tests/test_async_connection.py +++ b/tests/test_async_connection.py @@ -66,6 +66,69 @@ async def test_async_sftp_write_data(run_test_server): assert "hello.txt" in files +@pytest.mark.asyncio +async def test_async_sftp_put_dir(run_test_server, tmp_path): + """Test that we can recursively upload a directory tree over async SFTP.""" + async with AsyncConnection("localhost", username="root", password="toor", port=8022) as conn: + # Create a local directory tree + src = tmp_path / "async_src_dir" + src.mkdir() + (src / "file1.txt").write_text("file1 content") + (src / "file2.txt").write_text("file2 content") + sub = src / "subdir" + sub.mkdir() + (sub / "nested.txt").write_text("nested content") + + # Upload to remote + files_copied, bytes_transferred = await conn.sftp_put_dir( + str(src), "/root/async_test_put_dir" + ) + + # Verify files exist on remote + remote_ls = (await conn.execute("find /root/async_test_put_dir -type f | sort")).stdout + assert "file1.txt" in remote_ls + assert "file2.txt" in remote_ls + assert "nested.txt" in remote_ls + assert files_copied == 3 + assert bytes_transferred > 0 + + # Verify nested file contents + content = await conn.sftp_read("/root/async_test_put_dir/subdir/nested.txt") + assert content == "nested content" + + # Cleanup + await conn.execute("rm -rf /root/async_test_put_dir") + + +@pytest.mark.asyncio +async def test_async_sftp_get_dir(run_test_server, tmp_path): + """Test that we can recursively download a directory tree over async SFTP.""" + async with AsyncConnection("localhost", username="root", password="toor", port=8022) as conn: + # Set up a remote directory tree + await conn.execute("mkdir -p /root/async_test_get_dir/subdir") + await conn.sftp_write_data("remote file 1", "/root/async_test_get_dir/file1.txt") + await conn.sftp_write_data("remote file 2", "/root/async_test_get_dir/file2.txt") + await conn.sftp_write_data( + "nested remote", "/root/async_test_get_dir/subdir/nested.txt" + ) + + # Download to local + dest = tmp_path / "async_dest_dir" + files_copied, bytes_transferred = await conn.sftp_get_dir( + "/root/async_test_get_dir", str(dest) + ) + + # Verify local files + assert (dest / "file1.txt").read_text() == "remote file 1" + assert (dest / "file2.txt").read_text() == "remote file 2" + assert (dest / "subdir" / "nested.txt").read_text() == "nested remote" + assert files_copied == 3 + assert bytes_transferred > 0 + + # Cleanup + await conn.execute("rm -rf /root/async_test_get_dir") + + @pytest.mark.asyncio async def test_async_shell(run_test_server): async with ( diff --git a/tests/test_connection.py b/tests/test_connection.py index 0782603..d331f63 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -212,6 +212,59 @@ def test_sftp_write_data(conn): assert read_text == "hello" +def test_sftp_put_dir(conn, tmp_path): + """Test that we can recursively upload a directory tree over SFTP.""" + # Create a local directory tree + src = tmp_path / "src_dir" + src.mkdir() + (src / "file1.txt").write_text("file1 content") + (src / "file2.txt").write_text("file2 content") + sub = src / "subdir" + sub.mkdir() + (sub / "nested.txt").write_text("nested content") + + # Upload to remote + files_copied, bytes_transferred = conn.sftp_put_dir(str(src), "/root/test_put_dir") + + # Verify files exist on remote + remote_ls = conn.execute("find /root/test_put_dir -type f | sort").stdout + assert "file1.txt" in remote_ls + assert "file2.txt" in remote_ls + assert "nested.txt" in remote_ls + assert files_copied == 3 + assert bytes_transferred > 0 + + # Verify nested file contents + content = conn.sftp_read("/root/test_put_dir/subdir/nested.txt") + assert content == "nested content" + + # Cleanup + conn.execute("rm -rf /root/test_put_dir") + + +def test_sftp_get_dir(conn, tmp_path): + """Test that we can recursively download a directory tree over SFTP.""" + # Set up a remote directory tree + conn.execute("mkdir -p /root/test_get_dir/subdir") + conn.sftp_write_data("remote file 1", "/root/test_get_dir/file1.txt") + conn.sftp_write_data("remote file 2", "/root/test_get_dir/file2.txt") + conn.sftp_write_data("nested remote", "/root/test_get_dir/subdir/nested.txt") + + # Download to local + dest = tmp_path / "dest_dir" + files_copied, bytes_transferred = conn.sftp_get_dir("/root/test_get_dir", str(dest)) + + # Verify local files + assert (dest / "file1.txt").read_text() == "remote file 1" + assert (dest / "file2.txt").read_text() == "remote file 2" + assert (dest / "subdir" / "nested.txt").read_text() == "nested remote" + assert files_copied == 3 + assert bytes_transferred > 0 + + # Cleanup + conn.execute("rm -rf /root/test_get_dir") + + @pytest.mark.skip("non-text files are not supported by sftp") def test_non_utf8_sftp(conn): """Test that we can copy a non-text file to the server and read it back.""" From 17018ac1afeea94e27240e80a7df7d351f1c4249 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 05:57:43 +0000 Subject: [PATCH 04/12] refactor: address code review - use MAX_BUFF_SIZE constant and remove .max(1) Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- src/asynchronous.rs | 10 +++++----- src/connection.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/asynchronous.rs b/src/asynchronous.rs index 6a5d194..02d9480 100644 --- a/src/asynchronous.rs +++ b/src/asynchronous.rs @@ -98,7 +98,7 @@ //! print(tailer.contents) //! ``` -use crate::connection::SSHResult; +use crate::connection::{SSHResult, MAX_BUFF_SIZE}; use pyo3::exceptions::{PyRuntimeError, PyTimeoutError}; use pyo3::prelude::*; use russh::client::{Config, Handle, Handler}; @@ -516,7 +516,7 @@ impl AsyncConnection { PyRuntimeError::new_err(format!("Failed to create local file: {}", e)) })?; - let mut buffer = vec![0u8; 65536]; // 64KB buffer to match sync version + let mut buffer = vec![0u8; MAX_BUFF_SIZE]; loop { let n = remote_file.read(&mut buffer).await.map_err(|e| { PyRuntimeError::new_err(format!("Failed to read remote file: {}", e)) @@ -560,7 +560,7 @@ impl AsyncConnection { .await .map_err(|e| PyRuntimeError::new_err(format!("Failed to create remote file: {}", e)))?; - let mut buffer = vec![0u8; 65536]; // 64KB buffer to match sync version + let mut buffer = vec![0u8; MAX_BUFF_SIZE]; loop { let n = local_file.read(&mut buffer).await.map_err(|e| { PyRuntimeError::new_err(format!("Failed to read local file: {}", e)) @@ -687,7 +687,7 @@ impl AsyncConnection { remote_entry_str, e )) })?; - let mut buffer = vec![0u8; 65536]; + let mut buffer = vec![0u8; MAX_BUFF_SIZE]; loop { let n = local_file.read(&mut buffer).await.map_err(|e| { PyRuntimeError::new_err(format!("File read error: {}", e)) @@ -807,7 +807,7 @@ impl AsyncConnection { tokio::fs::File::create(&local_entry).await.map_err(|e| { PyRuntimeError::new_err(format!("File create error: {}", e)) })?; - let mut buffer = vec![0u8; 65536]; + let mut buffer = vec![0u8; MAX_BUFF_SIZE]; loop { let n = remote_file.read(&mut buffer).await.map_err(|e| { PyRuntimeError::new_err(format!("File read error: {}", e)) diff --git a/src/connection.rs b/src/connection.rs index af0a779..bfa70db 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -67,7 +67,7 @@ use std::path::Path; use pyo3::exceptions::{PyIOError, PyTimeoutError}; -const MAX_BUFF_SIZE: usize = 65536; +pub(crate) const MAX_BUFF_SIZE: usize = 65536; create_exception!( connection, AuthenticationError, @@ -676,7 +676,7 @@ impl Connection { remote_entry_str, e )) })?; - let buf_size = (file_size as usize).min(MAX_BUFF_SIZE).max(1); + let buf_size = (file_size as usize).min(MAX_BUFF_SIZE); let mut buffer = vec![0u8; buf_size]; loop { let n = local_file.read(&mut buffer).map_err(|e| { From e7ea15b8c8d5bc12b4586e5dfca1b4fd6be09dd7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 06:25:11 +0000 Subject: [PATCH 05/12] feat: change sftp_put_dir/sftp_get_dir return to (transferred, failed) lists and add fail_fast param Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- README.md | 4 +- docs/asynchronous.md | 11 +- docs/synchronous.md | 15 +- src/asynchronous.rs | 378 +++++++++++++++++++++------------ src/connection.rs | 373 +++++++++++++++++++------------- tests/test_async_connection.py | 27 ++- tests/test_connection.py | 38 +++- 7 files changed, 536 insertions(+), 310 deletions(-) diff --git a/README.md b/README.md index 77cc656..0d71e32 100644 --- a/README.md +++ b/README.md @@ -87,10 +87,10 @@ conn.sftp_write(local_path="/path/to/my/file", remote_path="/dest/path/file") contents = conn.sftp_read(remote_path="/dest/path/file") # Upload an entire local directory recursively -files_copied, bytes_transferred = conn.sftp_put_dir("/local/build/", "/remote/app/") +transferred, failed = conn.sftp_put_dir("/local/build/", "/remote/app/") # Download an entire remote directory recursively -files_copied, bytes_transferred = conn.sftp_get_dir("/remote/logs/", "/local/logs/") +transferred, failed = conn.sftp_get_dir("/remote/logs/", "/local/logs/") ``` 📚 **For complete documentation including SCP, file tailing, interactive shells, and more, see [Synchronous Usage](docs/synchronous.md).** diff --git a/docs/asynchronous.md b/docs/asynchronous.md index d8cd457..ea08518 100644 --- a/docs/asynchronous.md +++ b/docs/asynchronous.md @@ -209,29 +209,32 @@ async with AsyncConnection(host="my.test.server", password="pass") as conn: ### Directory Transfers -Upload or download entire directory trees with a single awaitable call. Both methods return a tuple of `(files_copied, bytes_transferred)`. +Upload or download entire directory trees with a single awaitable call. Both methods return a tuple of `(transferred_files, failed_files)`, where each is a list of file paths. ```python async with AsyncConnection(host="my.test.server", password="pass") as conn: # Upload an entire local directory to the remote server - files_copied, bytes_transferred = await conn.sftp_put_dir( + transferred, failed = await conn.sftp_put_dir( local_path="/local/build/", remote_path="/remote/app/", ) + if failed: + print(f"Failed to upload: {failed}") # Download an entire remote directory to a local destination - files_copied, bytes_transferred = await conn.sftp_get_dir( + transferred, failed = await conn.sftp_get_dir( remote_path="/remote/logs/", local_path="/local/logs/", ) ``` -Optional keyword arguments control symlink and permission behaviour: +Optional keyword arguments control symlink, permission, and error-handling behaviour: | Parameter | Default | Description | |-----------|---------|-------------| | `follow_symlinks` | `True` | Follow symlinks; set `False` to skip them | | `preserve_permissions` | `True` | Mirror source permissions on the destination | +| `fail_fast` | `False` | Raise on the first error instead of collecting failures | ### Concurrent File Operations diff --git a/docs/synchronous.md b/docs/synchronous.md index 8b0d11a..a2b0b29 100644 --- a/docs/synchronous.md +++ b/docs/synchronous.md @@ -125,30 +125,37 @@ print(contents) ### Directory Transfers -Upload or download entire directory trees with a single call. Both methods return a tuple of `(files_copied, bytes_transferred)`. +Upload or download entire directory trees with a single call. Both methods return a tuple of `(transferred_files, failed_files)`, where each is a list of file paths. ```python # Upload an entire local directory to the remote server -files_copied, bytes_transferred = conn.sftp_put_dir( +transferred, failed = conn.sftp_put_dir( local_path="/local/build/", remote_path="/remote/app/", ) +if failed: + print(f"Failed to upload: {failed}") # Download an entire remote directory to a local destination -files_copied, bytes_transferred = conn.sftp_get_dir( +transferred, failed = conn.sftp_get_dir( remote_path="/remote/logs/", local_path="/local/logs/", ) ``` -Optional keyword arguments control symlink and permission behaviour: +Optional keyword arguments control symlink, permission, and error-handling behaviour: | Parameter | Default | Description | |-----------|---------|-------------| | `follow_symlinks` | `True` | Follow symlinks; set `False` to skip them | | `preserve_permissions` | `True` | Mirror source permissions on the destination | +| `fail_fast` | `False` | Raise on the first error instead of collecting failures | ```python +# Raise immediately on the first error +conn.sftp_put_dir("/src/", "/dst/", fail_fast=True) + +# Skip symlinks and don't mirror permissions conn.sftp_put_dir("/src/", "/dst/", follow_symlinks=False, preserve_permissions=False) ``` diff --git a/src/asynchronous.rs b/src/asynchronous.rs index 02d9480..d9b9258 100644 --- a/src/asynchronous.rs +++ b/src/asynchronous.rs @@ -81,10 +81,10 @@ //! files = await conn.sftp_list("/remote/path") //! //! # Upload an entire local directory recursively -//! files_copied, bytes_transferred = await conn.sftp_put_dir("/local/build/", "/remote/app/") +//! transferred, failed = await conn.sftp_put_dir("/local/build/", "/remote/app/") //! //! # Download an entire remote directory recursively -//! files_copied, bytes_transferred = await conn.sftp_get_dir("/remote/logs/", "/local/logs/") +//! transferred, failed = await conn.sftp_get_dir("/remote/logs/", "/local/logs/") //! ``` //! //! For file tailing: @@ -230,21 +230,23 @@ impl Handler for ClientHandler { /// /// ### `sftp_put_dir` /// -/// Uploads a local directory recursively to a remote path over SFTP. Returns (files_copied, bytes_transferred). It takes the following parameters: +/// Uploads a local directory recursively to a remote path over SFTP. Returns (transferred_files, failed_files). It takes the following parameters: /// /// * `local_path`: The local directory to upload. /// * `remote_path`: The remote destination path. /// * `follow_symlinks`: Whether to follow symlinks (default: true). /// * `preserve_permissions`: Whether to preserve file permissions (default: true). +/// * `fail_fast`: Whether to raise an exception on the first error instead of collecting failures (default: false). /// /// ### `sftp_get_dir` /// -/// Downloads a remote directory recursively to a local path over SFTP. Returns (files_copied, bytes_transferred). It takes the following parameters: +/// Downloads a remote directory recursively to a local path over SFTP. Returns (transferred_files, failed_files). It takes the following parameters: /// /// * `remote_path`: The remote directory to download. /// * `local_path`: The local destination path. /// * `follow_symlinks`: Whether to follow symlinks (default: true). /// * `preserve_permissions`: Whether to preserve file permissions (default: true). +/// * `fail_fast`: Whether to raise an exception on the first error instead of collecting failures (default: false). /// /// ### `shell` /// @@ -605,22 +607,31 @@ impl AsyncConnection { remote_path: String, follow_symlinks: bool, preserve_permissions: bool, - ) -> PyResult<(u64, u64)> { + fail_fast: bool, + ) -> PyResult<(Vec, Vec)> { let sftp = AsyncConnection::get_or_init_sftp(self.session.clone(), self.sftp_session.clone()) .await?; - let mut files_copied = 0u64; - let mut bytes_transferred = 0u64; + let mut transferred: Vec = Vec::new(); + let mut failed: Vec = Vec::new(); // Ensure remote base directory exists if !sftp.try_exists(&remote_path).await.unwrap_or(false) { - sftp.create_dir(&remote_path).await.map_err(|e| { - PyRuntimeError::new_err(format!( - "Failed to create remote directory '{}': {}", - remote_path, e - )) - })?; + match sftp.create_dir(&remote_path).await { + Ok(_) => {} + Err(e) => { + let msg = format!( + "Failed to create remote directory '{}': {}", + remote_path, e + ); + if fail_fast { + return Err(PyRuntimeError::new_err(msg)); + } + failed.push(remote_path); + return Ok((transferred, failed)); + } + } } // Stack for depth-first traversal: (local_dir, remote_dir) @@ -630,42 +641,75 @@ impl AsyncConnection { )]; while let Some((local_dir, remote_dir)) = dirs_to_process.pop() { - let mut read_dir = tokio::fs::read_dir(&local_dir).await.map_err(|e| { - PyRuntimeError::new_err(format!( - "Failed to read local directory '{}': {}", - local_dir.display(), - e - )) - })?; + let mut read_dir = match tokio::fs::read_dir(&local_dir).await { + Ok(d) => d, + Err(e) => { + let msg = format!( + "Failed to read local directory '{}': {}", + local_dir.display(), + e + ); + if fail_fast { + return Err(PyRuntimeError::new_err(msg)); + } + failed.push(local_dir.to_string_lossy().to_string()); + continue; + } + }; + + loop { + let entry = match read_dir.next_entry().await { + Ok(Some(e)) => e, + Ok(None) => break, + Err(e) => { + if fail_fast { + return Err(PyRuntimeError::new_err(format!( + "Directory entry error: {}", + e + ))); + } + continue; + } + }; - while let Some(entry) = read_dir.next_entry().await.map_err(|e| { - PyRuntimeError::new_err(format!("Directory entry error: {}", e)) - })? { let local_entry = entry.path(); + let local_entry_str = local_entry.to_string_lossy().to_string(); let remote_entry = remote_dir.join(entry.file_name()); let remote_entry_str = remote_entry.to_string_lossy().to_string(); - let metadata = if follow_symlinks { + let metadata = match if follow_symlinks { tokio::fs::metadata(&local_entry).await } else { tokio::fs::symlink_metadata(&local_entry).await - } - .map_err(|e| { - PyRuntimeError::new_err(format!( - "Metadata error for '{}': {}", - local_entry.display(), - e - )) - })?; + } { + Ok(m) => m, + Err(e) => { + if fail_fast { + return Err(PyRuntimeError::new_err(format!( + "Metadata error for '{}': {}", + local_entry_str, e + ))); + } + failed.push(local_entry_str); + continue; + } + }; if metadata.is_dir() { if !sftp.try_exists(&remote_entry_str).await.unwrap_or(false) { - sftp.create_dir(&remote_entry_str).await.map_err(|e| { - PyRuntimeError::new_err(format!( - "Failed to create remote directory '{}': {}", - remote_entry_str, e - )) - })?; + match sftp.create_dir(&remote_entry_str).await { + Ok(_) => {} + Err(e) => { + if fail_fast { + return Err(PyRuntimeError::new_err(format!( + "Failed to create remote directory '{}': {}", + remote_entry_str, e + ))); + } + failed.push(local_entry_str); + continue; + } + } } #[cfg(unix)] if preserve_permissions { @@ -677,44 +721,54 @@ impl AsyncConnection { } dirs_to_process.push((local_entry, remote_entry)); } else if metadata.is_file() { - let mut local_file = - tokio::fs::File::open(&local_entry).await.map_err(|e| { - PyRuntimeError::new_err(format!("File open error: {}", e)) - })?; - let mut remote_file = sftp.create(&remote_entry_str).await.map_err(|e| { - PyRuntimeError::new_err(format!( - "Remote file creation error '{}': {}", - remote_entry_str, e - )) - })?; - let mut buffer = vec![0u8; MAX_BUFF_SIZE]; - loop { - let n = local_file.read(&mut buffer).await.map_err(|e| { - PyRuntimeError::new_err(format!("File read error: {}", e)) - })?; - if n == 0 { - break; + let transfer_result: Result<(), String> = async { + let mut local_file = tokio::fs::File::open(&local_entry) + .await + .map_err(|e| format!("File open error: {}", e))?; + let mut remote_file = sftp + .create(&remote_entry_str) + .await + .map_err(|e| format!("Remote file creation error: {}", e))?; + let mut buffer = vec![0u8; MAX_BUFF_SIZE]; + loop { + let n = local_file + .read(&mut buffer) + .await + .map_err(|e| format!("File read error: {}", e))?; + if n == 0 { + break; + } + remote_file + .write_all(&buffer[..n]) + .await + .map_err(|e| format!("Remote write error: {}", e))?; } - remote_file.write_all(&buffer[..n]).await.map_err(|e| { - PyRuntimeError::new_err(format!("Remote write error: {}", e)) - })?; - bytes_transferred += n as u64; + #[cfg(unix)] + if preserve_permissions { + use std::os::unix::fs::PermissionsExt; + let mode = metadata.permissions().mode(); + let mut attrs = russh_sftp::client::fs::Metadata::default(); + attrs.permissions = Some(mode); + let _ = sftp.set_metadata(&remote_entry_str, attrs).await; + } + Ok(()) } - #[cfg(unix)] - if preserve_permissions { - use std::os::unix::fs::PermissionsExt; - let mode = metadata.permissions().mode(); - let mut attrs = russh_sftp::client::fs::Metadata::default(); - attrs.permissions = Some(mode); - let _ = sftp.set_metadata(&remote_entry_str, attrs).await; + .await; + match transfer_result { + Ok(_) => transferred.push(local_entry_str), + Err(e) => { + if fail_fast { + return Err(PyRuntimeError::new_err(e)); + } + failed.push(local_entry_str); + } } - files_copied += 1; } // Symlinks with follow_symlinks=false are skipped } } - Ok((files_copied, bytes_transferred)) + Ok((transferred, failed)) } /// Internal async SFTP get_dir implementation @@ -724,21 +778,30 @@ impl AsyncConnection { local_path: String, follow_symlinks: bool, preserve_permissions: bool, - ) -> PyResult<(u64, u64)> { + fail_fast: bool, + ) -> PyResult<(Vec, Vec)> { let sftp = AsyncConnection::get_or_init_sftp(self.session.clone(), self.sftp_session.clone()) .await?; - let mut files_copied = 0u64; - let mut bytes_transferred = 0u64; + let mut transferred: Vec = Vec::new(); + let mut failed: Vec = Vec::new(); // Ensure local base directory exists - tokio::fs::create_dir_all(&local_path).await.map_err(|e| { - PyRuntimeError::new_err(format!( - "Failed to create local directory '{}': {}", - local_path, e - )) - })?; + match tokio::fs::create_dir_all(&local_path).await { + Ok(_) => {} + Err(e) => { + let msg = format!( + "Failed to create local directory '{}': {}", + local_path, e + ); + if fail_fast { + return Err(PyRuntimeError::new_err(msg)); + } + failed.push(remote_path); + return Ok((transferred, failed)); + } + } // Stack for depth-first traversal: (remote_dir, local_dir) let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( @@ -748,12 +811,20 @@ impl AsyncConnection { while let Some((remote_dir, local_dir)) = dirs_to_process.pop() { let remote_dir_str = remote_dir.to_string_lossy().to_string(); - let read_dir = sftp.read_dir(&remote_dir_str).await.map_err(|e| { - PyRuntimeError::new_err(format!( - "Failed to read remote directory '{}': {}", - remote_dir_str, e - )) - })?; + let read_dir = match sftp.read_dir(&remote_dir_str).await { + Ok(d) => d, + Err(e) => { + let msg = format!( + "Failed to read remote directory '{}': {}", + remote_dir_str, e + ); + if fail_fast { + return Err(PyRuntimeError::new_err(msg)); + } + failed.push(remote_dir_str); + continue; + } + }; for entry in read_dir { let file_name = entry.file_name(); @@ -775,13 +846,20 @@ impl AsyncConnection { // follow_symlinks=false: skip symlinks continue; } else if file_type.is_dir() { - tokio::fs::create_dir_all(&local_entry).await.map_err(|e| { - PyRuntimeError::new_err(format!( - "Failed to create local directory '{}': {}", - local_entry.display(), - e - )) - })?; + match tokio::fs::create_dir_all(&local_entry).await { + Ok(_) => {} + Err(e) => { + if fail_fast { + return Err(PyRuntimeError::new_err(format!( + "Failed to create local directory '{}': {}", + local_entry.display(), + e + ))); + } + failed.push(remote_entry_str); + continue; + } + } #[cfg(unix)] if preserve_permissions { if let Some(perm) = sftp @@ -800,51 +878,65 @@ impl AsyncConnection { } dirs_to_process.push((remote_entry, local_entry)); } else if file_type.is_file() { - let mut remote_file = sftp.open(&remote_entry_str).await.map_err(|e| { - PyRuntimeError::new_err(format!("SFTP open error: {}", e)) - })?; - let mut local_file = - tokio::fs::File::create(&local_entry).await.map_err(|e| { - PyRuntimeError::new_err(format!("File create error: {}", e)) - })?; - let mut buffer = vec![0u8; MAX_BUFF_SIZE]; - loop { - let n = remote_file.read(&mut buffer).await.map_err(|e| { - PyRuntimeError::new_err(format!("File read error: {}", e)) - })?; - if n == 0 { - break; + let transfer_result: Result<(), String> = async { + let mut remote_file = sftp + .open(&remote_entry_str) + .await + .map_err(|e| format!("SFTP open error: {}", e))?; + let mut local_file = tokio::fs::File::create(&local_entry) + .await + .map_err(|e| format!("File create error: {}", e))?; + let mut buffer = vec![0u8; MAX_BUFF_SIZE]; + loop { + let n = remote_file + .read(&mut buffer) + .await + .map_err(|e| format!("File read error: {}", e))?; + if n == 0 { + break; + } + local_file + .write_all(&buffer[..n]) + .await + .map_err(|e| format!("File write error: {}", e))?; } - local_file.write_all(&buffer[..n]).await.map_err(|e| { - PyRuntimeError::new_err(format!("File write error: {}", e)) - })?; - bytes_transferred += n as u64; - } - local_file.flush().await.map_err(|e| { - PyRuntimeError::new_err(format!("Flush error: {}", e)) - })?; - #[cfg(unix)] - if preserve_permissions { - if let Some(perm) = sftp - .metadata(&remote_entry_str) + local_file + .flush() .await - .ok() - .and_then(|m| m.permissions) - { - use std::os::unix::fs::PermissionsExt; - let _ = tokio::fs::set_permissions( - &local_entry, - std::fs::Permissions::from_mode(perm & 0o7777), - ) - .await; + .map_err(|e| format!("Flush error: {}", e))?; + #[cfg(unix)] + if preserve_permissions { + if let Some(perm) = sftp + .metadata(&remote_entry_str) + .await + .ok() + .and_then(|m| m.permissions) + { + use std::os::unix::fs::PermissionsExt; + let _ = tokio::fs::set_permissions( + &local_entry, + std::fs::Permissions::from_mode(perm & 0o7777), + ) + .await; + } + } + Ok(()) + } + .await; + match transfer_result { + Ok(_) => transferred.push(remote_entry_str), + Err(e) => { + if fail_fast { + return Err(PyRuntimeError::new_err(e)); + } + failed.push(remote_entry_str); } } - files_copied += 1; } } } - Ok((files_copied, bytes_transferred)) + Ok((transferred, failed)) } @@ -1029,8 +1121,9 @@ impl AsyncConnection { } /// Uploads a local directory recursively to a remote path over SFTP. - /// Returns a tuple of (files_copied, bytes_transferred). - #[pyo3(signature = (local_path, remote_path, follow_symlinks=true, preserve_permissions=true))] + /// Returns a tuple of (transferred_files, failed_files). + /// If `fail_fast` is true, the first transfer error raises an exception immediately. + #[pyo3(signature = (local_path, remote_path, follow_symlinks=true, preserve_permissions=true, fail_fast=false))] fn sftp_put_dir<'p>( &self, py: Python<'p>, @@ -1038,17 +1131,25 @@ impl AsyncConnection { remote_path: String, follow_symlinks: bool, preserve_permissions: bool, + fail_fast: bool, ) -> PyResult> { let conn = self.clone(); pyo3_async_runtimes::tokio::future_into_py(py, async move { - conn.sftp_put_dir_async(local_path, remote_path, follow_symlinks, preserve_permissions) - .await + conn.sftp_put_dir_async( + local_path, + remote_path, + follow_symlinks, + preserve_permissions, + fail_fast, + ) + .await }) } /// Downloads a remote directory recursively to a local path over SFTP. - /// Returns a tuple of (files_copied, bytes_transferred). - #[pyo3(signature = (remote_path, local_path, follow_symlinks=true, preserve_permissions=true))] + /// Returns a tuple of (transferred_files, failed_files). + /// If `fail_fast` is true, the first transfer error raises an exception immediately. + #[pyo3(signature = (remote_path, local_path, follow_symlinks=true, preserve_permissions=true, fail_fast=false))] fn sftp_get_dir<'p>( &self, py: Python<'p>, @@ -1056,11 +1157,18 @@ impl AsyncConnection { local_path: String, follow_symlinks: bool, preserve_permissions: bool, + fail_fast: bool, ) -> PyResult> { let conn = self.clone(); pyo3_async_runtimes::tokio::future_into_py(py, async move { - conn.sftp_get_dir_async(remote_path, local_path, follow_symlinks, preserve_permissions) - .await + conn.sftp_get_dir_async( + remote_path, + local_path, + follow_symlinks, + preserve_permissions, + fail_fast, + ) + .await }) } diff --git a/src/connection.rs b/src/connection.rs index bfa70db..b697514 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -186,21 +186,23 @@ impl SSHResult { /// /// ### `sftp_put_dir` /// -/// Uploads a local directory recursively to a remote path over SFTP. Returns (files_copied, bytes_transferred). It takes the following parameters: +/// Uploads a local directory recursively to a remote path over SFTP. Returns (transferred_files, failed_files). It takes the following parameters: /// /// * `local_path`: The local directory to upload. /// * `remote_path`: The remote destination path. /// * `follow_symlinks`: Whether to follow symlinks (default: true). /// * `preserve_permissions`: Whether to preserve file permissions (default: true). +/// * `fail_fast`: Whether to raise an exception on the first error instead of collecting failures (default: false). /// /// ### `sftp_get_dir` /// -/// Downloads a remote directory recursively to a local path over SFTP. Returns (files_copied, bytes_transferred). It takes the following parameters: +/// Downloads a remote directory recursively to a local path over SFTP. Returns (transferred_files, failed_files). It takes the following parameters: /// /// * `remote_path`: The remote directory to download. /// * `local_path`: The local destination path. /// * `follow_symlinks`: Whether to follow symlinks (default: true). /// * `preserve_permissions`: Whether to preserve file permissions (default: true). +/// * `fail_fast`: Whether to raise an exception on the first error instead of collecting failures (default: false). /// /// ### `shell` /// @@ -580,28 +582,36 @@ impl Connection { } /// Uploads a local directory recursively to a remote path over SFTP. - /// Returns a tuple of (files_copied, bytes_transferred). - #[pyo3(signature = (local_path, remote_path, follow_symlinks=true, preserve_permissions=true))] + /// Returns a tuple of (transferred_files, failed_files), where each is a list of local file paths. + /// If `fail_fast` is true, the first transfer error raises an exception immediately. + #[pyo3(signature = (local_path, remote_path, follow_symlinks=true, preserve_permissions=true, fail_fast=false))] fn sftp_put_dir( &mut self, local_path: String, remote_path: String, follow_symlinks: bool, preserve_permissions: bool, - ) -> PyResult<(u64, u64)> { - let mut files_copied = 0u64; - let mut bytes_transferred = 0u64; + fail_fast: bool, + ) -> PyResult<(Vec, Vec)> { + let mut transferred: Vec = Vec::new(); + let mut failed: Vec = Vec::new(); // Ensure remote base directory exists if self.sftp().stat(Path::new(&remote_path)).is_err() { - self.sftp() - .mkdir(Path::new(&remote_path), 0o755) - .map_err(|e| { - PyErr::new::(format!( + match self.sftp().mkdir(Path::new(&remote_path), 0o755) { + Ok(_) => {} + Err(e) => { + let msg = format!( "Failed to create remote directory '{}': {}", remote_path, e - )) - })?; + ); + if fail_fast { + return Err(PyErr::new::(msg)); + } + failed.push(remote_path); + return Ok((transferred, failed)); + } + } } // Stack for depth-first traversal: (local_dir, remote_dir) @@ -611,35 +621,58 @@ impl Connection { )]; while let Some((local_dir, remote_dir)) = dirs_to_process.pop() { - let entries = std::fs::read_dir(&local_dir).map_err(|e| { - PyErr::new::(format!( - "Failed to read local directory '{}': {}", - local_dir.display(), - e - )) - })?; + let entries = match std::fs::read_dir(&local_dir) { + Ok(e) => e, + Err(e) => { + let msg = format!( + "Failed to read local directory '{}': {}", + local_dir.display(), + e + ); + if fail_fast { + return Err(PyErr::new::(msg)); + } + failed.push(local_dir.to_string_lossy().to_string()); + continue; + } + }; for entry in entries { - let entry = entry.map_err(|e| { - PyErr::new::(format!("Directory entry error: {}", e)) - })?; + let entry = match entry { + Ok(e) => e, + Err(e) => { + if fail_fast { + return Err(PyErr::new::(format!( + "Directory entry error: {}", + e + ))); + } + continue; + } + }; let local_entry = entry.path(); + let local_entry_str = local_entry.to_string_lossy().to_string(); let remote_entry = remote_dir.join(entry.file_name()); let remote_entry_str = remote_entry.to_string_lossy().to_string(); - let metadata = if follow_symlinks { + let metadata = match if follow_symlinks { std::fs::metadata(&local_entry) } else { std::fs::symlink_metadata(&local_entry) - } - .map_err(|e| { - PyErr::new::(format!( - "Metadata error for '{}': {}", - local_entry.display(), - e - )) - })?; + } { + Ok(m) => m, + Err(e) => { + if fail_fast { + return Err(PyErr::new::(format!( + "Metadata error for '{}': {}", + local_entry_str, e + ))); + } + failed.push(local_entry_str); + continue; + } + }; if metadata.is_dir() { #[cfg(unix)] @@ -652,92 +685,110 @@ impl Connection { #[cfg(not(unix))] let mode = 0o755i32; if self.sftp().stat(Path::new(&remote_entry_str)).is_err() { - self.sftp() - .mkdir(Path::new(&remote_entry_str), mode) - .map_err(|e| { - PyErr::new::(format!( - "Failed to create remote directory '{}': {}", - remote_entry_str, e - )) - })?; + match self.sftp().mkdir(Path::new(&remote_entry_str), mode) { + Ok(_) => {} + Err(e) => { + if fail_fast { + return Err(PyErr::new::(format!( + "Failed to create remote directory '{}': {}", + remote_entry_str, e + ))); + } + failed.push(local_entry_str); + continue; + } + } } dirs_to_process.push((local_entry, remote_entry)); } else if metadata.is_file() { - let mut local_file = std::fs::File::open(&local_entry).map_err(|e| { - PyErr::new::(format!("File open error: {}", e)) - })?; - let file_size = metadata.len(); - let mut remote_file = self - .sftp() - .create(Path::new(&remote_entry_str)) - .map_err(|e| { - PyErr::new::(format!( - "Remote file creation error '{}': {}", - remote_entry_str, e - )) - })?; - let buf_size = (file_size as usize).min(MAX_BUFF_SIZE); - let mut buffer = vec![0u8; buf_size]; - loop { - let n = local_file.read(&mut buffer).map_err(|e| { - PyErr::new::(format!("File read error: {}", e)) - })?; - if n == 0 { - break; + let result: Result<(), String> = (|| { + let mut local_file = std::fs::File::open(&local_entry) + .map_err(|e| format!("File open error: {}", e))?; + let file_size = metadata.len(); + let mut remote_file = self + .sftp() + .create(Path::new(&remote_entry_str)) + .map_err(|e| format!("Remote file creation error: {}", e))?; + let buf_size = (file_size as usize).min(MAX_BUFF_SIZE); + // Fall back to MAX_BUFF_SIZE for empty files (size 0) so the read loop can still run. + let buf_size = if buf_size == 0 { MAX_BUFF_SIZE } else { buf_size }; + let mut buffer = vec![0u8; buf_size]; + loop { + let n = local_file + .read(&mut buffer) + .map_err(|e| format!("File read error: {}", e))?; + if n == 0 { + break; + } + remote_file + .write_all(&buffer[..n]) + .map_err(|e| format!("Remote write error: {}", e))?; + } + remote_file + .close() + .map_err(|e| format!("Close error: {}", e))?; + #[cfg(unix)] + if preserve_permissions { + use std::os::unix::fs::PermissionsExt; + let mode = metadata.permissions().mode(); + let _ = self.sftp().setstat( + Path::new(&remote_entry_str), + ssh2::FileStat { + perm: Some(mode), + size: None, + uid: None, + gid: None, + atime: None, + mtime: None, + }, + ); + } + Ok(()) + })(); + match result { + Ok(_) => transferred.push(local_entry_str), + Err(e) => { + if fail_fast { + return Err(PyErr::new::(e)); + } + failed.push(local_entry_str); } - remote_file.write_all(&buffer[..n]).map_err(|e| { - PyErr::new::(format!("Remote write error: {}", e)) - })?; - bytes_transferred += n as u64; - } - remote_file.close().map_err(|e| { - PyErr::new::(format!("Close error: {}", e)) - })?; - #[cfg(unix)] - if preserve_permissions { - use std::os::unix::fs::PermissionsExt; - let mode = metadata.permissions().mode(); - let _ = self.sftp().setstat( - Path::new(&remote_entry_str), - ssh2::FileStat { - perm: Some(mode), - size: None, - uid: None, - gid: None, - atime: None, - mtime: None, - }, - ); } - files_copied += 1; } // Symlinks with follow_symlinks=false are skipped } } - Ok((files_copied, bytes_transferred)) + Ok((transferred, failed)) } /// Downloads a remote directory recursively to a local path over SFTP. - /// Returns a tuple of (files_copied, bytes_transferred). - #[pyo3(signature = (remote_path, local_path, follow_symlinks=true, preserve_permissions=true))] + /// Returns a tuple of (transferred_files, failed_files), where each is a list of remote file paths. + /// If `fail_fast` is true, the first transfer error raises an exception immediately. + #[pyo3(signature = (remote_path, local_path, follow_symlinks=true, preserve_permissions=true, fail_fast=false))] fn sftp_get_dir( &mut self, remote_path: String, local_path: String, follow_symlinks: bool, preserve_permissions: bool, - ) -> PyResult<(u64, u64)> { - let mut files_copied = 0u64; - let mut bytes_transferred = 0u64; + fail_fast: bool, + ) -> PyResult<(Vec, Vec)> { + let mut transferred: Vec = Vec::new(); + let mut failed: Vec = Vec::new(); // Ensure local base directory exists - std::fs::create_dir_all(&local_path).map_err(|e| { - PyErr::new::(format!( + if let Err(e) = std::fs::create_dir_all(&local_path) { + let msg = format!( "Failed to create local directory '{}': {}", local_path, e - )) - })?; + ); + if fail_fast { + return Err(PyErr::new::(msg)); + } + failed.push(remote_path); + return Ok((transferred, failed)); + } // Stack for depth-first traversal: (remote_dir, local_dir) let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( @@ -747,21 +798,31 @@ impl Connection { while let Some((remote_dir, local_dir)) = dirs_to_process.pop() { let remote_dir_str = remote_dir.to_string_lossy().to_string(); - let entries = self - .sftp() - .readdir(Path::new(&remote_dir_str)) - .map_err(|e| { - PyErr::new::(format!( + let entries = match self.sftp().readdir(Path::new(&remote_dir_str)) { + Ok(e) => e, + Err(e) => { + let msg = format!( "Failed to read remote directory '{}': {}", remote_dir_str, e - )) - })?; + ); + if fail_fast { + return Err(PyErr::new::(msg)); + } + failed.push(remote_dir_str); + continue; + } + }; for (entry_name, stat) in entries { - let file_name = entry_name - .file_name() - .ok_or_else(|| PyErr::new::("Invalid entry path"))? - .to_os_string(); + let file_name = match entry_name.file_name() { + Some(n) => n.to_os_string(), + None => { + if fail_fast { + return Err(PyErr::new::("Invalid entry path")); + } + continue; + } + }; let local_entry = local_dir.join(&file_name); let remote_entry = remote_dir.join(&entry_name); let remote_entry_str = remote_entry.to_string_lossy().to_string(); @@ -779,13 +840,20 @@ impl Connection { // follow_symlinks=false: skip symlinks continue; } else if resolved_stat.is_dir() { - std::fs::create_dir_all(&local_entry).map_err(|e| { - PyErr::new::(format!( - "Failed to create local directory '{}': {}", - local_entry.display(), - e - )) - })?; + match std::fs::create_dir_all(&local_entry) { + Ok(_) => {} + Err(e) => { + if fail_fast { + return Err(PyErr::new::(format!( + "Failed to create local directory '{}': {}", + local_entry.display(), + e + ))); + } + failed.push(remote_entry_str); + continue; + } + } #[cfg(unix)] if preserve_permissions { if let Some(perm) = resolved_stat.perm { @@ -798,49 +866,56 @@ impl Connection { } dirs_to_process.push((remote_entry, local_entry)); } else if resolved_stat.is_file() { - let mut remote_file = BufReader::new( - self.sftp() - .open(Path::new(&remote_entry_str)) - .map_err(|e| { - PyErr::new::(format!("SFTP open error: {}", e)) - })?, - ); - let local_file = std::fs::File::create(&local_entry).map_err(|e| { - PyErr::new::(format!("File create error: {}", e)) - })?; - let mut writer = BufWriter::new(local_file); - let mut buffer = vec![0u8; MAX_BUFF_SIZE]; - loop { - let n = remote_file.read(&mut buffer).map_err(|e| { - PyErr::new::(format!("File read error: {}", e)) - })?; - if n == 0 { - break; + let result: Result<(), String> = (|| { + let mut remote_file = BufReader::new( + self.sftp() + .open(Path::new(&remote_entry_str)) + .map_err(|e| format!("SFTP open error: {}", e))?, + ); + let local_file = std::fs::File::create(&local_entry) + .map_err(|e| format!("File create error: {}", e))?; + let mut writer = BufWriter::new(local_file); + let mut buffer = vec![0u8; MAX_BUFF_SIZE]; + loop { + let n = remote_file + .read(&mut buffer) + .map_err(|e| format!("File read error: {}", e))?; + if n == 0 { + break; + } + writer + .write_all(&buffer[..n]) + .map_err(|e| format!("File write error: {}", e))?; } - writer.write_all(&buffer[..n]).map_err(|e| { - PyErr::new::(format!("File write error: {}", e)) - })?; - bytes_transferred += n as u64; - } - writer.flush().map_err(|e| { - PyErr::new::(format!("Flush error: {}", e)) - })?; - #[cfg(unix)] - if preserve_permissions { - if let Some(perm) = resolved_stat.perm { - use std::os::unix::fs::PermissionsExt; - let _ = std::fs::set_permissions( - &local_entry, - std::fs::Permissions::from_mode(perm & 0o7777), - ); + writer + .flush() + .map_err(|e| format!("Flush error: {}", e))?; + #[cfg(unix)] + if preserve_permissions { + if let Some(perm) = resolved_stat.perm { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions( + &local_entry, + std::fs::Permissions::from_mode(perm & 0o7777), + ); + } + } + Ok(()) + })(); + match result { + Ok(_) => transferred.push(remote_entry_str), + Err(e) => { + if fail_fast { + return Err(PyErr::new::(e)); + } + failed.push(remote_entry_str); } } - files_copied += 1; } } } - Ok((files_copied, bytes_transferred)) + Ok((transferred, failed)) } // Copy a file from this connection to another connection diff --git a/tests/test_async_connection.py b/tests/test_async_connection.py index 2b70fb8..5ad5ac6 100644 --- a/tests/test_async_connection.py +++ b/tests/test_async_connection.py @@ -80,17 +80,16 @@ async def test_async_sftp_put_dir(run_test_server, tmp_path): (sub / "nested.txt").write_text("nested content") # Upload to remote - files_copied, bytes_transferred = await conn.sftp_put_dir( - str(src), "/root/async_test_put_dir" - ) + transferred, failed = await conn.sftp_put_dir(str(src), "/root/async_test_put_dir") # Verify files exist on remote remote_ls = (await conn.execute("find /root/async_test_put_dir -type f | sort")).stdout assert "file1.txt" in remote_ls assert "file2.txt" in remote_ls assert "nested.txt" in remote_ls - assert files_copied == 3 - assert bytes_transferred > 0 + assert len(transferred) == 3 + assert len(failed) == 0 + assert any("file1.txt" in p for p in transferred) # Verify nested file contents content = await conn.sftp_read("/root/async_test_put_dir/subdir/nested.txt") @@ -114,21 +113,29 @@ async def test_async_sftp_get_dir(run_test_server, tmp_path): # Download to local dest = tmp_path / "async_dest_dir" - files_copied, bytes_transferred = await conn.sftp_get_dir( - "/root/async_test_get_dir", str(dest) - ) + transferred, failed = await conn.sftp_get_dir("/root/async_test_get_dir", str(dest)) # Verify local files assert (dest / "file1.txt").read_text() == "remote file 1" assert (dest / "file2.txt").read_text() == "remote file 2" assert (dest / "subdir" / "nested.txt").read_text() == "nested remote" - assert files_copied == 3 - assert bytes_transferred > 0 + assert len(transferred) == 3 + assert len(failed) == 0 + assert any("file1.txt" in p for p in transferred) # Cleanup await conn.execute("rm -rf /root/async_test_get_dir") +@pytest.mark.asyncio +async def test_async_sftp_get_dir_fail_fast(run_test_server, tmp_path): + """Test that sftp_get_dir with fail_fast=True raises on error.""" + async with AsyncConnection("localhost", username="root", password="toor", port=8022) as conn: + dest = tmp_path / "async_dest_fail" + with pytest.raises(RuntimeError): + await conn.sftp_get_dir("/path/does/not/exist", str(dest), fail_fast=True) + + @pytest.mark.asyncio async def test_async_shell(run_test_server): async with ( diff --git a/tests/test_connection.py b/tests/test_connection.py index d331f63..fd1bec9 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -224,15 +224,17 @@ def test_sftp_put_dir(conn, tmp_path): (sub / "nested.txt").write_text("nested content") # Upload to remote - files_copied, bytes_transferred = conn.sftp_put_dir(str(src), "/root/test_put_dir") + transferred, failed = conn.sftp_put_dir(str(src), "/root/test_put_dir") # Verify files exist on remote remote_ls = conn.execute("find /root/test_put_dir -type f | sort").stdout assert "file1.txt" in remote_ls assert "file2.txt" in remote_ls assert "nested.txt" in remote_ls - assert files_copied == 3 - assert bytes_transferred > 0 + assert len(transferred) == 3 + assert len(failed) == 0 + # Transferred list contains local paths + assert any("file1.txt" in p for p in transferred) # Verify nested file contents content = conn.sftp_read("/root/test_put_dir/subdir/nested.txt") @@ -252,19 +254,43 @@ def test_sftp_get_dir(conn, tmp_path): # Download to local dest = tmp_path / "dest_dir" - files_copied, bytes_transferred = conn.sftp_get_dir("/root/test_get_dir", str(dest)) + transferred, failed = conn.sftp_get_dir("/root/test_get_dir", str(dest)) # Verify local files assert (dest / "file1.txt").read_text() == "remote file 1" assert (dest / "file2.txt").read_text() == "remote file 2" assert (dest / "subdir" / "nested.txt").read_text() == "nested remote" - assert files_copied == 3 - assert bytes_transferred > 0 + assert len(transferred) == 3 + assert len(failed) == 0 + # Transferred list contains remote paths + assert any("file1.txt" in p for p in transferred) # Cleanup conn.execute("rm -rf /root/test_get_dir") +def test_sftp_put_dir_fail_fast(conn, tmp_path): + """Test that sftp_put_dir with fail_fast=True raises on error.""" + import pytest + + src = tmp_path / "src_fail" + src.mkdir() + (src / "ok.txt").write_text("ok") + + # Try to upload to a path whose parent doesn't exist (invalid path) + with pytest.raises(OSError): + conn.sftp_put_dir(str(src), "/nonexistent_parent/deep/path", fail_fast=True) + + +def test_sftp_get_dir_fail_fast(conn, tmp_path): + """Test that sftp_get_dir with fail_fast=True raises on error.""" + import pytest + + dest = tmp_path / "dest_fail" + with pytest.raises(OSError): + conn.sftp_get_dir("/path/does/not/exist", str(dest), fail_fast=True) + + @pytest.mark.skip("non-text files are not supported by sftp") def test_non_utf8_sftp(conn): """Test that we can copy a non-text file to the server and read it back.""" From 3142f2091bd55d9c06618cd74060a8fa0a194269 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 16:05:07 +0000 Subject: [PATCH 06/12] fix: address clippy field_reassign_with_default and ruff PLR2004/PT011/PLC0415 linting failures Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- src/asynchronous.rs | 12 ++++++++---- tests/test_async_connection.py | 10 +++++----- tests/test_connection.py | 14 ++++++-------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/asynchronous.rs b/src/asynchronous.rs index d9b9258..f0d4218 100644 --- a/src/asynchronous.rs +++ b/src/asynchronous.rs @@ -715,8 +715,10 @@ impl AsyncConnection { if preserve_permissions { use std::os::unix::fs::PermissionsExt; let mode = metadata.permissions().mode(); - let mut attrs = russh_sftp::client::fs::Metadata::default(); - attrs.permissions = Some(mode); + let attrs = russh_sftp::client::fs::Metadata { + permissions: Some(mode), + ..Default::default() + }; let _ = sftp.set_metadata(&remote_entry_str, attrs).await; } dirs_to_process.push((local_entry, remote_entry)); @@ -747,8 +749,10 @@ impl AsyncConnection { if preserve_permissions { use std::os::unix::fs::PermissionsExt; let mode = metadata.permissions().mode(); - let mut attrs = russh_sftp::client::fs::Metadata::default(); - attrs.permissions = Some(mode); + let attrs = russh_sftp::client::fs::Metadata { + permissions: Some(mode), + ..Default::default() + }; let _ = sftp.set_metadata(&remote_entry_str, attrs).await; } Ok(()) diff --git a/tests/test_async_connection.py b/tests/test_async_connection.py index 5ad5ac6..658e707 100644 --- a/tests/test_async_connection.py +++ b/tests/test_async_connection.py @@ -87,7 +87,8 @@ async def test_async_sftp_put_dir(run_test_server, tmp_path): assert "file1.txt" in remote_ls assert "file2.txt" in remote_ls assert "nested.txt" in remote_ls - assert len(transferred) == 3 + expected_file_count = 3 + assert len(transferred) == expected_file_count assert len(failed) == 0 assert any("file1.txt" in p for p in transferred) @@ -107,9 +108,7 @@ async def test_async_sftp_get_dir(run_test_server, tmp_path): await conn.execute("mkdir -p /root/async_test_get_dir/subdir") await conn.sftp_write_data("remote file 1", "/root/async_test_get_dir/file1.txt") await conn.sftp_write_data("remote file 2", "/root/async_test_get_dir/file2.txt") - await conn.sftp_write_data( - "nested remote", "/root/async_test_get_dir/subdir/nested.txt" - ) + await conn.sftp_write_data("nested remote", "/root/async_test_get_dir/subdir/nested.txt") # Download to local dest = tmp_path / "async_dest_dir" @@ -119,7 +118,8 @@ async def test_async_sftp_get_dir(run_test_server, tmp_path): assert (dest / "file1.txt").read_text() == "remote file 1" assert (dest / "file2.txt").read_text() == "remote file 2" assert (dest / "subdir" / "nested.txt").read_text() == "nested remote" - assert len(transferred) == 3 + expected_file_count = 3 + assert len(transferred) == expected_file_count assert len(failed) == 0 assert any("file1.txt" in p for p in transferred) diff --git a/tests/test_connection.py b/tests/test_connection.py index fd1bec9..0d5397b 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -231,7 +231,8 @@ def test_sftp_put_dir(conn, tmp_path): assert "file1.txt" in remote_ls assert "file2.txt" in remote_ls assert "nested.txt" in remote_ls - assert len(transferred) == 3 + expected_file_count = 3 + assert len(transferred) == expected_file_count assert len(failed) == 0 # Transferred list contains local paths assert any("file1.txt" in p for p in transferred) @@ -260,7 +261,8 @@ def test_sftp_get_dir(conn, tmp_path): assert (dest / "file1.txt").read_text() == "remote file 1" assert (dest / "file2.txt").read_text() == "remote file 2" assert (dest / "subdir" / "nested.txt").read_text() == "nested remote" - assert len(transferred) == 3 + expected_file_count = 3 + assert len(transferred) == expected_file_count assert len(failed) == 0 # Transferred list contains remote paths assert any("file1.txt" in p for p in transferred) @@ -271,23 +273,19 @@ def test_sftp_get_dir(conn, tmp_path): def test_sftp_put_dir_fail_fast(conn, tmp_path): """Test that sftp_put_dir with fail_fast=True raises on error.""" - import pytest - src = tmp_path / "src_fail" src.mkdir() (src / "ok.txt").write_text("ok") # Try to upload to a path whose parent doesn't exist (invalid path) - with pytest.raises(OSError): + with pytest.raises(OSError, match=r"(?i)(failed|error|no such)"): conn.sftp_put_dir(str(src), "/nonexistent_parent/deep/path", fail_fast=True) def test_sftp_get_dir_fail_fast(conn, tmp_path): """Test that sftp_get_dir with fail_fast=True raises on error.""" - import pytest - dest = tmp_path / "dest_fail" - with pytest.raises(OSError): + with pytest.raises(OSError, match=r"(?i)(failed|error|no such)"): conn.sftp_get_dir("/path/does/not/exist", str(dest), fail_fast=True) From ca5fb5033a7ab3b76e3eb3fb194112e18d0e9be8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 17:55:03 +0000 Subject: [PATCH 07/12] fix: run cargo fmt to fix Rust formatting check Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- src/asynchronous.rs | 12 ++---------- src/connection.rs | 20 ++++++++------------ 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/asynchronous.rs b/src/asynchronous.rs index f0d4218..f41590e 100644 --- a/src/asynchronous.rs +++ b/src/asynchronous.rs @@ -621,10 +621,7 @@ impl AsyncConnection { match sftp.create_dir(&remote_path).await { Ok(_) => {} Err(e) => { - let msg = format!( - "Failed to create remote directory '{}': {}", - remote_path, e - ); + let msg = format!("Failed to create remote directory '{}': {}", remote_path, e); if fail_fast { return Err(PyRuntimeError::new_err(msg)); } @@ -795,10 +792,7 @@ impl AsyncConnection { match tokio::fs::create_dir_all(&local_path).await { Ok(_) => {} Err(e) => { - let msg = format!( - "Failed to create local directory '{}': {}", - local_path, e - ); + let msg = format!("Failed to create local directory '{}': {}", local_path, e); if fail_fast { return Err(PyRuntimeError::new_err(msg)); } @@ -943,7 +937,6 @@ impl AsyncConnection { Ok((transferred, failed)) } - pub(crate) fn create( host: String, username: Option, @@ -1176,7 +1169,6 @@ impl AsyncConnection { }) } - fn shell<'p>(&self, py: Python<'p>, pty: Option) -> PyResult> { let session_arc = self.session.clone(); let pty = pty.unwrap_or(false); diff --git a/src/connection.rs b/src/connection.rs index b697514..747dc3c 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -601,10 +601,7 @@ impl Connection { match self.sftp().mkdir(Path::new(&remote_path), 0o755) { Ok(_) => {} Err(e) => { - let msg = format!( - "Failed to create remote directory '{}': {}", - remote_path, e - ); + let msg = format!("Failed to create remote directory '{}': {}", remote_path, e); if fail_fast { return Err(PyErr::new::(msg)); } @@ -711,7 +708,11 @@ impl Connection { .map_err(|e| format!("Remote file creation error: {}", e))?; let buf_size = (file_size as usize).min(MAX_BUFF_SIZE); // Fall back to MAX_BUFF_SIZE for empty files (size 0) so the read loop can still run. - let buf_size = if buf_size == 0 { MAX_BUFF_SIZE } else { buf_size }; + let buf_size = if buf_size == 0 { + MAX_BUFF_SIZE + } else { + buf_size + }; let mut buffer = vec![0u8; buf_size]; loop { let n = local_file @@ -779,10 +780,7 @@ impl Connection { // Ensure local base directory exists if let Err(e) = std::fs::create_dir_all(&local_path) { - let msg = format!( - "Failed to create local directory '{}': {}", - local_path, e - ); + let msg = format!("Failed to create local directory '{}': {}", local_path, e); if fail_fast { return Err(PyErr::new::(msg)); } @@ -887,9 +885,7 @@ impl Connection { .write_all(&buffer[..n]) .map_err(|e| format!("File write error: {}", e))?; } - writer - .flush() - .map_err(|e| format!("Flush error: {}", e))?; + writer.flush().map_err(|e| format!("Flush error: {}", e))?; #[cfg(unix)] if preserve_permissions { if let Some(perm) = resolved_stat.perm { From 5c6f402fa4a92e3c08955668687df2650f1db008 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 22:11:07 +0000 Subject: [PATCH 08/12] fix: flush async SFTP remote file handles with shutdown() after writes Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- src/asynchronous.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/asynchronous.rs b/src/asynchronous.rs index f41590e..5b76a5b 100644 --- a/src/asynchronous.rs +++ b/src/asynchronous.rs @@ -574,6 +574,10 @@ impl AsyncConnection { PyRuntimeError::new_err(format!("Failed to write remote file: {}", e)) })? } + remote_file + .shutdown() + .await + .map_err(|e| PyRuntimeError::new_err(format!("Failed to flush remote file: {}", e)))?; Ok(()) } @@ -597,6 +601,11 @@ impl AsyncConnection { .await .map_err(|e| PyRuntimeError::new_err(format!("Failed to write remote file: {}", e)))?; + remote_file + .shutdown() + .await + .map_err(|e| PyRuntimeError::new_err(format!("Failed to flush remote file: {}", e)))?; + Ok(()) } @@ -742,6 +751,10 @@ impl AsyncConnection { .await .map_err(|e| format!("Remote write error: {}", e))?; } + remote_file + .shutdown() + .await + .map_err(|e| format!("Remote file flush error: {}", e))?; #[cfg(unix)] if preserve_permissions { use std::os::unix::fs::PermissionsExt; From 5c51ff919990344a291f13eaa104aa442bc4e98f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 00:04:02 +0000 Subject: [PATCH 09/12] fix: flush() + shutdown() + mem::forget() to properly close async SFTP file handles Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- src/asynchronous.rs | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/asynchronous.rs b/src/asynchronous.rs index 5b76a5b..8718645 100644 --- a/src/asynchronous.rs +++ b/src/asynchronous.rs @@ -575,9 +575,14 @@ impl AsyncConnection { })? } remote_file - .shutdown() + .flush() .await .map_err(|e| PyRuntimeError::new_err(format!("Failed to flush remote file: {}", e)))?; + remote_file + .shutdown() + .await + .map_err(|e| PyRuntimeError::new_err(format!("Failed to close remote file: {}", e)))?; + std::mem::forget(remote_file); Ok(()) } @@ -602,9 +607,14 @@ impl AsyncConnection { .map_err(|e| PyRuntimeError::new_err(format!("Failed to write remote file: {}", e)))?; remote_file - .shutdown() + .flush() .await .map_err(|e| PyRuntimeError::new_err(format!("Failed to flush remote file: {}", e)))?; + remote_file + .shutdown() + .await + .map_err(|e| PyRuntimeError::new_err(format!("Failed to close remote file: {}", e)))?; + std::mem::forget(remote_file); Ok(()) } @@ -751,10 +761,20 @@ impl AsyncConnection { .await .map_err(|e| format!("Remote write error: {}", e))?; } + // Flush any buffered data then close the SFTP file handle. + // After shutdown() has sent SSH_FXP_CLOSE and received the + // status response, use mem::forget to prevent the Drop impl + // from sending a redundant close that could corrupt the shared + // SFTP session state for subsequent operations. remote_file - .shutdown() + .flush() .await .map_err(|e| format!("Remote file flush error: {}", e))?; + remote_file + .shutdown() + .await + .map_err(|e| format!("Remote file close error: {}", e))?; + std::mem::forget(remote_file); #[cfg(unix)] if preserve_permissions { use std::os::unix::fs::PermissionsExt; From 13368f367183a45cf1274931f08db41ef7d0c92a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 08:36:14 +0000 Subject: [PATCH 10/12] fix: use explicit None fields in set_metadata to prevent size=0 truncation of uploaded files Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- src/asynchronous.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/asynchronous.rs b/src/asynchronous.rs index 8718645..ce0df1b 100644 --- a/src/asynchronous.rs +++ b/src/asynchronous.rs @@ -731,9 +731,18 @@ impl AsyncConnection { if preserve_permissions { use std::os::unix::fs::PermissionsExt; let mode = metadata.permissions().mode(); + // Use explicit None for all fields except permissions. + // FileAttributes::default() sets size: Some(0) which would + // truncate the target via SSH_FXP_SETSTAT. let attrs = russh_sftp::client::fs::Metadata { + size: None, + uid: None, + user: None, + gid: None, + group: None, permissions: Some(mode), - ..Default::default() + atime: None, + mtime: None, }; let _ = sftp.set_metadata(&remote_entry_str, attrs).await; } @@ -779,9 +788,18 @@ impl AsyncConnection { if preserve_permissions { use std::os::unix::fs::PermissionsExt; let mode = metadata.permissions().mode(); + // Use explicit None for all fields except permissions. + // FileAttributes::default() sets size: Some(0) which would + // truncate the file via SSH_FXP_SETSTAT. let attrs = russh_sftp::client::fs::Metadata { + size: None, + uid: None, + user: None, + gid: None, + group: None, permissions: Some(mode), - ..Default::default() + atime: None, + mtime: None, }; let _ = sftp.set_metadata(&remote_entry_str, attrs).await; } From d70445ce578a29df333c12cff3b6e291cda633d2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 23:12:00 +0000 Subject: [PATCH 11/12] =?UTF-8?q?fix:=20address=20review=20comments=20?= =?UTF-8?q?=E2=80=94=20POSIX=20remote=20paths,=20mkdir-p,=20mem::forget=20?= =?UTF-8?q?removal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent-Logs-Url: https://github.com/JacobCallahan/Hussh/sessions/dc7f8f44-2dea-49ab-8962-ac2992439af6 Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- src/asynchronous.rs | 72 +++++++++++++++++++++------------------- src/connection.rs | 65 +++++++++++++++++++++--------------- tests/test_connection.py | 13 ++++++-- 3 files changed, 86 insertions(+), 64 deletions(-) diff --git a/src/asynchronous.rs b/src/asynchronous.rs index ce0df1b..991fbe9 100644 --- a/src/asynchronous.rs +++ b/src/asynchronous.rs @@ -582,7 +582,6 @@ impl AsyncConnection { .shutdown() .await .map_err(|e| PyRuntimeError::new_err(format!("Failed to close remote file: {}", e)))?; - std::mem::forget(remote_file); Ok(()) } @@ -614,7 +613,6 @@ impl AsyncConnection { .shutdown() .await .map_err(|e| PyRuntimeError::new_err(format!("Failed to close remote file: {}", e)))?; - std::mem::forget(remote_file); Ok(()) } @@ -635,26 +633,37 @@ impl AsyncConnection { let mut transferred: Vec = Vec::new(); let mut failed: Vec = Vec::new(); - // Ensure remote base directory exists - if !sftp.try_exists(&remote_path).await.unwrap_or(false) { - match sftp.create_dir(&remote_path).await { - Ok(_) => {} - Err(e) => { - let msg = format!("Failed to create remote directory '{}': {}", remote_path, e); - if fail_fast { - return Err(PyRuntimeError::new_err(msg)); + // Ensure remote base directory (and all missing parents) exist — like mkdir -p + { + let parts: Vec<&str> = remote_path.split('/').filter(|s| !s.is_empty()).collect(); + let is_absolute = remote_path.starts_with('/'); + let mut current = if is_absolute { + String::from("/") + } else { + String::new() + }; + for part in &parts { + if !current.is_empty() && !current.ends_with('/') { + current.push('/'); + } + current.push_str(part); + if !sftp.try_exists(¤t).await.unwrap_or(false) { + if let Err(e) = sftp.create_dir(¤t).await { + let msg = format!("Failed to create remote directory '{}': {}", current, e); + if fail_fast { + return Err(PyRuntimeError::new_err(msg)); + } + failed.push(local_path.clone()); + return Ok((transferred, failed)); } - failed.push(remote_path); - return Ok((transferred, failed)); } } } - // Stack for depth-first traversal: (local_dir, remote_dir) - let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( - std::path::PathBuf::from(&local_path), - std::path::PathBuf::from(&remote_path), - )]; + // Stack for depth-first traversal: (local_dir, remote_dir_str). + // Remote paths are kept as POSIX strings to avoid OS-native path separators on Windows. + let mut dirs_to_process: Vec<(std::path::PathBuf, String)> = + vec![(std::path::PathBuf::from(&local_path), remote_path.clone())]; while let Some((local_dir, remote_dir)) = dirs_to_process.pop() { let mut read_dir = match tokio::fs::read_dir(&local_dir).await { @@ -690,8 +699,8 @@ impl AsyncConnection { let local_entry = entry.path(); let local_entry_str = local_entry.to_string_lossy().to_string(); - let remote_entry = remote_dir.join(entry.file_name()); - let remote_entry_str = remote_entry.to_string_lossy().to_string(); + let file_name_str = entry.file_name().to_string_lossy().to_string(); + let remote_entry_str = format!("{}/{}", remote_dir, file_name_str); let metadata = match if follow_symlinks { tokio::fs::metadata(&local_entry).await @@ -746,7 +755,7 @@ impl AsyncConnection { }; let _ = sftp.set_metadata(&remote_entry_str, attrs).await; } - dirs_to_process.push((local_entry, remote_entry)); + dirs_to_process.push((local_entry, remote_entry_str)); } else if metadata.is_file() { let transfer_result: Result<(), String> = async { let mut local_file = tokio::fs::File::open(&local_entry) @@ -771,10 +780,7 @@ impl AsyncConnection { .map_err(|e| format!("Remote write error: {}", e))?; } // Flush any buffered data then close the SFTP file handle. - // After shutdown() has sent SSH_FXP_CLOSE and received the - // status response, use mem::forget to prevent the Drop impl - // from sending a redundant close that could corrupt the shared - // SFTP session state for subsequent operations. + // shutdown() sets closed=true so Drop won't send a redundant close. remote_file .flush() .await @@ -783,7 +789,6 @@ impl AsyncConnection { .shutdown() .await .map_err(|e| format!("Remote file close error: {}", e))?; - std::mem::forget(remote_file); #[cfg(unix)] if preserve_permissions { use std::os::unix::fs::PermissionsExt; @@ -852,14 +857,13 @@ impl AsyncConnection { } } - // Stack for depth-first traversal: (remote_dir, local_dir) - let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( - std::path::PathBuf::from(&remote_path), - std::path::PathBuf::from(&local_path), - )]; + // Stack for depth-first traversal: (remote_dir_str, local_dir). + // Remote paths are kept as POSIX strings to avoid OS-native path separators on Windows. + let mut dirs_to_process: Vec<(String, std::path::PathBuf)> = + vec![(remote_path.clone(), std::path::PathBuf::from(&local_path))]; while let Some((remote_dir, local_dir)) = dirs_to_process.pop() { - let remote_dir_str = remote_dir.to_string_lossy().to_string(); + let remote_dir_str = remote_dir.clone(); let read_dir = match sftp.read_dir(&remote_dir_str).await { Ok(d) => d, Err(e) => { @@ -877,9 +881,9 @@ impl AsyncConnection { for entry in read_dir { let file_name = entry.file_name(); + // Build remote path with POSIX separator to stay cross-platform. + let remote_entry_str = format!("{}/{}", remote_dir, file_name); let local_entry = local_dir.join(&file_name); - let remote_entry = remote_dir.join(&file_name); - let remote_entry_str = remote_entry.to_string_lossy().to_string(); // When follow_symlinks=true, resolve symlinks on remote via metadata() let file_type = if follow_symlinks && entry.file_type().is_symlink() { @@ -925,7 +929,7 @@ impl AsyncConnection { .await; } } - dirs_to_process.push((remote_entry, local_entry)); + dirs_to_process.push((remote_entry_str, local_entry)); } else if file_type.is_file() { let transfer_result: Result<(), String> = async { let mut remote_file = sftp diff --git a/src/connection.rs b/src/connection.rs index 747dc3c..e72cd12 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -596,26 +596,37 @@ impl Connection { let mut transferred: Vec = Vec::new(); let mut failed: Vec = Vec::new(); - // Ensure remote base directory exists - if self.sftp().stat(Path::new(&remote_path)).is_err() { - match self.sftp().mkdir(Path::new(&remote_path), 0o755) { - Ok(_) => {} - Err(e) => { - let msg = format!("Failed to create remote directory '{}': {}", remote_path, e); - if fail_fast { - return Err(PyErr::new::(msg)); + // Ensure remote base directory (and all missing parents) exist — like mkdir -p + { + let parts: Vec<&str> = remote_path.split('/').filter(|s| !s.is_empty()).collect(); + let is_absolute = remote_path.starts_with('/'); + let mut current = if is_absolute { + String::from("/") + } else { + String::new() + }; + for part in &parts { + if !current.is_empty() && !current.ends_with('/') { + current.push('/'); + } + current.push_str(part); + if self.sftp().stat(Path::new(¤t)).is_err() { + if let Err(e) = self.sftp().mkdir(Path::new(¤t), 0o755) { + let msg = format!("Failed to create remote directory '{}': {}", current, e); + if fail_fast { + return Err(PyErr::new::(msg)); + } + failed.push(local_path.clone()); + return Ok((transferred, failed)); } - failed.push(remote_path); - return Ok((transferred, failed)); } } } - // Stack for depth-first traversal: (local_dir, remote_dir) - let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( - std::path::PathBuf::from(&local_path), - std::path::PathBuf::from(&remote_path), - )]; + // Stack for depth-first traversal: (local_dir, remote_dir_str). + // Remote paths are kept as POSIX strings to avoid OS-native path separators on Windows. + let mut dirs_to_process: Vec<(std::path::PathBuf, String)> = + vec![(std::path::PathBuf::from(&local_path), remote_path.clone())]; while let Some((local_dir, remote_dir)) = dirs_to_process.pop() { let entries = match std::fs::read_dir(&local_dir) { @@ -650,8 +661,8 @@ impl Connection { let local_entry = entry.path(); let local_entry_str = local_entry.to_string_lossy().to_string(); - let remote_entry = remote_dir.join(entry.file_name()); - let remote_entry_str = remote_entry.to_string_lossy().to_string(); + let file_name_str = entry.file_name().to_string_lossy().to_string(); + let remote_entry_str = format!("{}/{}", remote_dir, file_name_str); let metadata = match if follow_symlinks { std::fs::metadata(&local_entry) @@ -696,7 +707,7 @@ impl Connection { } } } - dirs_to_process.push((local_entry, remote_entry)); + dirs_to_process.push((local_entry, remote_entry_str)); } else if metadata.is_file() { let result: Result<(), String> = (|| { let mut local_file = std::fs::File::open(&local_entry) @@ -788,14 +799,13 @@ impl Connection { return Ok((transferred, failed)); } - // Stack for depth-first traversal: (remote_dir, local_dir) - let mut dirs_to_process: Vec<(std::path::PathBuf, std::path::PathBuf)> = vec![( - std::path::PathBuf::from(&remote_path), - std::path::PathBuf::from(&local_path), - )]; + // Stack for depth-first traversal: (remote_dir_str, local_dir). + // Remote paths are kept as POSIX strings to avoid OS-native path separators on Windows. + let mut dirs_to_process: Vec<(String, std::path::PathBuf)> = + vec![(remote_path.clone(), std::path::PathBuf::from(&local_path))]; while let Some((remote_dir, local_dir)) = dirs_to_process.pop() { - let remote_dir_str = remote_dir.to_string_lossy().to_string(); + let remote_dir_str = remote_dir.clone(); let entries = match self.sftp().readdir(Path::new(&remote_dir_str)) { Ok(e) => e, Err(e) => { @@ -821,9 +831,10 @@ impl Connection { continue; } }; + let file_name_str = file_name.to_string_lossy(); let local_entry = local_dir.join(&file_name); - let remote_entry = remote_dir.join(&entry_name); - let remote_entry_str = remote_entry.to_string_lossy().to_string(); + // Build remote path with POSIX separator to stay cross-platform. + let remote_entry_str = format!("{}/{}", remote_dir, file_name_str); // When follow_symlinks=true, resolve symlinks on remote via stat() let resolved_stat = if follow_symlinks && stat.file_type().is_symlink() { @@ -862,7 +873,7 @@ impl Connection { ); } } - dirs_to_process.push((remote_entry, local_entry)); + dirs_to_process.push((remote_entry_str, local_entry)); } else if resolved_stat.is_file() { let result: Result<(), String> = (|| { let mut remote_file = BufReader::new( diff --git a/tests/test_connection.py b/tests/test_connection.py index 0d5397b..586f754 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -277,9 +277,16 @@ def test_sftp_put_dir_fail_fast(conn, tmp_path): src.mkdir() (src / "ok.txt").write_text("ok") - # Try to upload to a path whose parent doesn't exist (invalid path) - with pytest.raises(OSError, match=r"(?i)(failed|error|no such)"): - conn.sftp_put_dir(str(src), "/nonexistent_parent/deep/path", fail_fast=True) + # Pre-create a *file* at the target path so that attempting to create a + # sub-directory inside it fails, even after mkdir-p creates its parents. + conn.sftp_write_data("blocking", "/root/put_dir_fail_target") + try: + with pytest.raises(OSError, match=r"(?i)(failed|error|no such)"): + conn.sftp_put_dir( + str(src), "/root/put_dir_fail_target/deep", fail_fast=True + ) + finally: + conn.execute("rm -f /root/put_dir_fail_target") def test_sftp_get_dir_fail_fast(conn, tmp_path): From d7fe358f24549f1d86569497eeeac0a2ac10d9d1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 01:16:17 +0000 Subject: [PATCH 12/12] fix: apply ruff format to test_connection.py Agent-Logs-Url: https://github.com/JacobCallahan/Hussh/sessions/c6354d34-4e5b-40aa-af4a-3ecb6be860bb Co-authored-by: JacobCallahan <6618303+JacobCallahan@users.noreply.github.com> --- tests/test_connection.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index 586f754..9cc503a 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -282,9 +282,7 @@ def test_sftp_put_dir_fail_fast(conn, tmp_path): conn.sftp_write_data("blocking", "/root/put_dir_fail_target") try: with pytest.raises(OSError, match=r"(?i)(failed|error|no such)"): - conn.sftp_put_dir( - str(src), "/root/put_dir_fail_target/deep", fail_fast=True - ) + conn.sftp_put_dir(str(src), "/root/put_dir_fail_target/deep", fail_fast=True) finally: conn.execute("rm -f /root/put_dir_fail_target")