diff --git a/internal/authutil/login.go b/internal/authutil/login.go index ada3596..37485fd 100644 --- a/internal/authutil/login.go +++ b/internal/authutil/login.go @@ -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 { @@ -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() { @@ -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 } diff --git a/internal/authutil/login_test.go b/internal/authutil/login_test.go index a180791..80377a8 100644 --- a/internal/authutil/login_test.go +++ b/internal/authutil/login_test.go @@ -3,6 +3,7 @@ package authutil import ( "context" "errors" + "io" "net/http" "net/http/httptest" "os" @@ -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()