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

Strip extraneous carriage return from end of entered password #2371

Merged
merged 1 commit into from
Mar 23, 2016

Conversation

hiddentao
Copy link
Contributor

This is a repeat of #2369 but against the develop branch.

The fix has been tested to work on OS X, Ubuntu and Windows.

@robotally
Copy link

Vote Count Reviewers
👍 2 @bas-vk @obscuren
👎 0

Updated: Wed Mar 23 17:06:39 UTC 2016

@codecov-io
Copy link

Current coverage is 51.15%

Merging #2371 into develop will increase coverage by +0.22% as of 990b198

Powered by Codecov. Updated on successful CI builds.

@bas-vk
Copy link
Member

bas-vk commented Mar 22, 2016

👍

@@ -92,6 +92,7 @@ func PromptPassword(prompt string, warnTerm bool) (string, error) {
}
fmt.Print(prompt)
input, err := bufio.NewReader(os.Stdin).ReadString('\n')
input = strings.TrimRight(input, "\r\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The \r is weird. Why is it required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this to be necessary for Windows.

@obscuren
Copy link
Contributor

👍

@obscuren
Copy link
Contributor

@hiddentao please fix your commit message like: <changed package>: description

cmd/utils: removed password line endings when not using liner.

Strip extraneous carriage return from end of entered password when not able to use liner

@hiddentao hiddentao force-pushed the fix_prompt_passwd_input branch from 90aa0a1 to 6f30034 Compare March 23, 2016 14:53
@hiddentao
Copy link
Contributor Author

@obscuren commit msg updated.

@obscuren
Copy link
Contributor

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants