conradkleinespel / rpassword

Cross platform Rust library to read a password in the terminal (Linux, BSD, OSX, Windows, WASM).
Apache License 2.0
244 stars 38 forks source link

Permit lack of newline, given possible piping or EOF. #29

Closed longshorej closed 5 years ago

longshorej commented 5 years ago

Modifies the logic to no longer require a newline at the end of input. What I'm not clear on is why that was required to begin with, so please let me know if this is an unwanted change.

Before this change, echo -n password | yourprogram would result in an error.

Additionally, when a TTY is allocated, an error could be triggered by the user entering their password followed by CTRL-D twice.

Both of these scenarios are now handled by simply removing the requirement to end in a newline.

conradkleinespel commented 5 years ago

Hi @longshorej, thanks for your input. Does something prevent you from simply using echo password | program ? If so, what is it ?

Hitting Ctrl-D usually implies that you don't want the typed line to be used as input. That's why rpassword requires the newline character be present.

longshorej commented 5 years ago

I can work around it by explicitly adding a newline, but I don't think that is typical behavior.

Hitting Ctrl-D usually implies that you don't want the typed line to be used as input.

I'm not sure about this - I've just tested with ssh and linux VT login. CTRL-D for both results in my typed password being accepted as it simply signifies EOF. In general, GNU readline will return the input it has read when reaching EOF. It seems to me that rpassword's current behavior in this regard is not conventional.

longshorej commented 5 years ago

Hi @conradkdotcom -- any thoughts about my latest comment?

conradkleinespel commented 5 years ago

@longshorej Sorry about the delay. I get what you're saying. Let's do that then ! I'll merge this PR now.

Since changing the EOF behavior is a breaking change, I can only publish as a new major version. Would you be OK to use the crate from Git for a few days until I remove obsolete code and publish a major version upgrade ?

conradkleinespel commented 5 years ago

(woops, closed by mistake, so I re-opened to merge)

longshorej commented 5 years ago

Of course - thanks for accommodating @conradkdotcom!

conradkleinespel commented 5 years ago

Alright, great ! I'll notify you once it's done. Cheers ! Thanks a lot for your PR :+1:

longshorej commented 5 years ago

Hi @conradkdotcom - may I bother you for a Crates.io release when you have a moment?

conradkleinespel commented 5 years ago

@longshorej Will do within a few days, sorry for the wait

conradkleinespel commented 5 years ago

@longshorej Just updated the crate on crates.io, with version 3.0.0. Here's the release note: https://github.com/conradkdotcom/rpassword/releases/tag/v3.0.0

Thanks again for your PR !

longshorej commented 5 years ago

Great, much appreciated! Thanks @conradkdotcom.