perf: stream SFTP uploads/downloads instead of buffering whole file

[Yaminyam]

Summary

Upload (upload_file, upload_dir_recursive) used tokio::fs::read to load the entire local file into a Vec<u8> before calling write_all, and download (download_file, download_dir_recursive) used read_to_end into a pooled buffer + clone() to a separate Vec before writing locally. For multi-GB transfers this means peak RSS scales with file size and large files OOM the client.

Replace each path with a small stream_copy() helper that loops on 255 KiB reads and writes through the existing AsyncRead/AsyncWrite implementations on tokio::fs::File and russh_sftp::client::fs::File. The chunk size matches the russh-sftp default MAX_WRITE_LENGTH, so each chunk stays within a single SFTP read/write packet without the 255 KiB + 1 KiB split that a 256 KiB buffer would trigger.

Review follow-up commit ec04b839 tightened the implementation by aligning the chunk size to the actual 255 KiB packet limit, explicitly closing remote SFTP file handles after successful downloads, adding focused chunk-boundary unit coverage, and documenting the streaming transfer behavior in the architecture docs.

Measured impact

Verified locally on macOS arm64 against bssh-server v2.1.3 over loopback with a 1 GiB file:

Op	Build	real	RSS
upload	unpatched	38.65s	3.23 GB
upload	streaming	3.47s	20 MB
download	unpatched	3.93s	2.17 GB
download	streaming	3.41s	16 MB

Peak RSS drops ~160x and uploads complete ~11x faster (a single multi-MB write_all apparently serializes much worse through the SFTP pipeline than chunked writes).

Review result

• Correctness: The upload/download single-file and recursive directory paths now stream instead of buffering whole files, preserving existing error propagation and flush behavior while explicitly closing downloaded remote handles.
• Security: No authentication, host-key verification, command execution, credential handling, or logging behavior is changed. No new credential exposure, command injection, path expansion, or trust-boundary issue was found in the PR-scoped changes.
• Performance: The transfer payload memory is bounded to one 255 KiB buffer per active file copy, avoiding file-size-scaled RSS and avoiding accidental extra SFTP write fragmentation from the previous 256 KiB review version.

Test plan

• cargo check clean
• cargo build --release clean
• 1 GiB upload via patched client to bssh-server v2.1.3 loopback verified file integrity (md5 matches source)
• 1 GiB download verified
• cargo test --no-run
• cargo test ssh::tokio_client::file_transfer::tests::stream_copy_writes_sftp_sized_chunks
• cargo fmt --check
• cargo clippy -- -D warnings
• cargo test --lib --verbose
• cargo test --tests --verbose -- --skip integration_test
• GitHub CI on ec04b839

[inureyes]

Summary

Upload (upload_file, upload_dir_recursive) used tokio::fs::read to load the entire local file into a Vec<u8> before calling write_all, and download (download_file, download_dir_recursive) used read_to_end into a pooled buffer + clone() to a separate Vec before writing locally. For multi-GB transfers this means peak RSS scales with file size and large files OOM the client.

Replace each path with a small stream_copy() helper that loops on 255 KiB reads and writes through the existing AsyncRead/AsyncWrite implementations on tokio::fs::File and russh_sftp::client::fs::File. The chunk size matches the russh-sftp default MAX_WRITE_LENGTH, so each chunk stays within a single SFTP read/write packet without the 255 KiB + 1 KiB split that a 256 KiB buffer would trigger.

Review follow-up commit ec04b839 tightened the implementation by aligning the chunk size to the actual 255 KiB packet limit, explicitly closing remote SFTP file handles after successful downloads, adding focused chunk-boundary unit coverage, and documenting the streaming transfer behavior in the architecture docs.

Measured impact

Verified locally on macOS arm64 against bssh-server v2.1.3 over loopback with a 1 GiB file:

Op	Build	real	RSS
upload	unpatched	38.65s	3.23 GB
upload	streaming	3.47s	20 MB
download	unpatched	3.93s	2.17 GB
download	streaming	3.41s	16 MB

Peak RSS drops ~160x and uploads complete ~11x faster (a single multi-MB write_all apparently serializes much worse through the SFTP pipeline than chunked writes).

Review result

• Correctness: The upload/download single-file and recursive directory paths now stream instead of buffering whole files, preserving existing error propagation and flush behavior while explicitly closing downloaded remote handles.
• Security: No authentication, host-key verification, command execution, credential handling, or logging behavior is changed. No new credential exposure, command injection, path expansion, or trust-boundary issue was found in the PR-scoped changes.
• Performance: The transfer payload memory is bounded to one 255 KiB buffer per active file copy, avoiding file-size-scaled RSS and avoiding accidental extra SFTP write fragmentation from the previous 256 KiB review version.

Test plan

• cargo check clean
• cargo build --release clean
• 1 GiB upload via patched client to bssh-server v2.1.3 loopback verified file integrity (md5 matches source)
• 1 GiB download verified
• cargo test --no-run
• cargo test ssh::tokio_client::file_transfer::tests::stream_copy_writes_sftp_sized_chunks
• cargo fmt --check
• cargo clippy -- -D warnings
• cargo test --lib --verbose
• cargo test --tests --verbose -- --skip integration_test
• GitHub CI on ec04b839