Skip to content

Commit

Permalink
Address review comments (2)
Browse files Browse the repository at this point in the history
  • Loading branch information
codingllama committed Mar 25, 2022
1 parent e37cf8c commit 0ce2a5c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
29 changes: 15 additions & 14 deletions lib/utils/prompt/context_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,29 +52,30 @@ const (
readerStateClosed
)

// xTermI aggregates methods from /x/term for easy mocking.
type xTermI interface {
// termI aggregates methods from golang.org/x/term for easy mocking.
type termI interface {
GetState(fd int) (*term.State, error)
IsTerminal(fd int) bool
ReadPassword(fd int) ([]byte, error)
Restore(fd int, oldState *term.State) error
}

type xTerm struct{}
// gxTerm delegates method calls to golang.org/x/term methods.
type gxTerm struct{}

func (xTerm) GetState(fd int) (*term.State, error) {
func (gxTerm) GetState(fd int) (*term.State, error) {
return term.GetState(fd)
}

func (xTerm) IsTerminal(fd int) bool {
func (gxTerm) IsTerminal(fd int) bool {
return term.IsTerminal(fd)
}

func (xTerm) ReadPassword(fd int) ([]byte, error) {
func (gxTerm) ReadPassword(fd int) ([]byte, error) {
return term.ReadPassword(fd)
}

func (xTerm) Restore(fd int, oldState *term.State) error {
func (gxTerm) Restore(fd int, oldState *term.State) error {
return term.Restore(fd, oldState)
}

Expand All @@ -84,7 +85,7 @@ func (xTerm) Restore(fd int, oldState *term.State) error {
// ContextReader instances are not safe for concurrent use, callers may block
// indefinitely and reads may be lost.
type ContextReader struct {
xTerm xTermI
term termI

// reader is used for clean reads.
reader io.Reader
Expand All @@ -108,20 +109,20 @@ type ContextReader struct {
// Calling ContextReader.Close attempts to release resources, but note that
// ongoing reads cannot be interrupted.
func NewContextReader(rd io.Reader) *ContextReader {
xt := xTerm{}
term := gxTerm{}

fd := -1
if f, ok := rd.(*os.File); ok {
val := int(f.Fd())
if xt.IsTerminal(val) {
if term.IsTerminal(val) {
fd = val
}
}

mu := &sync.Mutex{}
cond := sync.NewCond(mu)
cr := &ContextReader{
xTerm: xt,
term: term,
reader: bufio.NewReader(rd),
fd: fd,
closed: make(chan struct{}),
Expand Down Expand Up @@ -162,7 +163,7 @@ func (cr *ContextReader) processReads() {
n, err = cr.reader.Read(value)
value = value[:n]
case readerStatePassword:
value, err = cr.xTerm.ReadPassword(cr.fd)
value, err = cr.term.ReadPassword(cr.fd)
}
cr.mu.Lock()
cr.previousTermState = nil // A finalized read resets the terminal.
Expand Down Expand Up @@ -209,7 +210,7 @@ func (cr *ContextReader) fireCleanRead() error {
if cr.previousTermState != nil {
state := cr.previousTermState
cr.previousTermState = nil
if err := cr.xTerm.Restore(cr.fd, state); err != nil {
if err := cr.term.Restore(cr.fd, state); err != nil {
return trace.Wrap(err)
}
}
Expand Down Expand Up @@ -252,7 +253,7 @@ func (cr *ContextReader) firePasswordRead() error {
case readerStateIdle: // OK, transition and broadcast.
// Save present terminal state, so it may be restored in case the read goes
// from password to clean.
state, err := cr.xTerm.GetState(cr.fd)
state, err := cr.term.GetState(cr.fd)
if err != nil {
return trace.Wrap(err)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/prompt/context_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestContextReader_ReadPassword(t *testing.T) {

term := &fakeTerm{reader: pr}
cr := NewContextReader(pr)
cr.xTerm = term
cr.term = term
cr.fd = int(devNull.Fd()) // arbitrary, doesn't matter because term functions are mocked.

ctx := context.Background()
Expand Down

0 comments on commit 0ce2a5c

Please # to comment.