-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle remote server closes channel without exit-status #71
Handle remote server closes channel without exit-status #71
Conversation
Some ssh-server (APC Network Management Card AP9640) close the channel without sending an eof. This is correctly handled by openssh, so NET:SCP should also handle it. Added a test for this case. The changed/added code passes Rubocop with default settings, with the exception of the length of the test name, wich ich intended to be long.
lib/net/scp.rb
Outdated
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 channel[:exit].nil? && channel[:state] == :finish | ||
# The remote closed the channel without sending an eof, but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused, but is it eof or exit-status?
https://www.ietf.org/rfc/rfc4254.txt
6.10. Returning Exit Status
When the command running at the other end terminates, the following
message can be sent to return the exit status of the command.
Returning the status is RECOMMENDED. No acknowledgement is sent for
this message. The channel needs to be closed with
SSH_MSG_CHANNEL_CLOSE after this message.The client MAY ignore these messages.
byte SSH_MSG_CHANNEL_REQUEST uint32 recipient channel string "exit-status" boolean FALSE uint32 exit_status
The remote command may also terminate violently due to a signal.
Such a condition can be indicated by the following message. A zero
'exit_status' usually means that the command terminated successfully.byte SSH_MSG_CHANNEL_REQUEST uint32 recipient channel string "exit-signal" boolean FALSE string signal name (without the "SIG" prefix) boolean core dumped string error message in ISO-10646 UTF-8 encoding string language tag [RFC3066]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are absolutely right. Not receiving SSH_MSG_CHANNEL_EOF is not the problem, I messed things up.
To clarify:
The remote implementation closes the channel without sending an eof nor an exit_status.
NET::SCP currently needs an exit_status = 0 to successfully end the transmission, which causes my problem.
As RFC4254 specifies sending exit_status als RECOMMENDED, NET::SCP should cope with not getting one.
I will correct my code and submit again.
And new test - download interrupted
The code of the test does not pass rubocop tests, I will fix this. |
This has been done with last commit, and this PR can be reviewed now. I will be happy to make any changes if needed. |
Thank you very much for approving this PR. Can someone merge it into master? |
Some ssh-server (APC Network Management Card AP9640) close the channel without sending an exit-status. This is correctly handled by openssh, so NET:SCP should also handle it.
I added two test cases: successful download without exit-status and interrupted download.
It would be great if you could review (and hopefully accept this PR), as it will help me to resove an issue on oxidized (ytti/oxidized#1802)