handle nil returns on gets

[soma]

This is a simple non-tested fix for an issue we had with this lib. Kernel#gets can return nil in some cases, and this should be handled in some way. You might know a way to do this tested and more dry, but I did not manage to be able to run the tests.

[mfazekas]

Thanks,
Can you describe in what conditions/usecase did you face the error? From the docsgets retuns nil at EOF, so stdin was a file rather than a console?
We could probably do it in a one liner like ($stdin.gets || "").chomp

[henrik]

(I work with @soma.) We've gotten undefined methodchomp' for nil:NilClass` a few times from some code that does networking stuff, presumably because of intermittent network issues.

A oneliner should be fine – assuming silently swallowing the error is the right thing to do. Maybe raising some exception would be better still? It would certainly be nicer from our perspective to rescue some explicit connection error than a NoMethodError. Though we could also deal with a string if that's good enough.

[mfazekas]

I'm still not clear on your usecase and how you get this error. Or is this just a general bug prevention?

Here we read from stdin, so it should not be related to network.

[henrik]

Hm, looking closer at the error. It comes from some code that does Net::SFTP.start(HOSTNAME, user, password: password).

The backtrace is

[GEM_ROOT]/gems/net-ssh-2.9.2/lib/net/ssh/prompt.rb:73 :in `prompt`
[GEM_ROOT]/gems/net-ssh-2.9.2/lib/net/ssh/authentication/methods/password.rb:57 :in `ask_password`
[GEM_ROOT]/gems/net-ssh-2.9.2/lib/net/ssh/authentication/methods/password.rb:22 :in `authenticate`
[GEM_ROOT]/gems/net-ssh-2.9.2/lib/net/ssh/authentication/session.rb:79 :in `block in authenticate`
[GEM_ROOT]/gems/net-ssh-2.9.2/lib/net/ssh/authentication/session.rb:66 :in `each`
[GEM_ROOT]/gems/net-ssh-2.9.2/lib/net/ssh/authentication/session.rb:66 :in `authenticate`
[GEM_ROOT]/gems/net-ssh-2.9.2/lib/net/ssh.rb:211 :in `start`
[GEM_ROOT]/gems/net-sftp-2.1.2/lib/net/sftp.rb:31 :in `start`
[PROJECT_ROOT]/lib/pay_pal_report_sftp_client.rb:47 :in `sftp_connection`

I'm not super familiar with this part of our code (@soma is, I think) but I think that this only happens intermittently, so presumably some connection error makes auth fail and then it tries to prompt, or something like that – I don't believe it's due to a consistently missing password on our side since the error is only intermittent. But I'd like @soma to confirm that.

[mfazekas]

There was a change in 2.9.2 (which should went only to 2.10) that changed behaviour of :password authntication to ask for password with prompt if it was not provided correctly. It should not be an issue for interactive apps but for non interactive you need to set :number_of_password_prompts to 0 to get back what 2.9.1 worked.

[carlhoerberg]

Yeah, not super happy about that change :( Breaks things in a weird ways, hanging threads instead of throwing exceptions for instance. Maybe you should default it to 0 instead and let interactive apps set it manually? Interactive usage will probably notice the error better than non interactive apps that just suddenly hangs and causes other strange behaviours in other parts of the system.

On Tuesday 24 March 2015 at 00:01, Miklós Fazekas wrote:

There was a change in 2.9.2 (which should went only to 2.10) that changed behaviour of :password authntication to ask for password with prompt if it was not provided correctly. It should not be an issue for interactive apps but for non interactive you need to set :number_of_password_prompts to 0 to get back what 2.9.1 worked.

—
Reply to this email directly or view it on GitHub (#234 (comment)).

[mfazekas]

There was a plan to revert back 2.9.3 to default behaviour with a warning to ineractive users. But 2.10 would ask for a password. But i'm a bit worried it would confuse stuff even more and i'm also lacking time.

I'd assume that non interactive apps would use public key auth rather than password. My understading is the password is really for interactive usecase and if you have special scenario requiring use of password in non interactive env then it's reasonable to expect you to do some extra work.

[carlhoerberg]

We always connect with public key, but the problem that happened to us was that the IP for a server changed but not the DNS so the DNS was pointing to a server that wasn't ours and that server accepted passwords. Now our app hanged on the password prompt, causing hard to debug errors.. We did debug it though and can live with it now, but others may still encounter similar problems..

On Tuesday 24 March 2015 at 21:03, Miklós Fazekas wrote:

There was a plan to revert back 2.9.3 to default behaviour with a warning to ineractive users. But 2.10 would ask for a password. But i'm a bit worried it would confuse stuff even more and i'm also lacking time.
I'd assume that non interactive apps would use public key auth rather than password. My understading is the password is really for interactive usecase and if you have special scenario requiring use of password in non interactive env then it's reasonable to expect you to do some extra work.

—
Reply to this email directly or view it on GitHub (#234 (comment)).

[os6sense]

@mfazekas

"There was a change in 2.9.2 (which should went only to 2.10) that changed behaviour of :password authntication to ask for password with prompt if it was not provided correctly."

Can you expand on what you mean by "not provided correctly"?

I've a strange issue where I have an app failing to authenticate when falling back to username and password for login. I've tried changing the number_of_password_prompts to 0 but I'm still getting the chomp error.

[mfazekas]

@os6sense can you give more details about your usecase? So before 2.9.2 if you provided an incorrect or empty password for password auth it did not prompt the user. After 2.9.2 if it fails password auth with given password it tries to ask for password, up to number_of_password_prompts times. So if number_of_password_prompts you should see pre 2.9.2 behaviour.

[os6sense]

@mfazekas

" So before 2.9.2 if you provided an incorrect or empty password ",

ahhh, that's what I needed to know - thank you.

can you give more details about your usecase?

Fixed. This is probably user (my) error, but I'll describe it in the event anyone else lands here with a search for the 'undefined method chomp' error.

I've a simple wrapper around Net:SFTP for uploading. Due to some cruddy configuration on the staging server I had a world readable passphrase protected ssh key for a user with the same username that SFTP was attempting to log in as; hence even though a username and password were supplied, the passphrase was being prompted for. Given there is no user input for the passphrase, it was this that was failing with the chomp error. e.g.

E, [2015-05-13T19:39:34.939446 #14948] ERROR -- : undefined method 'chomp' for nil:NilClass (NoMethodError)
/usr/local/lib/ruby/gems/2.2.0/gems/net-ssh-2.9.2/lib/net/ssh/prompt.rb:73:in 'prompt'
/usr/local/lib/ruby/gems/2.2.0/gems/net-ssh-2.9.2/lib/net/ssh/key_factory.rb:85:in 'rescue in load_data_private_key'
/usr/local/lib/ruby/gems/2.2.0/gems/net-ssh-2.9.2/lib/net/ssh/key_factory.rb:75:in 'load_data_private_key'
/usr/local/lib/ruby/gems/2.2.0/gems/net-ssh-2.9.2/lib/net/ssh/key_factory.rb:42:in 'load_private_key'
/usr/local/lib/ruby/gems/2.2.0/gems/net-ssh-2.9.2/lib/net/ssh/authentication/key_manager.rb:142:in 'sign'
/usr/local/lib/ruby/gems/2.2.0/gems/net-ssh-2.9.2/lib/net/ssh/authentication/methods/publickey.rb:62:in 'authenticate_with'

If I recall correctly the default behaviour of ssh is to attempt to use public key authentication first so I'd hazard to call this a bug, but it is another place where the prompt/chomp can cause problems.

I'm wondering if this patch had been applied, would net-ssh have silently switched to using the supplied username and password?

[mfazekas]

Ok so your stacktrace is net-ssh asking for password for public key auth. And in that case number_of_password_prompts should not have any effect.

I'm wondering why gets returned nil in your case? Do you have stdin redirected?

With this patch the pubkey mehtod would just fail. Then probably it would try with user/password which would succeed

[os6sense]

I'm wondering why gets returned nil in your case?

The application using this is running under Passenger hence I'm not quite sure what happens with stdin.

With this patch the pubkey mehtod would just fail. Then probably it would try with user/password which would succeed

That's what I thought.

Obviously the problem the patch addresses affects multiple areas and I'm not sure that glossing over the cracks is the best idea given that without the chomp exception there would have been an "invisible" bug that would have probably been far harder to diagnose than this was.

It would however be far preferable to receive an exception with a higher specificity than that currently raised.

[mfazekas]

@carlhoerberg I've now release 2.10.0 beta1 it adds a new non_interactive option that is expected to be set by non_interactive applications, to prevent putting up prompts from various places.