bgentry / speakeasy

cross-platform Golang helpers for reading password input without cgo
Other
115 stars 24 forks source link

Allow passwords with spaces #4

Closed razzius closed 10 years ago

razzius commented 10 years ago

Passwords with spaces currently cause an error:

> go run example/main.go  <I type a password with spaces>
Please enter a password: failed during password entry: expected newline
exit status 1

This commit uses a bufio.NewReader to read an entire line of user input.

kr commented 10 years ago

A buffered reader might read past the end of the line into its buffer.

I'd wager safest and easiest to read one byte at a time, something like:

var pw []byte
b := make([]byte, 1)
for {
  n, err := f.Read(b)
  ...
  if b[0] == '\n' {
    break
  }
  pw = append(pw, b[0])
}
bgentry commented 10 years ago

Ugh, I hadn't set this repo to "watch" so I just saw this now as it was cross-referenced from heroku/hk#147.

I think when I wrote this, for some reason I thought fmt.Scanln used bufio.ScanLines under the hood. Obviously that's not the case and I didn't read the description very carefully :)

@kr should we be using bufio.Scanner with bufio.ScanLines instead? AFAIK we need to handle \r\n line endings as well, and the description of ScanLines looks like what we want: http://golang.org/pkg/bufio/#ScanLines

Would this do the trick inside readline()?

s := bufio.NewScanner()
s.Scan()
return s.Text(), s.Err()
bgentry commented 10 years ago

I just fixed this in 4646fcf, which seems to work well in testing. Thanks @razzius for reporting this and offering a fix!

Sorry it took me so long to notice this one.

kr commented 10 years ago

We should not be using a bufio Scanner, which is why I didn't suggest it above. Scanner has the same problem as bufio.Reader. It's buffered, so it can consume from the underlying reader more than what you asked for – more than the current line. From the Scanner docs:

When a scan stops, the reader may have advanced arbitrarily far past the last token.

It means if you want to use the file after you're done reading the current line, you'll lose any bytes that are sitting in the buffer. This is just how buffered I/O works in general.

bgentry commented 10 years ago

ok, makes sense. Then we'd need to use the example you've shown above, but modified to handle either \r\n or just \n

kr commented 10 years ago

Yeah, something like

return strings.TrimSuffix(string(pw), "\r")