From 0ce2a5c2ad12e7b6d31f075417cac4646503f267 Mon Sep 17 00:00:00 2001
From: Alan Parra <alan.parra@goteleport.com>
Date: Fri, 25 Mar 2022 12:02:36 -0300
Subject: [PATCH] Address review comments (2)

---
 lib/utils/prompt/context_reader.go      | 29 +++++++++++++------------
 lib/utils/prompt/context_reader_test.go |  2 +-
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/lib/utils/prompt/context_reader.go b/lib/utils/prompt/context_reader.go
index d1e69fb4a18c9..f40099667e9d0 100644
--- a/lib/utils/prompt/context_reader.go
+++ b/lib/utils/prompt/context_reader.go
@@ -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)
 }
 
@@ -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
@@ -108,12 +109,12 @@ 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
 		}
 	}
@@ -121,7 +122,7 @@ func NewContextReader(rd io.Reader) *ContextReader {
 	mu := &sync.Mutex{}
 	cond := sync.NewCond(mu)
 	cr := &ContextReader{
-		xTerm:  xt,
+		term:   term,
 		reader: bufio.NewReader(rd),
 		fd:     fd,
 		closed: make(chan struct{}),
@@ -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.
@@ -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)
 			}
 		}
@@ -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)
 		}
diff --git a/lib/utils/prompt/context_reader_test.go b/lib/utils/prompt/context_reader_test.go
index 14de4c316e50f..83b26b11cc234 100644
--- a/lib/utils/prompt/context_reader_test.go
+++ b/lib/utils/prompt/context_reader_test.go
@@ -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()