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

Don't hang if no stdin or file provided #671

Merged
merged 1 commit into from
Nov 14, 2020

Conversation

arbovm
Copy link
Contributor

@arbovm arbovm commented Nov 12, 2020

If the yopass command is executed without providing --file or piping
data via stdin, the command would just hang.
This change checks if stdin can be read from via checking ModeCharDevice.

@codecov
Copy link

codecov bot commented Nov 12, 2020

Codecov Report

Merging #671 (844cf6b) into master (228b3b6) will increase coverage by 0.42%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #671      +/-   ##
==========================================
+ Coverage   78.38%   78.81%   +0.42%     
==========================================
  Files           6        6              
  Lines         347      354       +7     
==========================================
+ Hits          272      279       +7     
  Misses         49       49              
  Partials       26       26              
Impacted Files Coverage Δ
cmd/yopass/main.go 71.96% <93.75%> (+1.96%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b61b6a...844cf6b. Read the comment docs.

@arbovm arbovm force-pushed the detect-no-stdin-input branch 2 times, most recently from 5c590bf to 9311b13 Compare November 12, 2020 15:32
If the `yopass` command is executed without providing `--file` or piping
data via stdin, the command would just hang.
This change checks if stdin can be read from via checking `ModeCharDevice`.
@arbovm
Copy link
Contributor Author

arbovm commented Nov 12, 2020

As codecov is happy now, I'm happy.

@jhaals
Copy link
Owner

jhaals commented Nov 14, 2020

@arbovm I'm also very happy with this improvement! 🙌
Thanks a lot for the contribution!

@jhaals jhaals merged commit 0075c5f into jhaals:master Nov 14, 2020
@grobie
Copy link
Contributor

grobie commented Nov 19, 2020

FWIW, the POSIX default is to attach stdin to your TTY. Just typing yopass allowed the user to type the plaintext in their terminal. The standard termination EOF state is commonly signaled in terminals via Ctrl + d. Apparently that part of POSIX / shells is not that commonly known, but it's the standard behavior of many unix commands (.e.g. cat, tail, gpg, and so many more). The decsriptions of TestDetectsNoStdinInput and similar in this change are technically wrong: /dev/tty is the stdin input attached by default, so there is stdin input which might eventually end with EOF.

With this change, the common UNIX behavior can be restored by using cat | yopass though.

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.

3 participants