From fed96376e1ad255f5c8fd848bab2b48b6ed66cd2 Mon Sep 17 00:00:00 2001 From: Oskar Eichler Date: Mon, 31 Aug 2026 00:51:57 +0200 Subject: [PATCH 1/4] Leave caller-owned transfer streams open --- lib/net/sftp/operations/download.rb | 12 +++++++++--- lib/net/sftp/operations/upload.rb | 8 +++++--- test/test_download.rb | 3 ++- test/test_upload.rb | 7 ++++--- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/net/sftp/operations/download.rb b/lib/net/sftp/operations/download.rb index 54965ec..4aaefda 100644 --- a/lib/net/sftp/operations/download.rb +++ b/lib/net/sftp/operations/download.rb @@ -204,7 +204,7 @@ def []=(name, value) # A simple struct for encapsulating information about a single remote # file or directory that needs to be downloaded. - Entry = Struct.new(:remote, :local, :directory, :size, :handle, :offset, :sink) + Entry = Struct.new(:remote, :local, :directory, :size, :handle, :offset, :sink, :owned) #-- # "ruby -w" hates private attributes, so we have to do these longhand @@ -308,7 +308,13 @@ def on_open(response) raise StatusException.new(response, "open #{entry.remote}") unless response.ok? entry.handle = response[:handle] - entry.sink = entry.local.respond_to?(:write) ? entry.local : ::File.open(entry.local, "wb") + if entry.local.respond_to?(:write) + entry.sink = entry.local + entry.owned = false + else + entry.sink = ::File.open(entry.local, "wb") + entry.owned = true + end entry.offset = 0 download_next_chunk(entry) @@ -329,7 +335,7 @@ def on_read(response) if response.eof? update_progress(:close, entry) - entry.sink.close + entry.sink.close if entry.owned request = sftp.close(entry.handle, &method(:on_close)) request[:entry] = entry elsif !response.ok? diff --git a/lib/net/sftp/operations/upload.rb b/lib/net/sftp/operations/upload.rb index 1e98dbf..602be4b 100644 --- a/lib/net/sftp/operations/upload.rb +++ b/lib/net/sftp/operations/upload.rb @@ -227,7 +227,7 @@ def progress; @progress; end # A simple struct for recording metadata about the file currently being # uploaded. - LiveFile = Struct.new(:local, :remote, :io, :size, :handle) + LiveFile = Struct.new(:local, :remote, :io, :size, :handle, :owned) # The default # of bytes to read from disk at a time. DEFAULT_READ_SIZE = 32_000 @@ -283,9 +283,11 @@ def open_file(local, remote) if local.respond_to?(:read) file = local name = options[:name] || "" + owned = false else file = ::File.open(local, "rb") name = local + owned = true end if file.respond_to?(:stat) @@ -294,7 +296,7 @@ def open_file(local, remote) size = file.size end - metafile = LiveFile.new(name, remote, file, size) + metafile = LiveFile.new(name, remote, file, size, nil, owned) update_progress(:open, metafile) request = sftp.open(remote, "w", &method(:on_open)) @@ -363,7 +365,7 @@ def write_next_chunk(file) update_progress(:close, file) request = sftp.close(file.handle, &method(:on_close)) request[:file] = file - file.io.close + file.io.close if file.owned file.io = nil @uploads.delete(file) else diff --git a/test/test_download.rb b/test/test_download.rb index d9582b6..c0304c7 100644 --- a/test/test_download.rb +++ b/test/test_download.rb @@ -135,6 +135,7 @@ def test_download_file_should_transfer_remote_to_local_buffer assert_scripted_command { sftp.download(remote, local) } assert_equal text, local.string + assert !local.closed? end def test_download_directory_to_buffer_should_fail @@ -286,4 +287,4 @@ def prepare_directory_tree_download(local, remote) [file1, file2] end -end \ No newline at end of file +end diff --git a/test/test_upload.rb b/test/test_upload.rb index af52519..523e013 100644 --- a/test/test_upload.rb +++ b/test/test_upload.rb @@ -151,9 +151,10 @@ def test_upload_io_should_send_io_as_file channel.gets_packet(FXP_STATUS, :long, 2, :long, 0) end - assert_scripted_command do - sftp.upload(StringIO.new("this is some text"), "/path/to/remote") - end + source = StringIO.new("this is some text") + assert_scripted_command { sftp.upload(source, "/path/to/remote") } + + assert !source.closed? end private From af158a7e6e341c4951a8df16ce5aef79c5bf4fe2 Mon Sep 17 00:00:00 2001 From: Oskar Eichler Date: Mon, 31 Aug 2026 00:54:31 +0200 Subject: [PATCH 2/4] Support non-seekable upload streams --- lib/net/sftp/operations/upload.rb | 9 +++++---- test/test_upload.rb | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/net/sftp/operations/upload.rb b/lib/net/sftp/operations/upload.rb index 602be4b..6da8031 100644 --- a/lib/net/sftp/operations/upload.rb +++ b/lib/net/sftp/operations/upload.rb @@ -227,7 +227,7 @@ def progress; @progress; end # A simple struct for recording metadata about the file currently being # uploaded. - LiveFile = Struct.new(:local, :remote, :io, :size, :handle, :owned) + LiveFile = Struct.new(:local, :remote, :io, :size, :handle, :owned, :offset) # The default # of bytes to read from disk at a time. DEFAULT_READ_SIZE = 32_000 @@ -292,11 +292,11 @@ def open_file(local, remote) if file.respond_to?(:stat) size = file.stat.size - else + elsif file.respond_to?(:size) size = file.size end - metafile = LiveFile.new(name, remote, file, size, nil, owned) + metafile = LiveFile.new(name, remote, file, size, nil, owned, 0) update_progress(:open, metafile) request = sftp.open(remote, "w", &method(:on_open)) @@ -359,7 +359,7 @@ def write_next_chunk(file) process_next_entry else @active += 1 - offset = file.io.pos + offset = file.offset data = file.io.read(options[:read_size] || DEFAULT_READ_SIZE) if data.nil? update_progress(:close, file) @@ -369,6 +369,7 @@ def write_next_chunk(file) file.io = nil @uploads.delete(file) else + file.offset += data.bytesize update_progress(:put, file, offset, data) request = sftp.write(file.handle, offset, data, &method(:on_write)) request[:file] = file diff --git a/test/test_upload.rb b/test/test_upload.rb index 523e013..6898dd3 100644 --- a/test/test_upload.rb +++ b/test/test_upload.rb @@ -157,6 +157,15 @@ def test_upload_io_should_send_io_as_file assert !source.closed? end + def test_upload_should_accept_a_read_only_stream + expect_file_transfer_from_stream("/path/to/remote", "this is some text") + chunks = ["this is some text", nil] + source = Object.new + source.define_singleton_method(:read) { |_size| chunks.shift } + + assert_scripted_command { sftp.upload(source, "/path/to/remote") } + end + private def prepare_directory @@ -231,4 +240,15 @@ def expect_file_transfer(local, remote, data) expect_file(local, data) end + + def expect_file_transfer_from_stream(remote, data) + expect_sftp_session :server_version => 3 do |channel| + channel.sends_packet(FXP_OPEN, :long, 0, :string, remote, :long, 0x1A, :long, 0) + channel.gets_packet(FXP_HANDLE, :long, 0, :string, "handle") + channel.sends_packet(FXP_WRITE, :long, 1, :string, "handle", :int64, 0, :string, data) + channel.sends_packet(FXP_CLOSE, :long, 2, :string, "handle") + channel.gets_packet(FXP_STATUS, :long, 1, :long, 0) + channel.gets_packet(FXP_STATUS, :long, 2, :long, 0) + end + end end From 14f0f2ba3c1e048c24862205b15d296729b9a3ac Mon Sep 17 00:00:00 2001 From: Oskar Eichler Date: Mon, 31 Aug 2026 00:55:08 +0200 Subject: [PATCH 3/4] Stop downloads on empty data packets --- lib/net/sftp/operations/download.rb | 2 +- test/test_download.rb | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/net/sftp/operations/download.rb b/lib/net/sftp/operations/download.rb index 4aaefda..cc1a18a 100644 --- a/lib/net/sftp/operations/download.rb +++ b/lib/net/sftp/operations/download.rb @@ -333,7 +333,7 @@ def download_next_chunk(entry) def on_read(response) entry = response.request[:entry] - if response.eof? + if response.eof? || (response.ok? && response[:data].empty?) update_progress(:close, entry) entry.sink.close if entry.owned request = sftp.close(entry.handle, &method(:on_close)) diff --git a/test/test_download.rb b/test/test_download.rb index c0304c7..d3176c4 100644 --- a/test/test_download.rb +++ b/test/test_download.rb @@ -138,6 +138,21 @@ def test_download_file_should_transfer_remote_to_local_buffer assert !local.closed? end + def test_download_should_treat_empty_data_as_end_of_file + expect_sftp_session :server_version => 3 do |channel| + channel.sends_packet(FXP_OPEN, :long, 0, :string, "/path/to/remote", :long, 0x01, :long, 0) + channel.gets_packet(FXP_HANDLE, :long, 0, :string, "handle") + channel.sends_packet(FXP_READ, :long, 1, :string, "handle", :int64, 0, :long, 32_000) + channel.gets_packet(FXP_DATA, :long, 1, :string, "") + channel.sends_packet(FXP_CLOSE, :long, 2, :string, "handle") + channel.gets_packet(FXP_STATUS, :long, 2, :long, 0) + end + local = StringIO.new + + assert_scripted_command { sftp.download("/path/to/remote", local) } + assert_equal "", local.string + end + def test_download_directory_to_buffer_should_fail expect_sftp_session :server_version => 3 Net::SSH::Test::Extensions::IO.with_test_extension do From 00fe7ca527953f08e8be65f4d4ba91b9e76774ea Mon Sep 17 00:00:00 2001 From: Oskar Eichler Date: Mon, 31 Aug 2026 16:53:23 +0700 Subject: [PATCH 4/4] Treat empty upload chunks as EOF --- lib/net/sftp/operations/upload.rb | 2 +- test/test_upload.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/net/sftp/operations/upload.rb b/lib/net/sftp/operations/upload.rb index 6da8031..8dce920 100644 --- a/lib/net/sftp/operations/upload.rb +++ b/lib/net/sftp/operations/upload.rb @@ -361,7 +361,7 @@ def write_next_chunk(file) @active += 1 offset = file.offset data = file.io.read(options[:read_size] || DEFAULT_READ_SIZE) - if data.nil? + if data.nil? || data.empty? update_progress(:close, file) request = sftp.close(file.handle, &method(:on_close)) request[:file] = file diff --git a/test/test_upload.rb b/test/test_upload.rb index 6898dd3..8fe2b9e 100644 --- a/test/test_upload.rb +++ b/test/test_upload.rb @@ -166,6 +166,19 @@ def test_upload_should_accept_a_read_only_stream assert_scripted_command { sftp.upload(source, "/path/to/remote") } end + def test_upload_should_treat_an_empty_chunk_as_end_of_file + expect_sftp_session :server_version => 3 do |channel| + channel.sends_packet(FXP_OPEN, :long, 0, :string, "/path/to/remote", :long, 0x1A, :long, 0) + channel.gets_packet(FXP_HANDLE, :long, 0, :string, "handle") + channel.sends_packet(FXP_CLOSE, :long, 1, :string, "handle") + channel.gets_packet(FXP_STATUS, :long, 1, :long, 0) + end + source = Object.new + source.define_singleton_method(:read) { |_size| "" } + + assert_scripted_command { sftp.upload(source, "/path/to/remote") } + end + private def prepare_directory