Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions internal/authutil/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,10 @@ func runDeviceFlow(ctx context.Context, providerURL string, clientID string, sco
return pollDeviceToken(waitCtx, tokenURL, clientID, deviceResp.DeviceCode, deviceResp.Interval, deviceResp.ExpiresIn)
}

// eofPollInterval bounds each watcher read so the loop can check for stop().
// The same value probes deadline support before the watcher starts.
const eofPollInterval = 250 * time.Millisecond

// deadlineReader is a stream that supports read deadlines, satisfied by
// *os.File (including os.Stdin and os.Pipe ends).
type deadlineReader interface {
Expand All @@ -504,13 +508,24 @@ func watchStdinEOF(parent context.Context) (context.Context, context.CancelFunc)
// input intended for a later interactive prompt.
//
// Input received during the wait is read and discarded — nothing else reads
// the stream while login is blocking on the auth callback or device poll. On
// streams where read deadlines are unsupported the watcher falls back to a
// single blocking read; EOF still cancels, and any leftover reader is
// abandoned rather than stealing later input in the common (deadline-capable)
// case.
// the stream while login is blocking on the auth callback or device poll.
//
// Detecting EOF requires read deadlines, so that support is probed up front.
// Without them the only way to watch is a blocking read that stop() cannot
// interrupt, which parks a reader on the stream and swallows keystrokes meant
// for the interactive picker that runs right after login. In that case the
// watcher declines to run at all and the wait is cancellable by ^C only.
// Windows console handles are never pollable, so *os.File deadlines are
// always unsupported there — and ^D is not a console EOF convention on
// Windows regardless.
func watchReaderEOF(parent context.Context, in deadlineReader) (context.Context, context.CancelFunc) {
ctx, cancel := context.WithCancel(parent)

if err := in.SetReadDeadline(time.Now().Add(eofPollInterval)); err != nil {
_ = in.SetReadDeadline(time.Time{})
return ctx, cancel
}

done := make(chan struct{})

go func() {
Expand All @@ -527,13 +542,10 @@ func watchReaderEOF(parent context.Context, in deadlineReader) (context.Context,
default:
}

if err := in.SetReadDeadline(time.Now().Add(250 * time.Millisecond)); err != nil {
// Deadlines unsupported for this stream; fall back to a
// single blocking read. EOF cancels; anything else ends the
// watcher to avoid a busy loop.
if _, rerr := in.Read(buf); errors.Is(rerr, io.EOF) {
cancel()
}
if err := in.SetReadDeadline(time.Now().Add(eofPollInterval)); err != nil {
// Unreachable: support was probed before this goroutine
// started. Never fall back to a blocking read here — it
// cannot be interrupted by stop().
return
}

Expand Down
90 changes: 90 additions & 0 deletions internal/authutil/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package authutil
import (
"context"
"errors"
"io"
"net/http"
"net/http/httptest"
"os"
Expand Down Expand Up @@ -125,6 +126,95 @@ func TestWatchReaderEOF_NoCancelBeforeEOFOrStop(t *testing.T) {
}
}

// noDeadlineReader is a stream that does not support read deadlines, as every
// Windows console handle behaves (os.Stdin is never pollable there). Read
// blocks until the test releases it, standing in for a console read that no
// stop() can interrupt.
type noDeadlineReader struct {
reads atomic.Int32
release chan struct{}
}

func (r *noDeadlineReader) SetReadDeadline(time.Time) error { return os.ErrNoDeadline }

func (r *noDeadlineReader) Read(p []byte) (int, error) {
r.reads.Add(1)
<-r.release
return 0, io.EOF
}

// TestWatchReaderEOF_NoBlockingReadWhenDeadlinesUnsupported is the #263
// regression test. When deadlines are unsupported the watcher must not read
// the stream at all: a blocking read there cannot be interrupted by stop(),
// so it stays parked on stdin and eats the arrow keys the context picker
// needs after login.
func TestWatchReaderEOF_NoBlockingReadWhenDeadlinesUnsupported(t *testing.T) {
in := &noDeadlineReader{release: make(chan struct{})}
defer close(in.release)

ctx, stop := watchReaderEOF(context.Background(), in)

// Span several would-be poll cycles.
select {
case <-ctx.Done():
t.Fatal("context canceled while stream open and idle")
case <-time.After(500 * time.Millisecond):
}

if got := in.reads.Load(); got != 0 {
t.Fatalf("watcher read the stream %d time(s); it must not read a stream it cannot poll", got)
}

// The caller's contract still holds: stop() cancels.
stop()
select {
case <-ctx.Done():
case <-time.After(3 * time.Second):
t.Fatal("stop() did not cancel the context")
}
}

// TestWatchReaderEOF_UnsupportedDeadlineLeavesInputIntact asserts the
// user-visible property behind #263: every byte typed during the login wait is
// still there for the next reader, so the picker sees the user's keystrokes.
// A regular file is used because it reports the same ErrNoDeadline a Windows
// console handle does.
func TestWatchReaderEOF_UnsupportedDeadlineLeavesInputIntact(t *testing.T) {
f, err := os.CreateTemp(t.TempDir(), "stdin")
if err != nil {
t.Fatalf("CreateTemp: %v", err)
}
defer f.Close()

const typed = "hello\n"
if _, err := f.WriteString(typed); err != nil {
t.Fatalf("write: %v", err)
}
if _, err := f.Seek(0, io.SeekStart); err != nil {
t.Fatalf("seek: %v", err)
}

// Guard the premise: this test is meaningless if the file supports
// deadlines, since the watcher would then take the polling path.
if derr := f.SetReadDeadline(time.Now().Add(time.Second)); derr == nil {
t.Skip("regular files support read deadlines on this platform")
}
_ = f.SetReadDeadline(time.Time{})

_, stop := watchReaderEOF(context.Background(), f)
time.Sleep(300 * time.Millisecond)
stop()
time.Sleep(100 * time.Millisecond)

rest, err := io.ReadAll(f)
if err != nil {
t.Fatalf("read: %v", err)
}
if string(rest) != typed {
t.Fatalf("watcher consumed input meant for the picker: got %q, want %q", string(rest), typed)
}
}

func mustPipe(t *testing.T) (*os.File, *os.File) {
t.Helper()
r, w, err := os.Pipe()
Expand Down