Skip to content
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

wait for new packets on the agent socket after the handler is invoked… #267

Merged
merged 5 commits into from
Jan 31, 2022

Conversation

pfy
Copy link
Contributor

@pfy pfy commented Nov 18, 2021

wait for new data on the socket after the handler is called.
ruby net::ssh keeps the agent connection open, but the secretive agent closes the connection :(
this fixes #244

@maxgoedjen
Copy link
Owner

Hey @pfy, thanks for the fix! This looks correct to me on its face but lemme test it a bit to make sure it doesn't break any other cases.

@maxgoedjen
Copy link
Owner

Also, any chance you have a test script for this you could share?

@pfy
Copy link
Contributor Author

pfy commented Nov 20, 2021

Hi @maxgoedjen,
there is a test script in the bug report (in ruby)

require 'net/ssh'
Net::SSH.start('github.com','git',:verbose => :debug) do |ssh|
output = ssh.exec!("hostname")
puts output
end

This fails without the mr and works with the MR.

@maxgoedjen
Copy link
Owner

Fantastic thanks 🙏

@pfy
Copy link
Contributor Author

pfy commented Nov 30, 2021

@maxgoedjen how can i help you to get this pr merged ? what is still missing ?

@maxgoedjen
Copy link
Owner

@pfy sorry nothing, just slipped my mind. Giving it a test now.

@maxgoedjen
Copy link
Owner

@pfy tested it out with a simple SSH command and it looks as though it may cause an infinite loop. Can you take a look?

Screen.Recording.2021-11-30.at.9.06.43.PM.mov

@maxgoedjen maxgoedjen self-requested a review December 1, 2021 05:08
@maxgoedjen maxgoedjen self-assigned this Dec 1, 2021
@maxgoedjen
Copy link
Owner

I think this is working: #270
Give it a go and let me know if it works for you.

@pfy
Copy link
Contributor Author

pfy commented Jan 3, 2022

@maxgoedjen this PR works for me. How can I help you to get it merged ?

@maxgoedjen maxgoedjen enabled auto-merge (squash) January 30, 2022 02:17
@maxgoedjen
Copy link
Owner

maxgoedjen commented Jan 30, 2022

@pfy sorry this kinda fell off my radar post-vacation. It's looking good in my testing, I'll create a test build that I'll run for a while (would appreciate if you could test it out for a while until it's released as well). Thanks again for fixing this :)

@maxgoedjen
Copy link
Owner

@pfy I think you'll need to update this with changes from the base, but should be all good to go then.

@pfy
Copy link
Contributor Author

pfy commented Jan 31, 2022

ok, i will do that, thanks @maxgoedjen

@maxgoedjen maxgoedjen merged commit 03d3cc9 into maxgoedjen:main Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

secretive does not work as agent for ruby net::ssh
2 participants