Merge pull request #71 from robertcheramy/fix_remote_closes_without_eof

Handle remote server closes channel without exit-status

Author: mfazekas

lib/net/scp.rb

@@ -362,7 +362,21 @@ def start_command(mode, local, remote, options={}, &callback)
               channel[:stack   ] = []
               channel[:error_string] = ''
 
-              channel.on_close                  { |ch2| send("#{channel[:state]}_state", channel); raise Net::SCP::Error, "SCP did not finish successfully (#{channel[:exit]}): #{channel[:error_string]}" if channel[:exit] != 0 }
+              channel.on_close do
+                # If we got an exit-status and it is not 0, something went wrong
+                if !channel[:exit].nil? && channel[:exit] != 0
+                  raise Net::SCP::Error, 'SCP did not finish successfully ' \
+                                         "(#{channel[:exit]}): #{channel[:error_string]}"
+                end
+                # We may get no exit-status at all as returning a status is only RECOMENDED
+                # in RFC4254. But if our state is not :finish, something went wrong
+                if channel[:exit].nil? && channel[:state] != :finish
+                  raise Net::SCP::Error, 'SCP did not finish successfully ' \
+                                         '(channel closed before end of transmission)'
+                end
+                # At this point, :state can be :finish or :next_item
+                send("#{channel[:state]}_state", channel)
+              end
               channel.on_data                   { |ch2, data| channel[:buffer].append(data) }
               channel.on_extended_data          { |ch2, type, data| debug { data.chomp } }
               channel.on_request("exit-status") { |ch2, data| channel[:exit] = data.read_long }

test/test_download.rb

@@ -225,6 +225,52 @@ def test_download_directory_should_create_directory_and_files_locally
     assert_equal "a" * 1234, file.io.string
   end
 
+  def test_download_should_work_when_remote_closes_channel_without_exit_status
+    file = prepare_file('/path/to/local.txt', 'a' * 1234)
+
+    story do |session|
+      channel = session.opens_channel
+      channel.sends_exec 'scp -f /path/to/remote.txt'
+      simple_download(channel)
+      # Remote closes without sending an exit-status
+      channel.gets_close
+      # We just send eof and close the channel
+      channel.sends_eof
+      channel.sends_close
+    end
+
+    assert_scripted { scp.download!('/path/to/remote.txt', '/path/to/local.txt') }
+    assert_equal 'a' * 1234, file.io.string
+  end
+
+  def test_download_should_raise_error_when_remote_closes_channel_before_end
+    prepare_file('/path/to/local.txt', 'a' * 1234)
+
+    story do |session|
+      channel = session.opens_channel
+      channel.sends_exec 'scp -f /path/to/remote.txt'
+      channel.sends_ok
+      channel.gets_data "C0666 1234 remote.txt\n"
+      channel.sends_ok
+      channel.gets_data 'a' * 500
+      # We should have received 1234 bytes and \0 but remote closed before the end
+      channel.gets_close
+      # We just send eof and close the channel
+      channel.sends_eof
+      channel.sends_close
+    end
+
+    error = nil
+    begin
+      assert_scripted { scp.download!('/path/to/remote.txt', '/path/to/local.txt') }
+    rescue => e
+      error = e
+    end
+
+    assert_equal Net::SCP::Error, error.class
+    assert_equal 'SCP did not finish successfully (channel closed before end of transmission)', error.message
+  end
+
   private
 
     def simple_download(channel, mode=0666)