Skip to content

Commit 044d834

Browse files
author
Anthony Accardi
committed
Fixes Session#when_channel_polled to properly account for 4 byte length
header, so that it waits for the complete packet to arrive instead of processing an incomplete packet and loosing sync with server. A new test method, gets_packet_in_two, enables testing of fragmentation edge cases. expect_file_transfer now takes a :fragment_len parameter, allowing for DRY testing of common fragmentation cases like this one.
1 parent 491ddfd commit 044d834

File tree

3 files changed

+30
-4
lines changed

3 files changed

+30
-4
lines changed

lib/net/sftp/session.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,7 @@ def when_channel_polled(channel)
898898
@packet_length = input.read_long
899899
end
900900

901-
return unless input.length >= @packet_length
901+
return unless input.length >= @packet_length + 4
902902
packet = Net::SFTP::Packet.new(input.read(@packet_length))
903903
input.consume!
904904
@packet_length = nil

test/common.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,18 @@ def gets_packet(type, *args)
125125
gets_data(sftp_packet(type, *args))
126126
end
127127

128+
def gets_packet_in_two(fragment_len, type, *args)
129+
fragment_len ||= 0
130+
whole_packet = sftp_packet(type, *args)
131+
132+
if 0 < fragment_len && fragment_len < whole_packet.length
133+
gets_data(whole_packet[0, whole_packet.length - fragment_len])
134+
gets_data(whole_packet[-fragment_len..-1])
135+
else
136+
gets_data(whole_packet)
137+
end
138+
end
139+
128140
def sends_packet(type, *args)
129141
sends_data(sftp_packet(type, *args))
130142
end

test/test_download.rb

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,20 @@ def test_download_file_should_transfer_remote_to_local
2121
assert_equal text, file.string
2222
end
2323

24+
def test_download_file_should_transfer_remote_to_local_in_spite_of_fragmentation
25+
local = "/path/to/local"
26+
remote = "/path/to/remote"
27+
text = "this is some text\n"
28+
29+
expect_file_transfer(remote, text, :fragment_len => 1)
30+
31+
file = StringIO.new
32+
File.stubs(:open).with(local, "wb").returns(file)
33+
34+
assert_scripted_command { sftp.download(remote, local) }
35+
assert_equal text, file.string
36+
end
37+
2438
def test_download_large_file_should_transfer_remote_to_local
2539
local = "/path/to/local"
2640
remote = "/path/to/remote"
@@ -130,12 +144,12 @@ def test_download_directory_to_buffer_should_fail
130144

131145
private
132146

133-
def expect_file_transfer(remote, text)
147+
def expect_file_transfer(remote, text, opts={})
134148
expect_sftp_session :server_version => 3 do |channel|
135149
channel.sends_packet(FXP_OPEN, :long, 0, :string, remote, :long, 0x01, :long, 0)
136150
channel.gets_packet(FXP_HANDLE, :long, 0, :string, "handle")
137151
channel.sends_packet(FXP_READ, :long, 1, :string, "handle", :int64, 0, :long, 32_000)
138-
channel.gets_packet(FXP_DATA, :long, 1, :string, text)
152+
channel.gets_packet_in_two(opts[:fragment_len], FXP_DATA, :long, 1, :string, text)
139153
channel.sends_packet(FXP_READ, :long, 2, :string, "handle", :int64, text.bytesize, :long, 32_000)
140154
channel.gets_packet(FXP_STATUS, :long, 2, :long, 1)
141155
channel.sends_packet(FXP_CLOSE, :long, 3, :string, "handle")
@@ -160,7 +174,7 @@ def prepare_large_file_download(local, remote, text, requested_chunk_size = FXP_
160174
channel.sends_packet(FXP_CLOSE, :long, data_packet_count + 2, :string, "handle")
161175
channel.gets_packet(FXP_STATUS, :long, data_packet_count + 2, :long, 0)
162176
end
163-
177+
164178
file = StringIO.new
165179
File.stubs(:open).with(local, "wb").returns(file)
166180

0 commit comments

Comments
 (0)