From 45a09c17469e55ac7e20ee101d7fbf85a5d17194 Mon Sep 17 00:00:00 2001 From: Eric_Lee Date: Tue, 3 Nov 2026 22:31:46 +0800 Subject: [PATCH 1/4] fix: preserve Telnet mode boundaries and environment negotiation --- client.go | 12 +- decoder.go | 8 +- doc.go | 8 +- environment.go | 119 +++++++++++++++ environment_test.go | 133 +++++++++++++++++ terminal_mode.go | 29 ++++ terminal_protocol_test.go | 294 ++++++++++++++++++++++++++++++++++++++ transport.go | 79 +++++++--- 8 files changed, 657 insertions(+), 25 deletions(-) create mode 100644 environment.go create mode 100644 environment_test.go create mode 100644 terminal_mode.go create mode 100644 terminal_protocol_test.go diff --git a/client.go b/client.go index cec5883..42422d8 100644 --- a/client.go +++ b/client.go @@ -35,8 +35,9 @@ func Dial(network, addr string, config *ClientConfig) (*Client, error) { return newClientConn(conn, fullConfig) } -// NewClientConn creates an authenticated Telnet client over an existing -// net.Conn. This is the counterpart of Dial for gateway and tunneled links. +// NewClientConn creates a Telnet client over an existing net.Conn, performing +// prompt-based login when User or Password is configured. This is the +// counterpart of Dial for gateway and tunneled links. func NewClientConn(conn net.Conn, config *ClientConfig) (*Client, error) { if conn == nil { return nil, fmt.Errorf("%w: connection is nil", ErrInvalidConfig) @@ -87,12 +88,17 @@ func newClientConn(conn net.Conn, config clientConfig) (*Client, error) { return client, nil } +// Read receives terminal application data after Telnet decoding and optional +// stream wrappers. In non-BINARY mode the transport removes a NUL immediately +// following CR, while preserving CR LF and unrelated NUL bytes. func (c *Client) Read(p []byte) (int, error) { return c.reader.Read(p) } // Write sends terminal application data. Telnet IAC bytes are escaped by the -// transport after optional stream wrappers have encoded the data. +// transport after optional stream wrappers have encoded the data. Line endings +// are preserved; callers choose CR LF or CR NUL sequences for NVT mode, or a +// device-specific terminator when required. func (c *Client) Write(p []byte) (int, error) { return c.writer.Write(p) } diff --git a/decoder.go b/decoder.go index 01aeec1..3302703 100644 --- a/decoder.go +++ b/decoder.go @@ -28,6 +28,9 @@ type telnetEvent struct { command byte option byte parameters []byte + // dataOffset is the number of application bytes decoded before this + // event in this feed call. It preserves option changes within a read. + dataOffset int } type decoder struct { @@ -67,7 +70,7 @@ func (d *decoder) feed(input []byte) ([]byte, []telnetEvent, error) { case sb: d.state = decoderSubnegotiationOption case nop, dm, brk, ip, ao, ayt, ec, el, ga: - events = append(events, telnetEvent{typ: commandEvent, command: value}) + events = append(events, telnetEvent{typ: commandEvent, command: value, dataOffset: len(data)}) d.state = decoderData default: // Unknown Telnet commands are consumed, never exposed as terminal data. @@ -76,7 +79,7 @@ func (d *decoder) feed(input []byte) ([]byte, []telnetEvent, error) { case decoderNegotiationOption: events = append(events, telnetEvent{ - typ: negotiationEvent, command: d.negotiationCommand, option: value, + typ: negotiationEvent, command: d.negotiationCommand, option: value, dataOffset: len(data), }) d.state = decoderData @@ -101,6 +104,7 @@ func (d *decoder) feed(input []byte) ([]byte, []telnetEvent, error) { typ: subnegotiationEvent, option: d.subOption, parameters: append([]byte(nil), d.subData...), + dataOffset: len(data), }) d.subData = d.subData[:0] d.state = decoderData diff --git a/doc.go b/doc.go index d14a033..431cd4a 100644 --- a/doc.go +++ b/doc.go @@ -1,5 +1,7 @@ // Package tclientlib provides an SSH-style client API for a single interactive -// Telnet connection. Dial and NewClientConn return an authenticated Client; -// Client.Read and Client.Write exchange terminal application data while Telnet -// framing and option negotiation remain internal. +// Telnet connection. Dial and NewClientConn optionally perform prompt-based +// login when credentials are configured; without credentials they return a +// client for manual interaction. Client.Read and Client.Write exchange terminal +// application data while Telnet framing and option negotiation remain internal. +// TerminalMode reports the negotiated modes without changing the local terminal. package tclientlib diff --git a/environment.go b/environment.go new file mode 100644 index 0000000..e31e2c2 --- /dev/null +++ b/environment.go @@ -0,0 +1,119 @@ +package tclientlib + +const ( + environmentIS byte = 0 + environmentVAR byte = 0 + environmentVALUE byte = 1 + environmentESC byte = 2 + environmentUSERVAR byte = 3 +) + +type environmentRequest struct { + kind byte + name []byte +} + +// environmentResponse implements the USER-only default environment. Explicit +// requests retain their order, including unknown variables (without VALUE), +// duplicates and wildcard expansions. It never reads the process environment. +// An invalid request or an oversized response is ignored as a whole. +func environmentResponse(request []byte, user string, old, reversed bool, limit int) ([]byte, bool) { + varCode, valueCode := environmentVAR, environmentVALUE + if old { + var seenVAR, seenVALUE bool + for i := 0; i < len(request); i++ { + switch request[i] { + case environmentESC: + i++ // Escaped controls are part of a name, not commands. + case environmentVAR: + seenVAR = true + case environmentVALUE: + seenVALUE = true + } + } + if seenVAR && seenVALUE { + return nil, reversed + } + // RFC 1571: older BSD peers use the opposite VAR/VALUE codes. + // Retain the detected convention for later unqualified SENDs. + if seenVAR || seenVALUE { + reversed = seenVALUE + } + if reversed { + varCode, valueCode = valueCode, varCode + } + } + + requests := make([]environmentRequest, 0, 2) + for i := 0; i < len(request); i++ { + value := request[i] + switch value { + case varCode, environmentUSERVAR: + requests = append(requests, environmentRequest{kind: value}) + case valueCode: + return nil, reversed // SEND contains names, never values. + default: + if len(requests) == 0 { + return nil, reversed + } + if value == environmentESC { + i++ + if i == len(request) { + return nil, reversed + } + value = request[i] + } + last := &requests[len(requests)-1] + last.name = append(last.name, value) + } + } + if len(requests) == 0 { + requests = append(requests, environmentRequest{kind: varCode}) + } + + response := []byte{environmentIS} + for _, request := range requests { + name := string(request.name) + defined := request.kind == varCode && (name == "USER" || name == "") && user != "" + if name == "" { + if !defined { + continue + } + name = "USER" + } + size := 1 + escapedEnvironmentLength(name) + if defined { + size += 1 + escapedEnvironmentLength(user) + } + if size > limit-len(response) { + return nil, reversed + } + response = append(response, request.kind) + response = appendEscapedEnvironment(response, name) + if defined { + response = append(response, valueCode) + response = appendEscapedEnvironment(response, user) + } + } + return response, reversed +} + +func escapedEnvironmentLength(value string) int { + n := len(value) + for i := 0; i < len(value); i++ { + if value[i] <= environmentUSERVAR { + n++ + } + } + return n +} + +func appendEscapedEnvironment(dst []byte, value string) []byte { + for i := 0; i < len(value); i++ { + if value[i] <= environmentUSERVAR { + dst = append(dst, environmentESC) + } + dst = append(dst, value[i]) + } + return dst +} diff --git a/environment_test.go b/environment_test.go new file mode 100644 index 0000000..1ca6781 --- /dev/null +++ b/environment_test.go @@ -0,0 +1,133 @@ +package tclientlib + +import ( + "bytes" + "fmt" + "io" + "testing" +) + +func TestEnvironmentRequestsEverySplit(t *testing.T) { + tests := []struct { + name string + request []byte + response []byte // Includes IS; nil means a malformed SEND is ignored. + }{ + {"default", nil, []byte("\x00\x00USER\x01alice")}, + {"well known wildcard", []byte{0}, []byte("\x00\x00USER\x01alice")}, + {"user wildcard", []byte{3}, []byte{0}}, + {"all wildcards", []byte{0, 3}, []byte("\x00\x00USER\x01alice")}, + {"explicit USER", []byte("\x00USER"), []byte("\x00\x00USER\x01alice")}, + {"unknown variable", []byte("\x00DISPLAY"), []byte("\x00\x00DISPLAY")}, + {"distinct namespace", []byte("\x03USER"), []byte("\x00\x03USER")}, + {"ordered variables", []byte("\x00DISPLAY\x00USER\x03SECRET"), []byte("\x00\x00DISPLAY\x00USER\x01alice\x03SECRET")}, + {"duplicates and wildcard", []byte("\x00USER\x00\x00USER"), []byte("\x00\x00USER\x01alice\x00USER\x01alice\x00USER\x01alice")}, + {"escaped names", []byte{'\x00', 'a', 2, 0, 2, 1, 2, 2, 2, 3, iac}, []byte{0, 0, 'a', 2, 0, 2, 1, 2, 2, 2, 3, iac}}, + {"VALUE rejected", []byte("\x00USER\x01"), nil}, + {"missing type", []byte("USER"), nil}, + {"incomplete escape", []byte("\x00USER\x02"), nil}, + {"escape before type", []byte{2, 0}, nil}, + } + for _, option := range []byte{newEnvironment, oldEnvironment} { + for _, tc := range tests { + t.Run(fmt.Sprintf("%d/%s", option, tc.name), func(t *testing.T) { + params := append([]byte{1}, tc.request...) + wire := append([]byte{iac, do, option}, (optionPacket{command: sb, option: option, parameters: params}).bytes()...) + want := []byte{iac, will, option} + if tc.response != nil { + want = append(want, (optionPacket{command: sb, option: option, parameters: tc.response}).bytes()...) + } + for split := 0; split <= len(wire); split++ { + client, conn := protocolTestClient(t, [][]byte{wire[:split], wire[split:]}, "alice") + if _, err := io.ReadAll(client); err != nil { + t.Fatalf("split %d: %v", split, err) + } + if !bytes.Equal(conn.writes.Bytes(), want) { + t.Fatalf("split %d: reply = %v, want %v", split, conn.writes.Bytes(), want) + } + } + }) + } + } +} + +func TestEnvironmentEscapesValueAndIAC(t *testing.T) { + wire := []byte{iac, do, newEnvironment, iac, sb, newEnvironment, 1, iac, se} + client, conn := protocolTestClient(t, streamChunks(wire, 1), "a\x00\x01\x02\x03\xffz") + if _, err := io.ReadAll(client); err != nil { + t.Fatal(err) + } + want := []byte{iac, will, newEnvironment, iac, sb, newEnvironment, 0, 0, 'U', 'S', 'E', 'R', 1, + 'a', 2, 0, 2, 1, 2, 2, 2, 3, iac, iac, 'z', iac, se} + if !bytes.Equal(conn.writes.Bytes(), want) { + t.Fatalf("reply = %v, want %v", conn.writes.Bytes(), want) + } +} + +func TestEnvironmentRequiresNegotiationAndConfiguredUser(t *testing.T) { + send := []byte{iac, sb, newEnvironment, 1, iac, se} + for _, user := range []string{"", "alice"} { + wire := append([]byte(nil), send...) + wire = append(wire, iac, do, newEnvironment, iac, dont, newEnvironment) + wire = append(wire, send...) + client, conn := protocolTestClient(t, streamChunks(wire, 1), user) + if _, err := io.ReadAll(client); err != nil { + t.Fatal(err) + } + want := []byte{iac, wont, newEnvironment} + if user != "" { + want = []byte{iac, will, newEnvironment, iac, wont, newEnvironment} + } + if !bytes.Equal(conn.writes.Bytes(), want) { + t.Fatalf("user %q: reply = %v, want %v", user, conn.writes.Bytes(), want) + } + } +} + +func TestOldEnvironmentReversedCodesPersistPerConnection(t *testing.T) { + // A BSD-style SEND uses 1 for VAR and 0 for VALUE (RFC 1571). + wire := []byte{iac, do, oldEnvironment} + wire = append(wire, (optionPacket{command: sb, option: oldEnvironment, parameters: []byte("\x01\x01DISPLAY\x01USER")}).bytes()...) + wire = append(wire, (optionPacket{command: sb, option: oldEnvironment, parameters: []byte{1}}).bytes()...) + wire = append(wire, iac, do, newEnvironment) + wire = append(wire, (optionPacket{command: sb, option: newEnvironment, parameters: []byte{1}}).bytes()...) + for split := 0; split <= len(wire); split++ { + client, conn := protocolTestClient(t, [][]byte{wire[:split], wire[split:]}, "alice") + if _, err := io.ReadAll(client); err != nil { + t.Fatal(err) + } + want := []byte{iac, will, oldEnvironment} + want = append(want, (optionPacket{command: sb, option: oldEnvironment, parameters: []byte("\x00\x01DISPLAY\x01USER\x00alice")}).bytes()...) + want = append(want, (optionPacket{command: sb, option: oldEnvironment, parameters: []byte("\x00\x01USER\x00alice")}).bytes()...) + want = append(want, iac, will, newEnvironment) + want = append(want, (optionPacket{command: sb, option: newEnvironment, parameters: []byte("\x00\x00USER\x01alice")}).bytes()...) + if !bytes.Equal(conn.writes.Bytes(), want) { + t.Fatalf("split %d: reply = %v, want %v", split, conn.writes.Bytes(), want) + } + } + client, conn := protocolTestClient(t, [][]byte{{iac, do, oldEnvironment, iac, sb, oldEnvironment, 1, iac, se}}, "alice") + if _, err := io.ReadAll(client); err != nil { + t.Fatal(err) + } + want := []byte{iac, will, oldEnvironment} + want = append(want, (optionPacket{command: sb, option: oldEnvironment, parameters: []byte("\x00\x00USER\x01alice")}).bytes()...) + if !bytes.Equal(conn.writes.Bytes(), want) { + t.Fatalf("new connection retained reversed codes: %v", conn.writes.Bytes()) + } +} + +func TestEnvironmentResponseBound(t *testing.T) { + for _, tc := range []struct { + limit int + want bool + }{{11, false}, {12, true}} { + got, _ := environmentResponse(nil, "alice", false, false, tc.limit) + if (got != nil) != tc.want { + t.Fatalf("limit %d: response = %v", tc.limit, got) + } + } + got, _ := environmentResponse([]byte{0, 0}, "alice", false, false, 12) + if got != nil { + t.Fatalf("wildcard expansion exceeded response bound: %v", got) + } +} diff --git a/terminal_mode.go b/terminal_mode.go new file mode 100644 index 0000000..824d173 --- /dev/null +++ b/terminal_mode.go @@ -0,0 +1,29 @@ +package tclientlib + +// TerminalMode is a snapshot of the currently negotiated terminal options. +// Each direction is independent. The library never changes the caller's local +// terminal settings. Options are processed as Client.Read consumes the stream. +type TerminalMode struct { + RemoteEcho bool + LocalSuppressGA bool + RemoteSuppressGA bool + SendBinary bool + ReceiveBinary bool +} + +// TerminalMode returns a concurrency-safe snapshot of negotiated options. +// SendBinary describes the negotiated wire mode; Write continues to preserve +// application bytes (apart from IAC escaping) in either mode. Callers remain +// responsible for choosing complete CR LF or CR NUL sequences in NVT mode. +func (c *Client) TerminalMode() TerminalMode { + t := c.transport + t.stateMu.Lock() + defer t.stateMu.Unlock() + return TerminalMode{ + RemoteEcho: t.remoteOptions[echoOption], + LocalSuppressGA: t.localOptions[suppressGAOption], + RemoteSuppressGA: t.remoteOptions[suppressGAOption], + SendBinary: t.localOptions[binaryOption], + ReceiveBinary: t.remoteOptions[binaryOption], + } +} diff --git a/terminal_protocol_test.go b/terminal_protocol_test.go new file mode 100644 index 0000000..02abcfe --- /dev/null +++ b/terminal_protocol_test.go @@ -0,0 +1,294 @@ +package tclientlib + +import ( + "bytes" + "encoding/binary" + "fmt" + "io" + "net" + "runtime" + "sync" + "testing" + "time" +) + +func TestNVTReceiveEverySplit(t *testing.T) { + tests := []struct { + name string + wire []byte + want []byte + }{ + {"line endings", []byte("a\r\x00b\r\nc\x00d\r\re\r"), []byte("a\rb\r\nc\x00d\r\re\r")}, + {"repeated CR NUL", []byte("\r\r\x00\x00\r\x00"), []byte("\r\r\x00\r")}, + {"escaped IAC", []byte{'\r', iac, iac, 0, '\r', 0, iac, iac}, []byte{'\r', iac, 0, '\r', iac}}, + {"command inside CR NUL", []byte{'\r', iac, nop, 0}, []byte{'\r'}}, + {"negotiation inside CR NUL", []byte{'\r', iac, will, echoOption, 0}, []byte{'\r'}}, + {"subnegotiation inside CR NUL", []byte{'\r', iac, sb, terminalType, 0, '\r', 0, iac, se, 0}, []byte{'\r'}}, + {"outbound binary is independent", []byte{iac, do, binaryOption, '\r', 0}, []byte{'\r'}}, + {"binary preserves all data", []byte{iac, will, binaryOption, '\r', 0, '\r', '\n', 0, iac, iac}, []byte{'\r', 0, '\r', '\n', 0, iac}}, + {"mode changes within read", []byte{'\r', 0, iac, will, binaryOption, '\r', 0, iac, wont, binaryOption, '\r', 0}, []byte{'\r', '\r', 0, '\r'}}, + {"enter binary after CR", []byte{'\r', iac, will, binaryOption, 0}, []byte{'\r', 0}}, + {"leave binary after CR", []byte{iac, will, binaryOption, '\r', iac, wont, binaryOption, 0}, []byte{'\r', 0}}, + {"empty binary span resets CR", []byte{'\r', iac, will, binaryOption, iac, wont, binaryOption, 0}, []byte{'\r', 0}}, + {"duplicate WONT preserves CR", []byte{'\r', iac, wont, binaryOption, 0}, []byte{'\r'}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + for split := 0; split <= len(tc.wire); split++ { + t.Run(fmt.Sprint(split), func(t *testing.T) { + client, _ := protocolTestClient(t, [][]byte{tc.wire[:split], tc.wire[split:]}, "") + got, err := io.ReadAll(client) + if err != nil || !bytes.Equal(got, tc.want) { + t.Fatalf("read = %v, %v; want %v", got, err, tc.want) + } + }) + } + client, _ := protocolTestClient(t, streamChunks(tc.wire, 1), "") + got, err := io.ReadAll(client) + if err != nil || !bytes.Equal(got, tc.want) { + t.Fatalf("byte reads = %v, %v; want %v", got, err, tc.want) + } + }) + } +} + +func TestWriteRetainsApplicationLineEndingsInBothModes(t *testing.T) { + for _, binary := range []bool{false, true} { + t.Run(fmt.Sprint(binary), func(t *testing.T) { + var wire []byte + if binary { + wire = []byte{iac, do, binaryOption} + } + client, conn := protocolTestClient(t, [][]byte{wire}, "") + if _, err := io.ReadAll(client); err != nil { + t.Fatal(err) + } + conn.writes.Reset() + input := []byte{'\r', '\n', '\r', 0, '\r', 'x', '\n', iac} + for _, value := range input { + if n, err := client.Write([]byte{value}); err != nil || n != 1 { + t.Fatalf("write = %d, %v", n, err) + } + } + want := append(append([]byte(nil), input...), iac) + if !bytes.Equal(conn.writes.Bytes(), want) { + t.Fatalf("wire = %v, want %v", conn.writes.Bytes(), want) + } + }) + } +} + +func TestTerminalModeNegotiationDirectionsAndDuplicates(t *testing.T) { + enable := []byte{iac, will, echoOption, iac, do, suppressGAOption, iac, will, suppressGAOption, + iac, do, binaryOption, iac, will, binaryOption} + disable := []byte{iac, wont, echoOption, iac, dont, suppressGAOption, iac, wont, suppressGAOption, + iac, dont, binaryOption, iac, wont, binaryOption} + first := append(append(append([]byte(nil), enable...), enable...), 'a') + second := append(append(append([]byte(nil), disable...), disable...), 'b') + client, conn := protocolTestClient(t, [][]byte{first, second}, "") + if got := client.TerminalMode(); got != (TerminalMode{}) { + t.Fatalf("initial mode = %+v", got) + } + p := make([]byte, 1) + if _, err := io.ReadFull(client, p); err != nil || p[0] != 'a' { + t.Fatalf("first read = %q, %v", p, err) + } + wantMode := TerminalMode{RemoteEcho: true, LocalSuppressGA: true, RemoteSuppressGA: true, SendBinary: true, ReceiveBinary: true} + if got := client.TerminalMode(); got != wantMode { + t.Fatalf("enabled mode = %+v, want %+v", got, wantMode) + } + wantReplies := []byte{iac, do, echoOption, iac, will, suppressGAOption, iac, do, suppressGAOption, + iac, will, binaryOption, iac, do, binaryOption} + if !bytes.Equal(conn.writes.Bytes(), wantReplies) { + t.Fatalf("enable replies = %v, want %v", conn.writes.Bytes(), wantReplies) + } + if _, err := io.ReadFull(client, p); err != nil || p[0] != 'b' { + t.Fatalf("second read = %q, %v", p, err) + } + if got := client.TerminalMode(); got != (TerminalMode{}) { + t.Fatalf("disabled mode = %+v", got) + } + wantReplies = append(wantReplies, iac, dont, echoOption, iac, wont, suppressGAOption, iac, dont, suppressGAOption, + iac, wont, binaryOption, iac, dont, binaryOption) + if !bytes.Equal(conn.writes.Bytes(), wantReplies) { + t.Fatalf("all replies = %v, want %v", conn.writes.Bytes(), wantReplies) + } +} + +func TestTerminalModeConcurrentAccess(t *testing.T) { + cycle := []byte{iac, will, echoOption, iac, do, suppressGAOption, iac, do, binaryOption, + iac, will, binaryOption, 'x', iac, wont, echoOption, iac, dont, suppressGAOption, + iac, dont, binaryOption, iac, wont, binaryOption} + client, _ := protocolTestClient(t, streamChunks(bytes.Repeat(cycle, 100), 3), "") + var wg sync.WaitGroup + errors := make(chan error, 3) + wg.Add(4) + go func() { + defer wg.Done() + _, err := io.ReadAll(client) + errors <- err + }() + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + _ = client.TerminalMode() + } + }() + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + if _, err := client.Write([]byte("x\r\n")); err != nil { + errors <- err + return + } + } + errors <- nil + }() + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + if err := client.WindowChange(24+i, 80+i); err != nil { + errors <- err + return + } + } + errors <- nil + }() + wg.Wait() + close(errors) + for err := range errors { + if err != nil { + t.Fatal(err) + } + } +} + +func TestCommandOutputDoesNotWaitForBlockedApplicationWrite(t *testing.T) { + server, peer := net.Pipe() + defer server.Close() + deadline := time.Now().Add(2 * time.Second) + _ = server.SetDeadline(deadline) + _ = peer.SetDeadline(deadline) + conn := &protocolWriteStartedConn{Conn: peer, started: make(chan struct{})} + client, err := NewClientConn(conn, &ClientConfig{}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + written := make(chan error, 1) + go func() { _, err := client.Write([]byte("input")); written <- err }() + <-conn.started // The peer is not reading, so application Write holds writeMu. + served := make(chan error, 1) + go func() { _, err := server.Write([]byte{iac, nop, 'x'}); served <- err }() + p := make([]byte, 1) + if _, err := io.ReadFull(client, p); err != nil || p[0] != 'x' { + t.Fatalf("NOP unnecessarily blocked output: %q, %v", p, err) + } + if err := <-served; err != nil { + t.Fatal(err) + } + if _, err := io.ReadFull(server, make([]byte, len("input"))); err != nil { + t.Fatal(err) + } + if err := <-written; err != nil { + t.Fatal(err) + } +} + +type protocolWriteStartedConn struct { + net.Conn + started chan struct{} + once sync.Once +} + +func (c *protocolWriteStartedConn) Write(p []byte) (int, error) { + c.once.Do(func() { close(c.started) }) + return c.Conn.Write(p) +} + +func TestConcurrentNAWSKeepsWireModeAndLatestSize(t *testing.T) { + client, conn := protocolTestClient(t, nil, "") + transport := client.transport + if err := transport.handleEvent(telnetEvent{typ: negotiationEvent, command: do, option: windowSize}); err != nil { + t.Fatal(err) + } + start := make(chan struct{}) + errors := make(chan error, 3) + var wg sync.WaitGroup + for worker := 0; worker < 2; worker++ { + worker := worker + wg.Add(1) + go func() { + defer wg.Done() + <-start + for i := 0; i < 200; i++ { + if err := client.WindowChange(24+worker*200+i, 80+worker*200+i); err != nil { + errors <- err + return + } + runtime.Gosched() + } + }() + } + wg.Add(1) + go func() { + defer wg.Done() + <-start + for i := 0; i < 200; i++ { + for _, command := range []byte{dont, do} { + if err := transport.handleEvent(telnetEvent{typ: negotiationEvent, command: command, option: windowSize}); err != nil { + errors <- err + return + } + runtime.Gosched() + } + } + }() + close(start) + wg.Wait() + close(errors) + for err := range errors { + t.Fatal(err) + } + data, events, err := newDecoder(defaultMaxSubnegotiation).feed(conn.writes.Bytes()) + if err != nil || len(data) != 0 { + t.Fatalf("unexpected application bytes or decode error: %v, %v", data, err) + } + enabled := false + width, height := 0, 0 + for _, event := range events { + switch event.typ { + case negotiationEvent: + if event.option != windowSize || event.command != will && event.command != wont { + t.Fatalf("unexpected negotiation: %+v", event) + } + enabled = event.command == will + case subnegotiationEvent: + if !enabled || event.option != windowSize || len(event.parameters) != 4 { + t.Fatalf("NAWS sent outside negotiated mode: %+v", event) + } + width = int(binary.BigEndian.Uint16(event.parameters[:2])) + height = int(binary.BigEndian.Uint16(event.parameters[2:])) + default: + t.Fatalf("unexpected event: %+v", event) + } + } + if !enabled || width != transport.config.Width || height != transport.config.Height { + t.Fatalf("final wire state=%t %dx%d; latest size=%dx%d", enabled, width, height, transport.config.Width, transport.config.Height) + } +} + +func protocolTestClient(t *testing.T, chunks [][]byte, user string) (*Client, *streamTestConn) { + t.Helper() + conn := &streamTestConn{chunks: chunks} + config, err := prepareConfig(&ClientConfig{User: user}) + if err != nil { + t.Fatal(err) + } + // Bypass automatic login to exercise USER negotiation independently of + // prompts; the same transport is used by NewClientConn. + transport := newTransport(conn, config.transportConfig()) + client := &Client{conn: conn, transport: transport, reader: transport, writer: transport} + t.Cleanup(func() { _ = client.Close() }) + return client, conn +} diff --git a/transport.go b/transport.go index d22f0bb..aa2b344 100644 --- a/transport.go +++ b/transport.go @@ -20,9 +20,11 @@ type transport struct { readBuffer []byte pending []byte pendingErr error + nvtCR bool // Last received NVT application byte was CR; protected by readMu. - localOptions [256]bool - remoteOptions [256]bool + localOptions [256]bool + remoteOptions [256]bool + oldEnvironmentReversed bool } func newTransport(conn net.Conn, config transportConfig) *transport { @@ -67,14 +69,16 @@ func (t *transport) Read(p []byte) (int, error) { raw := t.readBuffer[:n] t.trace.bytes("wire", "recv", raw) data, events, decodeErr := t.decoder.feed(raw) - t.trace.bytes("application", "recv", data) - t.pending = append(t.pending, data...) + offset := 0 for _, event := range events { + t.receiveData(data[offset:event.dataOffset]) + offset = event.dataOffset t.trace.event("recv", event) if err := t.handleEvent(event); err != nil && t.pendingErr == nil { t.pendingErr = err } } + t.receiveData(data[offset:]) if decodeErr != nil && t.pendingErr == nil { t.trace.printf("decode error: %v", decodeErr) t.pendingErr = decodeErr @@ -90,6 +94,33 @@ func (t *transport) Read(p []byte) (int, error) { } } +// receiveData maps the NVT stream before charset conversion. CR is delivered +// immediately so a server's standalone CR cannot delay interactive output. +// Only its following NUL is removed; CR LF and unrelated NULs are preserved. +// Read calls this even for empty spans between events so a BINARY transition +// cannot leave a CR pending from the previous mode. +func (t *transport) receiveData(data []byte) { + t.stateMu.Lock() + binary := t.remoteOptions[binaryOption] + t.stateMu.Unlock() + + start := len(t.pending) + if binary { + t.nvtCR = false + t.pending = append(t.pending, data...) + } else { + for _, value := range data { + if t.nvtCR && value == 0 { + t.nvtCR = false + continue + } + t.pending = append(t.pending, value) + t.nvtCR = value == '\r' + } + } + t.trace.bytes("application", "recv", t.pending[start:]) +} + func (t *transport) Write(p []byte) (int, error) { t.trace.bytes("application", "send", p) encoded := appendEscapedIAC(make([]byte, 0, len(p)), p) @@ -135,6 +166,10 @@ func (t *transport) windowChange(height, width int) error { width = maxWindowSize } + // Serialize the new size and its packet with option changes and other + // window updates; an old snapshot must not overtake a later DONT or size. + t.writeMu.Lock() + defer t.writeMu.Unlock() t.stateMu.Lock() t.config.Height = height t.config.Width = width @@ -146,17 +181,22 @@ func (t *transport) windowChange(height, width int) error { if !enabled { return nil } - return t.writePackets(packet) + return t.writePacketsLocked(packet) } func (t *transport) handleEvent(event telnetEvent) error { + if event.typ != negotiationEvent && event.typ != subnegotiationEvent { + return nil + } + // Keep option state changes and their replies ahead of concurrent writes. + // In particular, data chosen using SendBinary must not overtake WILL. + t.writeMu.Lock() + defer t.writeMu.Unlock() switch event.typ { case negotiationEvent: - return t.writePackets(t.handleNegotiation(event.command, event.option)...) + return t.writePacketsLocked(t.handleNegotiation(event.command, event.option)...) case subnegotiationEvent: - return t.writePackets(t.handleSubnegotiation(event.option, event.parameters)...) - case commandEvent: - return nil + return t.writePacketsLocked(t.handleSubnegotiation(event.option, event.parameters)...) default: return nil } @@ -228,13 +268,14 @@ func (t *transport) handleSubnegotiation(option byte, parameters []byte) []optio case terminalSpeed: return []optionPacket{{command: sb, option: option, parameters: []byte("\x0038400,38400")}} case oldEnvironment, newEnvironment: - if t.config.User == "" { + params, reversed := environmentResponse(parameters[1:], t.config.User, + option == oldEnvironment, t.oldEnvironmentReversed, t.config.maxSubnegotiation) + if params == nil { return nil } - params := []byte{0, 0} - params = append(params, []byte("USER")...) - params = append(params, 1) - params = append(params, []byte(t.config.User)...) + if option == oldEnvironment { + t.oldEnvironmentReversed = reversed + } return []optionPacket{{command: sb, option: option, parameters: params}} default: return nil @@ -243,7 +284,7 @@ func (t *transport) handleSubnegotiation(option byte, parameters []byte) []optio func (t *transport) supportsLocalOption(option byte) bool { switch option { - case binaryOption, terminalType, windowSize, terminalSpeed: + case binaryOption, suppressGAOption, terminalType, windowSize, terminalSpeed: return true case oldEnvironment, newEnvironment: return t.config.User != "" @@ -268,7 +309,7 @@ func (t *transport) windowPacketLocked() optionPacket { return optionPacket{command: sb, option: windowSize, parameters: params} } -func (t *transport) writePackets(packets ...optionPacket) error { +func (t *transport) writePacketsLocked(packets ...optionPacket) error { if len(packets) == 0 { return nil } @@ -277,7 +318,8 @@ func (t *transport) writePackets(packets ...optionPacket) error { t.trace.packet("send", packet) data = append(data, packet.bytes()...) } - return t.writeAll(data) + _, err := t.writeRawLocked(data) + return err } func (t *transport) writeAll(data []byte) error { @@ -288,7 +330,10 @@ func (t *transport) writeAll(data []byte) error { func (t *transport) writeRaw(data []byte) (int, error) { t.writeMu.Lock() defer t.writeMu.Unlock() + return t.writeRawLocked(data) +} +func (t *transport) writeRawLocked(data []byte) (int, error) { total := 0 for len(data) > 0 { n, err := t.conn.Write(data) From ae38e62bd49aa7a052e6531d4fd2d4ea04169302 Mon Sep 17 00:00:00 2001 From: Eric_Lee Date: Tue, 3 Nov 2026 22:32:04 +0800 Subject: [PATCH 2/4] feat: add interactive tclient CLI with reliable terminal cleanup --- .github/workflows/test.yml | 4 + README.md | 61 +++- _example/telnet_client/go.mod | 11 +- _example/telnet_client/go.sum | 10 +- _example/telnet_client/main.go | 87 +---- cmd/tclient/main.go | 11 + docs/cli.md | 181 +++++++++++ go.mod | 6 + go.sum | 6 + internal/cli/input.go | 274 ++++++++++++++++ internal/cli/input_test.go | 138 ++++++++ internal/cli/options.go | 135 ++++++++ internal/cli/run.go | 307 ++++++++++++++++++ internal/cli/run_test.go | 294 +++++++++++++++++ internal/cli/terminal_other.go | 34 ++ internal/cli/terminal_unix.go | 143 +++++++++ scripts/test_cli_pty.py | 558 +++++++++++++++++++++++++++++++++ 17 files changed, 2163 insertions(+), 97 deletions(-) create mode 100644 cmd/tclient/main.go create mode 100644 docs/cli.md create mode 100644 go.sum create mode 100644 internal/cli/input.go create mode 100644 internal/cli/input_test.go create mode 100644 internal/cli/options.go create mode 100644 internal/cli/run.go create mode 100644 internal/cli/run_test.go create mode 100644 internal/cli/terminal_other.go create mode 100644 internal/cli/terminal_unix.go create mode 100644 scripts/test_cli_pty.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 49d2970..488c8dd 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,5 +20,9 @@ jobs: go-version: ${{ matrix.go-version }} - run: go test -race ./... - run: go vet ./... + - name: Verify interactive CLI with a real PTY and local TCP server + run: | + go build -o "$RUNNER_TEMP/tclient" ./cmd/tclient + python3 scripts/test_cli_pty.py "$RUNNER_TEMP/tclient" - run: cd _example/telnet_client && go test ./... - run: cd _example/monitor && go test ./... diff --git a/README.md b/README.md index f405ad5..b885138 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,31 @@ `tclientlib` is a small Go Telnet client for interactive host and network-device sessions. Its public API follows the same principle as `golang.org/x/crypto/ssh`: -constructors return a ready-to-use authenticated client, application reads and -writes never expose protocol framing, and protocol controls use explicit methods. +constructors return a ready-to-use client (with optional prompt-based login), +application reads and writes never expose protocol framing, and protocol controls +use explicit methods. + +## Command-line client + +Install the interactive client with Go 1.20 or later: + +```bash +go install github.com/LeeEirc/tclientlib/cmd/tclient@latest +``` + +Or build the current checkout: + +```bash +go build ./cmd/tclient +./tclient 192.0.2.10 23 +./tclient --auto-login --user admin --ask-pass 192.0.2.10 23 +./tclient --charset gbk --line-ending crnul 192.0.2.10 23 +``` + +The default is manual interaction. `Ctrl+]` opens local commands such as `help`, +`status`, `send brk`, and `quit`. The client handles terminal restoration, window +changes, negotiated remote echo, and redirected input. See [the CLI guide](docs/cli.md) +for options, exit codes, tracing, and compatibility boundaries. ## API @@ -37,13 +60,15 @@ func (*Client) Write([]byte) (int, error) func (*Client) Close() error func (*Client) WindowChange(height, width int) error func (*Client) SendCommand(Command) error +func (*Client) TerminalMode() TerminalMode ``` There is deliberately one I/O path: - `Client.Write` always writes terminal application data and escapes Telnet `IAC` bytes internally. -- `Client.Read` returns terminal application data only. +- `Client.Read` returns terminal application data only, removing the protocol + NUL after a carriage return in the non-BINARY receive direction. - `SendCommand` sends Telnet controls such as `CommandBreak`. - `Dial` and `NewClientConn` perform initial prompt-based login when credentials are configured. @@ -51,6 +76,23 @@ There is deliberately one I/O path: The library does not expose raw writes, a second data writer, or a separate public login lifecycle. +`TerminalMode()` returns a concurrency-safe snapshot of `RemoteEcho`, +`LocalSuppressGA`, `RemoteSuppressGA`, `SendBinary`, and `ReceiveBinary`. Modes +change as `Read` consumes negotiation packets. The library never changes the +caller's local terminal and does not start a background reader. + +Receive NVT decoding follows the exact position of BINARY negotiation in the +stream, including changes within one network read. `CR LF` and standalone NUL +are retained; `CR NUL` becomes `CR` outside BINARY mode. `Client.Write` continues +to preserve caller-supplied CR/LF/NUL bytes and escape IAC. The caller is +responsible for choosing appropriate outgoing line endings; the CLI maps TTY +Enter keys and automatic login uses `LineEnding`. + +The client accepts SGA in both directions. Environment negotiation exposes only +the explicitly configured `User`: requested variables are returned in order, +unknown variables are marked undefined, and environment control bytes are +escaped. Process environment variables are never exported. + ## Basic usage ```go @@ -219,13 +261,20 @@ active; semantic negotiation and login-state events remain visible. **Warning:** post-login commands and terminal output may still contain sensitive information. Store traces securely and remove them after troubleshooting. -The example client can write a trace directly: +The CLI can write a trace explicitly (the file must not already exist): ```bash -cd _example/telnet_client -go run . -ip 192.0.2.10 -port 23 -username admin -trace-file telnet.trace +go run ./cmd/tclient --trace-file telnet.trace 192.0.2.10 23 ``` +Manual login does not run the automatic-login redaction phase. A trace can +therefore contain manually typed passwords as well as commands and output. The +CLI warns when tracing is enabled and creates the file with mode `0600`. + +`_example/telnet_client` now delegates to the same CLI, with the same flags. +Replace its old `-ip`/`-port` flags with positional `host [port]`, and replace +`-username`/`-password` with `--auto-login --user ... --ask-pass`. + For failures involving another Telnet client, or for observing traffic without changing application code, use the transparent monitor: diff --git a/_example/telnet_client/go.mod b/_example/telnet_client/go.mod index 3230a03..825a816 100644 --- a/_example/telnet_client/go.mod +++ b/_example/telnet_client/go.mod @@ -1,12 +1,13 @@ -module telnet_client +module github.com/LeeEirc/tclientlib/_example/telnet_client go 1.20 +require github.com/LeeEirc/tclientlib v0.0.0 + require ( - github.com/LeeEirc/tclientlib v0.0.0 - golang.org/x/term v0.10.0 + golang.org/x/sys v0.30.0 // indirect + golang.org/x/term v0.29.0 // indirect + golang.org/x/text v0.22.0 // indirect ) -require golang.org/x/sys v0.10.0 // indirect - replace github.com/LeeEirc/tclientlib => ../../ diff --git a/_example/telnet_client/go.sum b/_example/telnet_client/go.sum index 6fd71e2..4acb987 100644 --- a/_example/telnet_client/go.sum +++ b/_example/telnet_client/go.sum @@ -1,4 +1,6 @@ -golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA= -golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/term v0.10.0 h1:3R7pNqamzBraeqj/Tj8qt1aQ2HpmlC+Cx/qL/7hn4/c= -golang.org/x/term v0.10.0/go.mod h1:lpqdcUyK/oCiQxvxVrppt5ggO2KCZ5QblwqPnfZ6d5o= +golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= +golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.29.0 h1:L6pJp37ocefwRRtYPKSWOWzOtWSxVajvz2ldH/xi3iU= +golang.org/x/term v0.29.0/go.mod h1:6bl4lRlvVuDgSf3179VpIxBF0o10JUpXWOnI7nErv7s= +golang.org/x/text v0.22.0 h1:bofq7m3/HAFvbF51jz3Q9wLg3jkvSPuiZu/pD1XwgtM= +golang.org/x/text v0.22.0/go.mod h1:YRoo4H8PVmsu+E3Ou7cqLVH8oXWIHVoX0jqUWALQhfY= diff --git a/_example/telnet_client/main.go b/_example/telnet_client/main.go index 3f7238a..6acadf4 100644 --- a/_example/telnet_client/main.go +++ b/_example/telnet_client/main.go @@ -1,91 +1,14 @@ +// This entry point is retained for existing example checkouts. The supported +// client lives in cmd/tclient; both entry points use the same implementation +// and flags (see docs/cli.md). package main import ( - "flag" - "io" - "log" - "net" "os" - "os/signal" - "syscall" - "time" - "golang.org/x/term" - - "github.com/LeeEirc/tclientlib" -) - -var ( - ip string - port string - username string - password string - traceFile string + "github.com/LeeEirc/tclientlib/internal/cli" ) -func init() { - flag.StringVar(&ip, "ip", "127.0.0.1", "telnet IP") - flag.StringVar(&port, "port", "23", "telnet port") - flag.StringVar(&username, "username", "", "telnet username") - flag.StringVar(&password, "password", "", "telnet password") - flag.StringVar(&traceFile, "trace-file", "", "write Telnet wire and protocol trace to this file") -} - func main() { - flag.Parse() - - var trace io.Writer - var traceCloser io.Closer - if traceFile != "" { - file, err := os.Create(traceFile) - if err != nil { - log.Fatal(err) - } - trace = file - traceCloser = file - defer traceCloser.Close() - log.Printf("warning: trace file may contain credentials and terminal data: %s", traceFile) - } - - fd := int(os.Stdin.Fd()) - state, err := term.MakeRaw(fd) - if err != nil { - log.Fatal(err) - } - defer term.Restore(fd, state) - - width, height, _ := term.GetSize(fd) - client, err := tclientlib.Dial("tcp", net.JoinHostPort(ip, port), &tclientlib.ClientConfig{ - User: username, Password: password, Timeout: 30 * time.Second, - TerminalType: "xterm", Width: width, Height: height, - Trace: trace, - }) - if err != nil { - log.Fatal(err) - } - defer client.Close() - - done := make(chan struct{}, 2) - go func() { - _, _ = io.Copy(os.Stdout, client) - done <- struct{}{} - }() - go func() { - _, _ = io.Copy(client, os.Stdin) - done <- struct{}{} - }() - - resize := make(chan os.Signal, 1) - signal.Notify(resize, syscall.SIGWINCH) - for { - select { - case <-done: - return - case <-resize: - width, height, _ = term.GetSize(fd) - if err := client.WindowChange(height, width); err != nil { - log.Printf("window change: %v", err) - } - } - } + os.Exit(cli.Run(os.Args[1:], os.Stdin, os.Stdout, os.Stderr)) } diff --git a/cmd/tclient/main.go b/cmd/tclient/main.go new file mode 100644 index 0000000..eed6c8f --- /dev/null +++ b/cmd/tclient/main.go @@ -0,0 +1,11 @@ +package main + +import ( + "os" + + "github.com/LeeEirc/tclientlib/internal/cli" +) + +func main() { + os.Exit(cli.Run(os.Args[1:], os.Stdin, os.Stdout, os.Stderr)) +} diff --git a/docs/cli.md b/docs/cli.md new file mode 100644 index 0000000..2bf35b3 --- /dev/null +++ b/docs/cli.md @@ -0,0 +1,181 @@ +# tclient CLI + +`tclient` uses the same Telnet transport and optional login state machine as the +library. It is an interactive client and a way to reproduce a connection outside +Koko. It is not a complete GNU/NetKit Telnet replacement. + +## Build and connect + +```bash +go build ./cmd/tclient +./tclient 192.0.2.10 +./tclient 192.0.2.10 2323 +./tclient 2001:db8::10 23 +``` + +The port defaults to 23. Options go before the host. Use `--help` for the full +option list. This client supports plain TCP Telnet; it does not encrypt traffic. + +Interactive terminal support currently targets Unix systems. The PTY regression +suite runs on Linux; macOS can build the same Unix implementation. Other +platforms reject terminal input before changing console settings; redirected +input remains available. + +The default is **manual login**. Server prompts, menus, password changes, and +subsequent interactions are displayed without trying to infer authentication +success. No environment username is sent in this mode. + +To use the library's automatic login behavior explicitly: + +```bash +./tclient --auto-login --user admin --ask-pass 192.0.2.10 23 +``` + +`--ask-pass` reads the password from the terminal without displaying it. There +is no command-line password flag. `--user` and `--ask-pass` require +`--auto-login`, which avoids confusing environment-based Telnet `-l` behavior +with prompt-based authentication. An automatic login failure or required +password change ends the connection; it does not silently fall back to manual +mode. A username-only automatic login is also supported. + +`--timeout` bounds connection establishment and initial automatic login. It is +not an interactive-session idle timeout. + +## Input, output, and character encoding + +```bash +./tclient --charset gbk --line-ending crnul 192.0.2.10 23 +./tclient --echo off 192.0.2.10 23 +``` + +`--charset` supports `utf-8`, `gbk`, `gb18030`, and `big5`. Incoming bytes are +decoded before login matching and display; outgoing UTF-8 input is encoded +before Telnet IAC escaping. Charset conversion is a single stream shared by +login and interaction, not a second Telnet implementation. + +For terminal input, Enter maps to `--line-ending`: + +| Value | Bytes sent | +| --- | --- | +| `crlf` (default) | CR LF | +| `crnul` | CR NUL | +| `cr` | CR | +| `lf` | LF | + +A CR followed by LF or NUL is treated as one Enter even when split across +input reads. This prevents pasted CRLF text from producing extra empty lines. +The selected terminator also configures automatic login. The choice is an +application input policy and stays in effect if BINARY is negotiated. + +The interactive client sends characters as they are typed. `--echo auto` +enables local echo when remote ECHO is not negotiated, and disables it when the +server announces `WILL ECHO`. `on` and `off` override local echo explicitly. +This is character-at-a-time interaction, without local line editing or +RFC 1184 LINEMODE. Echo negotiation is not inferred from password prompt text. +Use the server's echo policy or an explicit `--echo off` for a peer that does +not negotiate password echo correctly. + +Local raw mode is used only for terminal input and is restored before returning +from the CLI, including connection errors, remote disconnects, local quit, and +handled termination signals. Terminal dimensions are sent when NAWS is +negotiated; supported Unix systems also send changes after SIGWINCH. + +For redirected input, no raw terminal mode, local echo, escape commands, or +Enter rewriting is applied: + +```bash +printf 'show version\r\nexit\r\n' | ./tclient 192.0.2.10 23 +``` + +Charset conversion, if requested, still applies. After stdin EOF, the encoder +is flushed and the CLI continues reading final server output. The write side +stays open so the library can still reply to Telnet negotiation. Include the +device's logout command when appropriate. `--drain-timeout` bounds this wait +(default 5 seconds); a server that does not close within this interval produces +an I/O error. Redirected output receives application bytes without local +status messages or local echo; these go to stderr. + +For an endpoint that needs TCP EOF before producing its reply, explicitly use +`--close-write`. This half-closes TCP after the input encoder has been flushed. +Further Telnet negotiation cannot be answered after half-close, so this option +is appropriate only when the endpoint does not require subsequent negotiation. + +## Local commands + +Press **Ctrl+]** in an interactive terminal to open the local command prompt: + +```text +tclient> help +tclient> status +tclient> send brk +tclient> send ip +tclient> send ayt +tclient> send nop +tclient> continue +tclient> quit +``` + +`status` reports the negotiated terminal options. `continue` resumes input to +the remote endpoint. These command names are processed locally and are never +sent as terminal text. `send` calls the library's existing `SendCommand` API. +It does not implement TCP urgent data or Telnet SYNCH. + +## Tracing + +Tracing is disabled by default. To enable it explicitly: + +```bash +./tclient --trace-file session.trace 192.0.2.10 23 +``` + +The CLI exclusively creates a new file with permission mode `0600`; it refuses +to overwrite an existing file. **Manual passwords, commands, and output may be +recorded.** Automatic-login redaction only covers the library's initial login +phase. A warning is printed when trace recording is enabled. Filesystem access +controls on non-Unix platforms follow that platform's file-permission behavior. + +## Exit codes + +| Code | Meaning | +| --- | --- | +| 0 | Normal remote close or local quit; help also succeeds | +| 1 | Connection, transport, terminal, output, or drain failure | +| 2 | Invalid arguments or configuration | +| 3 | Automatic-login failure or required password change | + +## Protocol boundaries + +The library decodes NVT `CR NUL` on receive unless the remote direction has +negotiated BINARY. BINARY transitions are applied in wire order, including +several transitions within one network read. `CR LF`, standalone NUL, and +binary data are preserved. Telnet IAC escaping still applies in both modes. + +`Client.Write` preserves its existing contract: the caller supplies application +bytes and line endings; the transport escapes IAC. It does not silently change +bare CR/LF, buffer a trailing CR, or rewrite binary streams. Only the CLI's TTY +input maps Enter keys. Callers that require NVT newlines must send complete +CR LF or CR NUL sequences themselves. + +SGA is accepted independently in both directions. NEW-ENVIRON exports only the +configured USER and respects requested names, ordering, undefined values, and +environment escaping. The legacy ENVIRON option also recognizes peers with +reversed VAR/VALUE control codes. No process environment is exported. + +This release does not implement active option toggles, complete half-duplex GA +turn-taking, local line editing, LINEMODE, TCP urgent/SYNCH, `.telnetrc`, Kerberos, +Telnet encryption, or a general Expect scripting language. A successful test +against a simulated endpoint does not certify every network-device firmware. + +Relevant specifications: [RFC 854](https://www.rfc-editor.org/rfc/rfc854), +[RFC 856](https://www.rfc-editor.org/rfc/rfc856), +[RFC 858](https://www.rfc-editor.org/rfc/rfc858), +[RFC 1571](https://www.rfc-editor.org/rfc/rfc1571), and +[RFC 1572](https://www.rfc-editor.org/rfc/rfc1572). + +## Old example entry point + +`_example/telnet_client` delegates to the same implementation as `cmd/tclient`. +It no longer maintains its own terminal, signal, password, or trace logic. +Its flags have changed: use positional `host [port]` instead of `-ip` and +`-port`; use `--auto-login --user ... --ask-pass` instead of `-username` and +`-password`. diff --git a/go.mod b/go.mod index f3af748..0564704 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,9 @@ module github.com/LeeEirc/tclientlib go 1.20 + +require ( + golang.org/x/sys v0.30.0 + golang.org/x/term v0.29.0 + golang.org/x/text v0.22.0 +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..4acb987 --- /dev/null +++ b/go.sum @@ -0,0 +1,6 @@ +golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= +golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.29.0 h1:L6pJp37ocefwRRtYPKSWOWzOtWSxVajvz2ldH/xi3iU= +golang.org/x/term v0.29.0/go.mod h1:6bl4lRlvVuDgSf3179VpIxBF0o10JUpXWOnI7nErv7s= +golang.org/x/text v0.22.0 h1:bofq7m3/HAFvbF51jz3Q9wLg3jkvSPuiZu/pD1XwgtM= +golang.org/x/text v0.22.0/go.mod h1:YRoo4H8PVmsu+E3Ou7cqLVH8oXWIHVoX0jqUWALQhfY= diff --git a/internal/cli/input.go b/internal/cli/input.go new file mode 100644 index 0000000..09ad2fa --- /dev/null +++ b/internal/cli/input.go @@ -0,0 +1,274 @@ +package cli + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "strings" + "unicode/utf8" + + "github.com/LeeEirc/tclientlib" + "golang.org/x/term" +) + +const escapeKey = 0x1d // Ctrl+] + +var errQuit = errors.New("local quit") + +type interactiveClient interface { + io.Writer + SendCommand(tclientlib.Command) error + TerminalMode() tclientlib.TerminalMode +} + +// terminalInput maps local keys, not arbitrary application data. A CR's LF or +// NUL follower is consumed even when it arrives in a later stdin read. This +// prevents a pasted CRLF from becoming two Enter keys. +type terminalInput struct { + client interactiveClient + local io.Writer + ending []byte + echo string + afterCR bool + commandMode bool + command []byte + tooLong bool +} + +func (t *terminalInput) feed(p []byte) error { + for _, b := range p { + if t.afterCR { + t.afterCR = false + if b == '\n' || b == 0 { + continue + } + } + if t.commandMode { + if err := t.commandByte(b); err != nil { + return err + } + continue + } + if b == escapeKey { + t.commandMode = true + t.command = t.command[:0] + t.tooLong = false + if _, err := io.WriteString(t.local, "\r\ntclient> "); err != nil { + return err + } + continue + } + data := []byte{b} + if b == '\r' || b == '\n' { + data = t.ending + t.afterCR = b == '\r' + } + if err := writeFull(t.client, data); err != nil { + return err + } + if t.localEcho() { + switch b { + case '\r', '\n': + data = []byte("\r\n") + case 8, 127: + data = []byte("\b \b") + } + if err := writeFull(t.local, data); err != nil { + return err + } + } + } + return nil +} + +func (t *terminalInput) localEcho() bool { + return t.echo == "on" || t.echo == "auto" && !t.client.TerminalMode().RemoteEcho +} + +func (t *terminalInput) commandByte(b byte) error { + switch b { + case escapeKey: + if len(t.command) == 0 && !t.tooLong { + t.commandMode = false + if _, err := io.WriteString(t.local, "\r\n"); err != nil { + return err + } + return writeFull(t.client, []byte{escapeKey}) + } + case '\r', '\n': + t.afterCR = b == '\r' + if _, err := io.WriteString(t.local, "\r\n"); err != nil { + return err + } + if t.tooLong { + if _, err := io.WriteString(t.local, "Command too long.\r\n"); err != nil { + return err + } + } else if err := t.executeCommand(); err != nil { + return err + } + t.command = t.command[:0] + t.tooLong = false + if t.commandMode { + _, err := io.WriteString(t.local, "tclient> ") + return err + } + case 3: // Ctrl+C cancels a local command; in session mode it is sent remotely. + t.commandMode = false + t.command = t.command[:0] + _, err := io.WriteString(t.local, "^C\r\n") + return err + case 8, 127: + if len(t.command) > 0 && !t.tooLong { + t.command = t.command[:len(t.command)-1] + _, err := io.WriteString(t.local, "\b \b") + return err + } + default: + // Commands are ASCII. Ignore terminal control sequences and cap storage. + if b >= 32 && b < 127 && !t.tooLong { + if len(t.command) == 1024 { + t.tooLong = true + return nil + } + t.command = append(t.command, b) + return writeFull(t.local, []byte{b}) + } + } + return nil +} + +func (t *terminalInput) executeCommand() error { + words := strings.Fields(string(t.command)) + if len(words) == 0 || len(words) == 1 && words[0] == "continue" { + t.commandMode = false + return nil + } + if len(words) == 1 { + switch words[0] { + case "quit": + return errQuit + case "help": + _, err := io.WriteString(t.local, "help | status | send brk/ip/ayt/nop | continue (or Enter) | quit\r\nCtrl+] twice sends a literal Ctrl+]. Input is character-at-a-time.\r\n") + return err + case "status": + m := t.client.TerminalMode() + _, err := fmt.Fprintf(t.local, "Remote echo: %t; local echo: %t; binary send/receive: %t/%t; SGA send/receive: %t/%t\r\n", m.RemoteEcho, t.localEcho(), m.SendBinary, m.ReceiveBinary, m.LocalSuppressGA, m.RemoteSuppressGA) + return err + } + } + if len(words) == 2 && words[0] == "send" { + commands := map[string]tclientlib.Command{ + "brk": tclientlib.CommandBreak, "ip": tclientlib.CommandInterruptProcess, + "ayt": tclientlib.CommandAreYouThere, "nop": tclientlib.CommandNoOperation, + } + if command, ok := commands[words[1]]; ok { + return t.client.SendCommand(command) + } + } + _, err := io.WriteString(t.local, "Unknown command. Use help.\r\n") + return err +} + +func copyInput(ctx context.Context, input *os.File, client interactiveClient, terminal *terminalInput) error { + buffer := make([]byte, 4096) + for { + n, err := readInput(ctx, input, buffer) + if n > 0 { + var writeErr error + if terminal != nil { + writeErr = terminal.feed(buffer[:n]) + } else { + writeErr = writeFull(client, buffer[:n]) + } + if writeErr != nil { + return writeErr + } + } + if err != nil { + if errors.Is(err, io.EOF) { + return nil + } + return err + } + if n == 0 { + return nil + } + } +} + +func writeFull(writer io.Writer, data []byte) error { + for len(data) > 0 { + n, err := writer.Write(data) + if err != nil { + return err + } + if n == 0 { + return io.ErrShortWrite + } + data = data[n:] + } + return nil +} + +func readPassword(ctx context.Context, input *os.File, output io.Writer) (password string, afterCR bool, retErr error) { + fd := int(input.Fd()) + state, err := term.MakeRaw(fd) + if err != nil { + return "", false, err + } + defer func() { + if err := term.Restore(fd, state); retErr == nil { + retErr = err + } + _, _ = io.WriteString(output, "\r\n") + }() + if _, err := io.WriteString(output, "Password: "); err != nil { + return "", false, err + } + var secret []byte + defer func() { + for i := range secret { + secret[i] = 0 + } + }() + var single [1]byte + for { + n, err := readInput(ctx, input, single[:]) + if err != nil { + return "", false, err + } + if n == 0 { + return "", false, io.EOF + } + switch single[0] { + case '\r', '\n': + return string(secret), single[0] == '\r', nil + case 3: + return "", false, context.Canceled + case 4: + return "", false, io.EOF + case 8, 127: + if len(secret) > 0 { + _, size := utf8.DecodeLastRune(secret) + start := len(secret) - size + for i := start; i < len(secret); i++ { + secret[i] = 0 + } + secret = secret[:start] + } + case 21: // Ctrl+U clears the hidden line. + for i := range secret { + secret[i] = 0 + } + secret = secret[:0] + default: + if len(secret) == 64*1024 { + return "", false, fmt.Errorf("password is too long") + } + secret = append(secret, single[0]) + } + } +} diff --git a/internal/cli/input_test.go b/internal/cli/input_test.go new file mode 100644 index 0000000..98eb386 --- /dev/null +++ b/internal/cli/input_test.go @@ -0,0 +1,138 @@ +package cli + +import ( + "bytes" + "errors" + "io" + "strings" + "testing" + + "github.com/LeeEirc/tclientlib" +) + +type fakeClient struct { + bytes.Buffer + mode tclientlib.TerminalMode + commands []tclientlib.Command +} + +func (c *fakeClient) TerminalMode() tclientlib.TerminalMode { return c.mode } +func (c *fakeClient) SendCommand(command tclientlib.Command) error { + c.commands = append(c.commands, command) + return nil +} + +func TestTerminalEnterAtEverySplit(t *testing.T) { + input := []byte("a\r\nb\rc\nd\r\x00e\r\r\n") + for _, ending := range []string{"crlf", "crnul", "cr", "lf"} { + t.Run(ending, func(t *testing.T) { + _, eol, _ := (options{lineEnding: ending}).ending() + want := strings.Join([]string{"a", "b", "c", "d", "e", "", ""}, string(eol)) + for split := 0; split <= len(input); split++ { + client := new(fakeClient) + terminal := terminalInput{client: client, local: io.Discard, ending: eol, echo: "off"} + if err := terminal.feed(input[:split]); err != nil { + t.Fatal(err) + } + if err := terminal.feed(input[split:]); err != nil { + t.Fatal(err) + } + if got := client.String(); got != want { + t.Fatalf("split %d: got %q, want %q", split, got, want) + } + } + }) + } +} + +func TestTerminalCommandsAreLocal(t *testing.T) { + client := new(fakeClient) + var local bytes.Buffer + terminal := terminalInput{client: client, local: &local, ending: []byte("\r\n"), echo: "off"} + input := []byte("a\x1dhelp\r\nstatus\rsend brk\rsend ip\rsend ayt\rsend nop\rcontinue\r\nb\x1d\x1dc\x1dquit\rignored") + for _, b := range input { + err := terminal.feed([]byte{b}) + if errors.Is(err, errQuit) { + break + } + if err != nil { + t.Fatal(err) + } + } + if got, want := client.String(), "ab\x1dc"; got != want { + t.Fatalf("wire got %q, want %q", got, want) + } + wantCommands := []tclientlib.Command{tclientlib.CommandBreak, tclientlib.CommandInterruptProcess, tclientlib.CommandAreYouThere, tclientlib.CommandNoOperation} + if len(client.commands) != len(wantCommands) { + t.Fatalf("commands = %v", client.commands) + } + for i := range wantCommands { + if client.commands[i] != wantCommands[i] { + t.Fatalf("commands = %v", client.commands) + } + } + if !strings.Contains(local.String(), "Remote echo:") || !strings.Contains(local.String(), "Ctrl+] twice") { + t.Fatalf("missing local help/status: %q", local.String()) + } +} + +func TestTerminalEchoFollowsNegotiatedMode(t *testing.T) { + client := new(fakeClient) + var local bytes.Buffer + terminal := terminalInput{client: client, local: &local, ending: []byte("\r\n"), echo: "auto"} + if err := terminal.feed([]byte("a")); err != nil { + t.Fatal(err) + } + client.mode.RemoteEcho = true + if err := terminal.feed([]byte("b\r")); err != nil { + t.Fatal(err) + } + client.mode.RemoteEcho = false + if err := terminal.feed([]byte("c\r")); err != nil { + t.Fatal(err) + } + if got, want := local.String(), "ac\r\n"; got != want { + t.Fatalf("echo got %q, want %q", got, want) + } + if got, want := client.String(), "ab\r\nc\r\n"; got != want { + t.Fatalf("wire got %q, want %q", got, want) + } +} + +func TestTerminalDiscardsOverlongCommandUntilNewline(t *testing.T) { + client := new(fakeClient) + var local bytes.Buffer + terminal := terminalInput{client: client, local: &local, ending: []byte("\r\n"), echo: "off"} + if err := terminal.feed([]byte("\x1d" + strings.Repeat("x", 4096) + "\rcontinue\rhello")); err != nil { + t.Fatal(err) + } + if got := client.String(); got != "hello" { + t.Fatalf("command escaped to wire: %q", got) + } + if !strings.Contains(local.String(), "Command too long.") { + t.Fatal("missing command length diagnostic") + } +} + +func TestPasswordCRFollowerDoesNotBecomeEnter(t *testing.T) { + client := new(fakeClient) + terminal := terminalInput{client: client, local: io.Discard, ending: []byte("\r\n"), echo: "off", afterCR: true} + if err := terminal.feed([]byte("\nshow version\r")); err != nil { + t.Fatal(err) + } + if got, want := client.String(), "show version\r\n"; got != want { + t.Fatalf("got %q, want %q", got, want) + } +} + +type brokenWriter struct{ err error } + +func (w brokenWriter) Write([]byte) (int, error) { return 0, w.err } + +func TestTerminalReportsLocalOutputError(t *testing.T) { + want := errors.New("output failed") + terminal := terminalInput{client: new(fakeClient), local: brokenWriter{want}, ending: []byte("\r\n"), echo: "on"} + if err := terminal.feed([]byte{escapeKey}); !errors.Is(err, want) { + t.Fatalf("got %v, want output error", err) + } +} diff --git a/internal/cli/options.go b/internal/cli/options.go new file mode 100644 index 0000000..227f495 --- /dev/null +++ b/internal/cli/options.go @@ -0,0 +1,135 @@ +// Package cli implements the interactive tclient command. Telnet framing and +// login stay in the root package; this package owns only local terminal policy. +package cli + +import ( + "flag" + "fmt" + "io" + "net" + "os" + "strconv" + "strings" + "time" + + "github.com/LeeEirc/tclientlib" + "golang.org/x/text/encoding" + "golang.org/x/text/encoding/simplifiedchinese" + "golang.org/x/text/encoding/traditionalchinese" +) + +type options struct { + address string + timeout time.Duration + drainTimeout time.Duration + closeWrite bool + autoLogin bool + user string + askPass bool + terminal string + charset string + lineEnding string + echo string + traceFile string +} + +func parseOptions(args []string, output io.Writer) (options, error) { + var o options + f := flag.NewFlagSet("tclient", flag.ContinueOnError) + f.SetOutput(output) + f.DurationVar(&o.timeout, "timeout", 15*time.Second, "connection and automatic-login timeout") + f.DurationVar(&o.drainTimeout, "drain-timeout", 5*time.Second, "maximum wait for remote output after stdin EOF") + f.BoolVar(&o.closeWrite, "close-write", false, "half-close TCP on stdin EOF (disables further Telnet replies)") + f.BoolVar(&o.autoLogin, "auto-login", false, "use prompt-based automatic login") + f.StringVar(&o.user, "user", "", "automatic-login username (requires --auto-login)") + f.BoolVar(&o.askPass, "ask-pass", false, "read an automatic-login password without echo (requires a terminal)") + terminal := os.Getenv("TERM") + if terminal == "" { + terminal = "xterm" + } + f.StringVar(&o.terminal, "term", terminal, "terminal type sent to the server") + f.StringVar(&o.charset, "charset", "utf-8", "remote charset: utf-8, gbk, gb18030, big5") + f.StringVar(&o.lineEnding, "line-ending", "crlf", "TTY Enter and automatic-login terminator: crlf, crnul, cr, lf") + f.StringVar(&o.echo, "echo", "auto", "local character echo: auto, on, off (TTY only)") + f.StringVar(&o.traceFile, "trace-file", "", "create a private diagnostic trace; may contain passwords and output") + f.Usage = func() { + fmt.Fprintln(output, "Usage: tclient [options] host [port]") + fmt.Fprintln(output, "Defaults to manual login and port 23. Options precede the host.") + fmt.Fprintln(output, "Press Ctrl+] for local commands; use help to list them.") + f.PrintDefaults() + } + if err := f.Parse(args); err != nil { + return o, err + } + if f.NArg() < 1 || f.NArg() > 2 { + return o, fmt.Errorf("expected host [port]; use --help for usage") + } + if o.timeout <= 0 || o.drainTimeout <= 0 { + return o, fmt.Errorf("timeouts must be positive") + } + if !o.autoLogin && (o.user != "" || o.askPass) { + return o, fmt.Errorf("--user and --ask-pass require --auto-login") + } + if o.autoLogin && o.user == "" && !o.askPass { + return o, fmt.Errorf("--auto-login requires --user or --ask-pass") + } + if _, err := o.encoding(); err != nil { + return o, err + } + if _, _, err := o.ending(); err != nil { + return o, err + } + if o.echo != "auto" && o.echo != "on" && o.echo != "off" { + return o, fmt.Errorf("invalid echo mode %q", o.echo) + } + if o.terminal == "" { + return o, fmt.Errorf("terminal type must not be empty") + } + host := f.Arg(0) + if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { + host = host[1 : len(host)-1] + } + if host == "" { + return o, fmt.Errorf("host must not be empty") + } + port := "23" + if f.NArg() == 2 { + port = f.Arg(1) + } + portNumber, err := strconv.Atoi(port) + if err != nil || portNumber < 1 || portNumber > 65535 { + return o, fmt.Errorf("port must be a number from 1 to 65535") + } + o.address = net.JoinHostPort(host, port) + return o, nil +} + +func (o options) encoding() (encoding.Encoding, error) { + switch strings.ToLower(o.charset) { + case "utf-8", "utf8": + return nil, nil + case "gbk": + return simplifiedchinese.GBK, nil + case "gb18030": + return simplifiedchinese.GB18030, nil + case "big5": + return traditionalchinese.Big5, nil + default: + return nil, fmt.Errorf("unsupported charset %q", o.charset) + } +} + +func (o options) ending() (tclientlib.LineEnding, []byte, error) { + switch o.lineEnding { + case "crlf": + return tclientlib.LineEndingCRLF, []byte{'\r', '\n'}, nil + case "crnul": + return tclientlib.LineEndingCRNUL, []byte{'\r', 0}, nil + case "cr": + return tclientlib.LineEndingCR, []byte{'\r'}, nil + case "lf": + return tclientlib.LineEndingLF, []byte{'\n'}, nil + default: + return 0, nil, fmt.Errorf("unsupported line ending %q", o.lineEnding) + } +} diff --git a/internal/cli/run.go b/internal/cli/run.go new file mode 100644 index 0000000..2730de9 --- /dev/null +++ b/internal/cli/run.go @@ -0,0 +1,307 @@ +package cli + +import ( + "context" + "errors" + "flag" + "fmt" + "io" + "net" + "os" + "os/signal" + "reflect" + "sync" + "time" + + "github.com/LeeEirc/tclientlib" + "golang.org/x/term" + "golang.org/x/text/transform" +) + +// Run runs tclient and returns an exit code after restoring the local terminal. +// Codes are 0 for normal completion, 1 for connection or I/O errors, 2 for +// invalid arguments, and 3 for automatic-login failures. It does not exit the +// process, so example commands can share the same implementation. +func Run(args []string, stdin *os.File, stdout, stderr io.Writer) int { + o, err := parseOptions(args, stderr) + if errors.Is(err, flag.ErrHelp) { + return 0 + } + if err != nil { + fmt.Fprintln(stderr, "tclient:", err) + return 2 + } + if o.askPass && !term.IsTerminal(int(stdin.Fd())) { + fmt.Fprintln(stderr, "tclient: --ask-pass requires a terminal on stdin") + return 2 + } + if term.IsTerminal(int(stdin.Fd())) && !supportsTerminal() { + fmt.Fprintln(stderr, "tclient: interactive terminals are currently supported on Unix; redirected input remains available") + return 2 + } + ctx, stop := signal.NotifyContext(context.Background(), interruptSignals()...) + defer stop() + err = run(ctx, o, stdin, stdout, stderr) + if err == nil { + return 0 + } + // A full diagnostic pipe must not undo cancellation after the terminal has + // already been restored. Reporting is best effort and bounded on Unix files. + if ctx.Err() == nil { + diagnosticCtx, cancel := context.WithTimeout(ctx, 250*time.Millisecond) + _, diagnostics, restore, outputErr := terminalOutputs(diagnosticCtx, io.Discard, stderr) + if outputErr == nil { + fmt.Fprintln(diagnostics, "tclient:", err) + _ = restore() + } + cancel() + } + if errors.Is(err, tclientlib.ErrLoginFailed) || errors.Is(err, tclientlib.ErrPasswordChangeRequired) || errors.Is(err, tclientlib.ErrLoginTimeout) { + return 3 + } + return 1 +} + +func run(ctx context.Context, o options, stdin *os.File, stdout, stderr io.Writer) (retErr error) { + // File streams are independently cancellable and must not share a lock: + // a full stdout pipe must not prevent a local quit command on stderr. + sharedOutput := sameMemoryWriter(stdout, stderr) + outputCtx, cancelOutput := context.WithCancel(ctx) + defer cancelOutput() + stdout, stderr, restoreOutput, err := terminalOutputs(outputCtx, stdout, stderr) + if err != nil { + return fmt.Errorf("prepare output: %w", err) + } + defer func() { + if err := restoreOutput(); retErr == nil { + retErr = err + } + }() + outputMu, localMu := new(sync.Mutex), new(sync.Mutex) + if sharedOutput { + localMu = outputMu + } + stdout = &lockedWriter{mu: outputMu, writer: stdout} + stderr = &lockedWriter{mu: localMu, writer: stderr} + fd := int(stdin.Fd()) + isTTY := term.IsTerminal(fd) + password := "" + passwordCR := false + if o.askPass { + var err error + password, passwordCR, err = readPassword(ctx, stdin, stderr) + if err != nil { + return fmt.Errorf("read password: %w", err) + } + if password == "" && o.user == "" { + return fmt.Errorf("automatic login requires a username or a nonempty password") + } + } + var trace io.Writer + if o.traceFile != "" { + file, err := openTrace(o.traceFile) + if err != nil { + return fmt.Errorf("create trace: %w", err) + } + defer file.Close() + trace = file + if _, err := fmt.Fprintln(stderr, "Warning: trace may record manually entered passwords, commands, and remote output:", o.traceFile); err != nil { + return err + } + } + conn, err := (&net.Dialer{Timeout: o.timeout}).DialContext(ctx, "tcp", o.address) + if err != nil { + return err + } + defer conn.Close() + // Automatic login can be blocked in Read/Write. Closing the connection on + // cancellation interrupts both directions, including before the CLI is raw. + watchDone := make(chan struct{}) + defer close(watchDone) + go func() { + select { + case <-ctx.Done(): + _ = conn.Close() + case <-watchDone: + } + }() + lineEnding, ending, _ := o.ending() + config := &tclientlib.ClientConfig{ + Timeout: o.timeout, TerminalType: o.terminal, LineEnding: lineEnding, Trace: trace, + } + if o.autoLogin { + config.User, config.Password = o.user, password + } + if isTTY { + if width, height, err := term.GetSize(fd); err == nil { + config.Width, config.Height = width, height + } + } + flush := func() error { return nil } + if encoding, _ := o.encoding(); encoding != nil { + config.WrapStream = func(reader io.Reader, writer io.Writer) (io.Reader, io.Writer) { + encoded := &closingWriter{writer: transform.NewWriter(writer, encoding.NewEncoder())} + flush = encoded.Close + return transform.NewReader(reader, encoding.NewDecoder()), encoded + } + } + client, err := tclientlib.NewClientConn(conn, config) + if err != nil { + return err + } + defer client.Close() + var terminal *terminalInput + if isTTY { + state, err := term.MakeRaw(fd) + if err != nil { + return fmt.Errorf("enter terminal raw mode: %w", err) + } + defer func() { + if err := term.Restore(fd, state); retErr == nil { + retErr = err + } + }() + terminal = &terminalInput{client: client, local: stderr, ending: ending, echo: o.echo, afterCR: passwordCR} + } + return runSession(ctx, o, stdin, stdout, conn, client, terminal, flush, cancelOutput) +} + +func runSession(ctx context.Context, o options, stdin *os.File, output io.Writer, conn net.Conn, client *tclientlib.Client, terminal *terminalInput, flush func() error, cancelOutput context.CancelFunc) error { + sessionCtx, cancel := context.WithCancel(ctx) + sent := make(chan error, 1) + received := make(chan error, 1) + inputStopped := make(chan struct{}) + outputStopped := make(chan struct{}) + go func() { + defer close(inputStopped) + sent <- copyInput(sessionCtx, stdin, client, terminal) + }() + go func() { + defer close(outputStopped) + _, err := io.Copy(output, client) + received <- err + }() + defer func() { + cancel() + cancelOutput() + // Close the socket before waiting or flushing a charset writer: either + // input or negotiation may currently be blocked writing to the peer. + _ = conn.Close() + stopInput(stdin) + <-inputStopped + <-outputStopped + }() + var resize <-chan os.Signal + if terminal != nil { + var stop func() + resize, stop = watchResize() + defer stop() + } + var timer *time.Timer + var drained <-chan time.Time + defer func() { + if timer != nil { + timer.Stop() + } + }() + for { + select { + case <-ctx.Done(): + return ctx.Err() + case err := <-received: + if err != nil { + return fmt.Errorf("read remote output: %w", err) + } + return nil + case err := <-sent: + if errors.Is(err, errQuit) { + return nil + } + if err != nil { + return fmt.Errorf("send input: %w", err) + } + // Keep the socket's write side available for negotiation while + // draining. Some endpoints require TCP EOF to produce output; the + // explicit close-write option trades later Telnet replies for that. + deadline := time.Now().Add(o.drainTimeout) + if err := conn.SetDeadline(deadline); err != nil { + return fmt.Errorf("set EOF drain deadline: %w", err) + } + if err := flush(); err != nil { + return fmt.Errorf("flush input: %w", err) + } + if o.closeWrite { + halfCloser, ok := conn.(interface{ CloseWrite() error }) + if !ok { + return fmt.Errorf("connection does not support closing the input direction") + } + if err := halfCloser.CloseWrite(); err != nil { + return fmt.Errorf("close input direction: %w", err) + } + } + sent = nil + timer = time.NewTimer(time.Until(deadline)) + drained = timer.C + case <-drained: + return fmt.Errorf("remote output did not finish within %s after stdin EOF", o.drainTimeout) + case <-resize: + width, height, err := term.GetSize(int(stdin.Fd())) + if err == nil { + if err := client.WindowChange(height, width); err != nil { + return fmt.Errorf("resize terminal: %w", err) + } + } + } + } +} + +func openTrace(path string) (*os.File, error) { + return os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600) +} + +type lockedWriter struct { + mu *sync.Mutex + writer io.Writer +} + +func sameMemoryWriter(a, b io.Writer) bool { + if _, ok := a.(*os.File); ok { + return false + } + typ := reflect.TypeOf(a) + return typ != nil && typ == reflect.TypeOf(b) && typ.Comparable() && a == b +} + +func (w *lockedWriter) Write(p []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + return w.writer.Write(p) +} + +// closingWriter lets stdin EOF flush a transform.Writer before TCP CloseWrite; +// the library's later Client.Close must not flush the transformer a second time. +type closingWriter struct { + mu sync.Mutex + writer *transform.Writer + closed bool + err error +} + +func (w *closingWriter) Write(p []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + if w.closed { + return 0, io.ErrClosedPipe + } + return w.writer.Write(p) +} + +func (w *closingWriter) Close() error { + w.mu.Lock() + defer w.mu.Unlock() + if !w.closed { + w.closed = true + w.err = w.writer.Close() + } + return w.err +} diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go new file mode 100644 index 0000000..c3d209c --- /dev/null +++ b/internal/cli/run_test.go @@ -0,0 +1,294 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "io" + "net" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/LeeEirc/tclientlib" + "golang.org/x/text/encoding/simplifiedchinese" +) + +func TestOptionsRejectAmbiguousCredentials(t *testing.T) { + for _, args := range [][]string{ + {"--user", "admin", "localhost"}, {"--ask-pass", "localhost"}, + {"--auto-login", "localhost"}, {"--password", "secret", "localhost"}, + {"--line-ending", "bogus", "localhost"}, {"--charset", "bogus", "localhost"}, + {"--echo", "bogus", "localhost"}, {"--timeout", "0", "localhost"}, + {"--drain-timeout", "-1s", "localhost"}, {"localhost", "65536"}, + } { + if _, err := parseOptions(args, io.Discard); err == nil { + t.Errorf("accepted %q", args) + } + } + for _, host := range []string{"::1", "[::1]"} { + o, err := parseOptions([]string{host, "2323"}, io.Discard) + if err != nil || o.address != "[::1]:2323" { + t.Fatalf("host %q: %s, %v", host, o.address, err) + } + } +} + +func testInput(t *testing.T, data []byte) *os.File { + t.Helper() + file, err := os.CreateTemp(t.TempDir(), "stdin") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { file.Close() }) + if _, err := file.Write(data); err != nil { + t.Fatal(err) + } + if _, err := file.Seek(0, io.SeekStart); err != nil { + t.Fatal(err) + } + return file +} + +func testServer(t *testing.T, serve func(net.Conn)) (host, port string) { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + done := make(chan struct{}) + go func() { + defer close(done) + conn, err := listener.Accept() + if err != nil { + return + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(3 * time.Second)) + serve(conn) + }() + t.Cleanup(func() { + listener.Close() + select { + case <-done: + case <-time.After(4 * time.Second): + t.Error("test server did not stop") + } + }) + host, port, err = net.SplitHostPort(listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + return host, port +} + +func TestPipedInputPreservesBytesAndDrainsFinalOutput(t *testing.T) { + payload := []byte("show\r\n\x1dhelp\r\x00\xff") + wire := make(chan []byte, 1) + host, port := testServer(t, func(conn net.Conn) { + data, err := io.ReadAll(conn) + if err != nil { + t.Errorf("read stdin: %v", err) + } + wire <- data + _, _ = conn.Write([]byte("final output\r\n")) + }) + var output, diagnostics bytes.Buffer + code := Run([]string{"--close-write", "--line-ending", "lf", host, port}, testInput(t, payload), &output, &diagnostics) + if code != 0 { + t.Fatalf("exit %d: %s", code, diagnostics.String()) + } + if got, want := <-wire, append(append([]byte(nil), payload...), 255); !bytes.Equal(got, want) { + t.Fatalf("wire got %q, want %q", got, want) + } + if got, want := output.String(), "final output\r\n"; got != want { + t.Fatalf("output got %q, want %q", got, want) + } +} + +func TestCharsetRoundTripAndFlushAtEOF(t *testing.T) { + text := "你好\r\n" + encoded, err := simplifiedchinese.GBK.NewEncoder().Bytes([]byte(text)) + if err != nil { + t.Fatal(err) + } + wire := make(chan []byte, 1) + host, port := testServer(t, func(conn net.Conn) { + data, _ := io.ReadAll(conn) + wire <- data + _, _ = conn.Write(encoded) + }) + var output, diagnostics bytes.Buffer + if code := Run([]string{"--close-write", "--charset", "gbk", host, port}, testInput(t, []byte(text)), &output, &diagnostics); code != 0 { + t.Fatalf("exit %d: %s", code, diagnostics.String()) + } + if got := <-wire; !bytes.Equal(got, encoded) { + t.Fatalf("wire got %x, want %x", got, encoded) + } + if got := output.String(); got != text { + t.Fatalf("output got %q, want %q", got, text) + } +} + +func TestRemoteEOFCancelsIdleInput(t *testing.T) { + input, writer, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + defer input.Close() + defer writer.Close() + host, port := testServer(t, func(conn net.Conn) { _, _ = conn.Write([]byte("bye")) }) + var output, diagnostics bytes.Buffer + start := time.Now() + if code := Run([]string{host, port}, input, &output, &diagnostics); code != 0 { + t.Fatalf("exit %d: %s", code, diagnostics.String()) + } + if time.Since(start) > time.Second || output.String() != "bye" { + t.Fatalf("slow or incomplete remote EOF: %s, %q", time.Since(start), output.String()) + } + if runtime.GOOS == "linux" || runtime.GOOS == "darwin" { + // Ending a session must not leave a goroutine holding the user's stdin. + if _, err := writer.Write([]byte{'x'}); err != nil { + t.Fatal(err) + } + var one [1]byte + if n, err := input.Read(one[:]); err != nil || n != 1 || one[0] != 'x' { + t.Fatalf("stdin unusable after remote EOF: %d %v %q", n, err, one) + } + } +} + +func TestEOFDrainHasDeadline(t *testing.T) { + finish := make(chan struct{}) + defer close(finish) + host, port := testServer(t, func(conn net.Conn) { + var first [1]byte + _, _ = conn.Read(first[:]) + <-finish + }) + var diagnostics bytes.Buffer + start := time.Now() + code := Run([]string{"--drain-timeout", "40ms", host, port}, testInput(t, []byte("x")), io.Discard, &diagnostics) + if code != 1 || time.Since(start) > time.Second { + t.Fatalf("drain did not time out: code=%d duration=%s diagnostic=%s", code, time.Since(start), diagnostics.String()) + } +} + +func TestDefaultEOFStillAnswersNegotiation(t *testing.T) { + flushed := make(chan struct{}) + response := make(chan []byte, 1) + host, port := testServer(t, func(conn net.Conn) { + <-flushed + _, _ = conn.Write([]byte{255, 251, 1}) // WILL ECHO, after stdin is drained. + var reply [3]byte + n, _ := io.ReadFull(conn, reply[:]) + response <- reply[:n] + _, _ = conn.Write([]byte("final output")) + }) + o, err := parseOptions([]string{host, port}, io.Discard) + if err != nil { + t.Fatal(err) + } + conn, err := net.Dial("tcp", o.address) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + client, err := tclientlib.NewClientConn(conn, &tclientlib.ClientConfig{}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + var output bytes.Buffer + flush := func() error { close(flushed); return nil } + if err := runSession(context.Background(), o, testInput(t, nil), &output, conn, client, nil, flush, func() {}); err != nil { + t.Fatal(err) + } + if got := <-response; !bytes.Equal(got, []byte{255, 253, 1}) { + t.Fatalf("negotiation after EOF: %x", got) + } + if got := output.String(); got != "final output" { + t.Fatalf("output after negotiation = %q", got) + } +} + +func TestOutputFailureIsReported(t *testing.T) { + input, writer, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + defer input.Close() + defer writer.Close() + host, port := testServer(t, func(conn net.Conn) { _, _ = conn.Write([]byte("hello")) }) + var diagnostics bytes.Buffer + code := Run([]string{host, port}, input, brokenWriter{errors.New("disk full")}, &diagnostics) + if code != 1 || !strings.Contains(diagnostics.String(), "disk full") { + t.Fatalf("exit %d: %s", code, diagnostics.String()) + } +} + +func TestAutomaticFailureExitCode(t *testing.T) { + host, port := testServer(t, func(conn net.Conn) { _, _ = conn.Write([]byte("Login incorrect\r\n")) }) + var diagnostics bytes.Buffer + code := Run([]string{"--auto-login", "--user", "admin", host, port}, testInput(t, nil), io.Discard, &diagnostics) + if code != 3 { + t.Fatalf("exit %d: %s", code, diagnostics.String()) + } +} + +func TestRunCancellationUnblocksIdleInputAndRemote(t *testing.T) { + input, writer, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + defer input.Close() + defer writer.Close() + connected := make(chan struct{}) + host, port := testServer(t, func(conn net.Conn) { + close(connected) + _, _ = io.Copy(io.Discard, conn) + }) + o, _ := parseOptions([]string{host, port}, io.Discard) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := make(chan error, 1) + go func() { done <- run(ctx, o, input, io.Discard, io.Discard) }() + <-connected + cancel() + select { + case err := <-done: + if err == nil { + t.Fatal("cancellation was not reported") + } + case <-time.After(time.Second): + t.Fatal("cancellation did not stop the session") + } +} + +func TestTraceIsPrivateAndNeverOverwritten(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.trace") + file, err := openTrace(path) + if err != nil { + t.Fatal(err) + } + _, _ = io.WriteString(file, "keep") + file.Close() + if file, err := openTrace(path); err == nil { + file.Close() + t.Fatal("existing trace was overwritten") + } + data, err := os.ReadFile(path) + if err != nil || string(data) != "keep" { + t.Fatalf("trace contents: %q, %v", data, err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if runtime.GOOS != "windows" && info.Mode().Perm() != 0600 { + t.Fatalf("trace permissions = %o", info.Mode().Perm()) + } +} diff --git a/internal/cli/terminal_other.go b/internal/cli/terminal_other.go new file mode 100644 index 0000000..ce721bd --- /dev/null +++ b/internal/cli/terminal_other.go @@ -0,0 +1,34 @@ +//go:build !aix && !darwin && !dragonfly && !freebsd && !linux && !netbsd && !openbsd && !solaris + +package cli + +import ( + "context" + "io" + "os" + "time" +) + +func supportsTerminal() bool { return false } + +func interruptSignals() []os.Signal { return []os.Signal{os.Interrupt} } + +func watchResize() (<-chan os.Signal, func()) { return nil, func() {} } + +// Other platforms use the runtime's descriptor cancellation on Close. Unix +// uses poll instead, allowing it to leave the caller's stdin descriptor open. +func readInput(ctx context.Context, input *os.File, p []byte) (int, error) { + if err := ctx.Err(); err != nil { + return 0, err + } + return input.Read(p) +} + +func stopInput(input *os.File) { + _ = input.SetReadDeadline(time.Now()) + _ = input.Close() +} + +func terminalOutputs(_ context.Context, stdout, stderr io.Writer) (io.Writer, io.Writer, func() error, error) { + return stdout, stderr, func() error { return nil }, nil +} diff --git a/internal/cli/terminal_unix.go b/internal/cli/terminal_unix.go new file mode 100644 index 0000000..f71703b --- /dev/null +++ b/internal/cli/terminal_unix.go @@ -0,0 +1,143 @@ +//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris + +package cli + +import ( + "context" + "io" + "os" + "os/signal" + "syscall" + + "golang.org/x/sys/unix" +) + +func supportsTerminal() bool { return true } + +func interruptSignals() []os.Signal { + // Go otherwise exits immediately on a broken stdout/stderr pipe, bypassing + // terminal restoration. Catch SIGPIPE so the write error follows cleanup. + return []os.Signal{os.Interrupt, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGPIPE} +} + +func watchResize() (<-chan os.Signal, func()) { + ch := make(chan os.Signal, 1) + signal.Notify(ch, syscall.SIGWINCH) + return ch, func() { signal.Stop(ch) } +} + +func readInput(ctx context.Context, input *os.File, p []byte) (int, error) { + fd := int(input.Fd()) + for { + if err := ctx.Err(); err != nil { + return 0, err + } + poll := []unix.PollFd{{Fd: int32(fd), Events: unix.POLLIN}} + _, err := unix.Poll(poll, 100) + if err == unix.EINTR { + continue + } + if err != nil { + return 0, err + } + if poll[0].Revents == 0 { + continue + } + if poll[0].Revents&unix.POLLNVAL != 0 { + return 0, os.ErrClosed + } + n, err := unix.Read(fd, p) + if err == unix.EINTR || err == unix.EAGAIN { + continue + } + return n, err + } +} + +// Poll checks cancellation without closing or changing the caller's stdin. +func stopInput(*os.File) {} + +type descriptorFlags struct { + fd uintptr + flags int +} + +// A stdout pipe can remain open while its reader stops consuming data. Plain +// File.Write then cannot be interrupted by closing the Telnet socket. Use +// nonblocking writes plus bounded poll so signals and session cancellation also +// interrupt local output. Snapshot both descriptors before changing either: +// stdout and stderr may refer to the same open-file description. +func terminalOutputs(ctx context.Context, stdout, stderr io.Writer) (io.Writer, io.Writer, func() error, error) { + writers := []io.Writer{stdout, stderr} + var saved []descriptorFlags + seen := make(map[uintptr]bool) + for i, writer := range writers { + file, ok := writer.(*os.File) + if !ok { + continue + } + fd := file.Fd() + if !seen[fd] { + flags, err := unix.FcntlInt(fd, unix.F_GETFL, 0) + if err != nil { + return nil, nil, nil, err + } + saved = append(saved, descriptorFlags{fd: fd, flags: flags}) + seen[fd] = true + } + writers[i] = &pollWriter{ctx: ctx, fd: int(fd)} + } + restore := func() error { + var first error + for i := len(saved) - 1; i >= 0; i-- { + entry := saved[i] + if _, err := unix.FcntlInt(entry.fd, unix.F_SETFL, entry.flags); err != nil && first == nil { + first = err + } + } + return first + } + for _, entry := range saved { + if _, err := unix.FcntlInt(entry.fd, unix.F_SETFL, entry.flags|unix.O_NONBLOCK); err != nil { + _ = restore() + return nil, nil, nil, err + } + } + return writers[0], writers[1], restore, nil +} + +type pollWriter struct { + ctx context.Context + fd int +} + +func (w *pollWriter) Write(p []byte) (int, error) { + total := 0 + for len(p) > 0 { + if err := w.ctx.Err(); err != nil { + return total, err + } + n, err := unix.Write(w.fd, p) + if n > 0 { + total += n + p = p[n:] + } + if err == unix.EINTR { + continue + } + if err != nil && err != unix.EAGAIN && err != unix.EWOULDBLOCK { + return total, err + } + if err == nil { + if n == 0 { + return total, io.ErrShortWrite + } + continue + } + _, err = unix.Poll([]unix.PollFd{{Fd: int32(w.fd), Events: unix.POLLOUT}}, 100) + if err != nil && err != unix.EINTR { + return total, err + } + } + return total, nil +} diff --git a/scripts/test_cli_pty.py b/scripts/test_cli_pty.py new file mode 100644 index 0000000..1f2f400 --- /dev/null +++ b/scripts/test_cli_pty.py @@ -0,0 +1,558 @@ +#!/usr/bin/env python3 +"""Black-box PTY/TCP regression tests for tclient (Python standard library only). + +Usage: go build -o /tmp/tclient ./cmd/tclient + python3 scripts/test_cli_pty.py /tmp/tclient + +Every server binds only to loopback and all child processes and sockets are +closed on failure. PTYs verify the real terminal behavior that pipe-only tests +cannot exercise. Linux is the CI target; /dev/full is the only Linux-specific +scenario and is skipped when the device is unavailable. +""" + +import argparse +import errno +import fcntl +import json +import os +import pty +import select +import signal +import socket +import struct +import subprocess +import tempfile +import termios +import threading +import time + + +IAC = b"\xff" +WILL = b"\xfb" +WONT = b"\xfc" +DO = b"\xfd" +DONT = b"\xfe" +ECHO = b"\x01" +NAWS = b"\x1f" + + +def eventually(predicate, message, timeout=4.0): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + result = predicate() + if result: + return result + time.sleep(0.01) + raise AssertionError(message) + + +class Peer: + def __init__(self): + self.listener = socket.socket() + self.listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + self.listener.bind(("127.0.0.1", 0)) + self.listener.listen(1) + self.listener.settimeout(0.1) + self.port = self.listener.getsockname()[1] + self.conn = None + self.received = bytearray() + self.eof = threading.Event() + self.connected = threading.Event() + self.errors = [] + self.stopped = threading.Event() + self.thread = threading.Thread(target=self._receive, daemon=True) + self.thread.start() + + def _receive(self): + try: + while not self.stopped.is_set(): + try: + self.conn, _ = self.listener.accept() + break + except socket.timeout: + continue + if self.conn is None: + return + self.conn.settimeout(0.1) + self.connected.set() + while not self.stopped.is_set(): + try: + data = self.conn.recv(4096) + except socket.timeout: + continue + if not data: + self.eof.set() + return + self.received.extend(data) + except OSError as exc: + if not self.stopped.is_set(): + self.errors.append(str(exc)) + + def send(self, data): + assert self.connected.wait(4), "client did not connect" + self.conn.sendall(data) + + def wait_bytes(self, data, timeout=4): + return eventually(lambda: data in bytes(self.received), + f"missing wire bytes {data!r}, received {bytes(self.received)!r}", timeout) + + def close_remote(self): + assert self.connected.wait(4) + self.conn.shutdown(socket.SHUT_WR) + + def close(self): + self.stopped.set() + if self.conn: + try: + self.conn.shutdown(socket.SHUT_RDWR) + except OSError: + pass + self.conn.close() + self.listener.close() + self.thread.join(timeout=2) + assert not self.thread.is_alive(), "server thread did not terminate" + + +class TTYClient: + def __init__(self, binary, port, extra=(), stdout=None): + self.master, self.slave = pty.openpty() + fcntl.ioctl(self.slave, termios.TIOCSWINSZ, struct.pack("HHHH", 24, 80, 0, 0)) + self.original = termios.tcgetattr(self.slave) + self.original_flags = fcntl.fcntl(self.slave, fcntl.F_GETFL) + self.output = bytearray() + self.process = subprocess.Popen( + [binary, *extra, "127.0.0.1", str(port)], stdin=self.slave, + stdout=self.slave if stdout is None else stdout, + stderr=self.slave, start_new_session=True) + + def read(self): + while select.select([self.master], [], [], 0)[0]: + try: + data = os.read(self.master, 65536) + except OSError as exc: + if exc.errno == errno.EIO: + break + raise + if not data: + break + self.output.extend(data) + return bytes(self.output) + + def wait_output(self, text): + return eventually(lambda: text in self.read(), + f"missing output {text!r}; got {self.read()!r}") + + def send(self, data): + os.write(self.master, data) + + def wait_raw(self): + eventually(lambda: not termios.tcgetattr(self.slave)[3] & termios.ICANON, + "client never entered raw/noncanonical mode") + + def wait_exit(self, code=None): + result = self.process.wait(timeout=5) + self.read() + assert termios.tcgetattr(self.slave) == self.original, "terminal was not restored" + assert fcntl.fcntl(self.slave, fcntl.F_GETFL) == self.original_flags, "terminal file flags were not restored" + if code is not None: + assert result == code, f"exit={result}, expected={code}; output={self.output!r}" + return result + + def quit(self): + self.send(b"\x1dquit\r") + return self.wait_exit(0) + + def close(self): + if self.process.poll() is None: + self.process.kill() + self.process.wait() + for stream in (self.process.stdout, self.process.stderr): + if stream is not None: + stream.close() + os.close(self.master) + os.close(self.slave) + + +def run_case(name, fn, results): + start = time.monotonic() + try: + detail = fn() + results.append({"case": name, "ok": True, "seconds": round(time.monotonic() - start, 3), + "detail": detail}) + except Exception as exc: + results.append({"case": name, "ok": False, "seconds": round(time.monotonic() - start, 3), + "error": f"{type(exc).__name__}: {exc}"}) + print(json.dumps(results[-1], ensure_ascii=False), flush=True) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("binary") + parser.add_argument("--out", help="optional JSON results file") + args = parser.parse_args() + binary = os.path.abspath(args.binary) + results = [] + + def failed_dial(): + reserve = socket.socket() + reserve.bind(("127.0.0.1", 0)) + port = reserve.getsockname()[1] + child = TTYClient(binary, port, ["--timeout", "100ms"]) + try: + code = child.wait_exit() + assert code != 0, "connection failure returned success" + return {"exit": code} + finally: + child.close() + reserve.close() + + def remote_eof(): + peer = Peer() + child = TTYClient(binary, peer.port) + try: + peer.send(b"REMOTE OUTPUT\r\n") + child.wait_raw() + child.wait_output(b"REMOTE OUTPUT") + peer.close_remote() + code = child.wait_exit(0) + return {"exit": code} + finally: + child.close() + peer.close() + + def escape_quit(): + peer = Peer() + child = TTYClient(binary, peer.port) + try: + peer.send(b"READY\r\n") + child.wait_output(b"READY") + child.wait_raw() + child.quit() + assert peer.eof.wait(3), "remote socket did not close" + assert bytes(peer.received) == b"", f"local command leaked: {bytes(peer.received)!r}" + return {"wire_hex": bytes(peer.received).hex()} + finally: + child.close() + peer.close() + + def terminated(): + peer = Peer() + child = TTYClient(binary, peer.port) + try: + peer.send(b"READY\r\n") + child.wait_raw() + child.process.send_signal(signal.SIGTERM) + code = child.wait_exit() + assert code != 0, "signal interruption returned success" + return {"exit": code} + finally: + child.close() + peer.close() + + def naws_resize(): + peer = Peer() + child = TTYClient(binary, peer.port) + try: + peer.send(IAC + DO + NAWS + b"READY\r\n") + peer.wait_bytes(IAC + WILL + NAWS) + peer.wait_bytes(IAC + b"\xfa" + NAWS + b"\x00\x50\x00\x18" + IAC + b"\xf0") + child.wait_raw() + fcntl.ioctl(child.slave, termios.TIOCSWINSZ, struct.pack("HHHH", 37, 111, 0, 0)) + child.process.send_signal(signal.SIGWINCH) + peer.wait_bytes(IAC + b"\xfa" + NAWS + b"\x00\x6f\x00\x25" + IAC + b"\xf0") + child.quit() + return {"wire_hex": bytes(peer.received).hex()} + finally: + child.close() + peer.close() + + def echo_transitions(): + peer = Peer() + child = TTYClient(binary, peer.port) + try: + peer.send(IAC + WILL + ECHO + b"REMOTE-ECHO\r\n") + peer.wait_bytes(IAC + DO + ECHO) + child.wait_output(b"REMOTE-ECHO") + child.wait_raw() + child.send(b"abc") + peer.wait_bytes(b"abc") + peer.send(IAC + WONT + ECHO + b"LOCAL-ECHO\r\n") + peer.wait_bytes(IAC + DONT + ECHO) + child.wait_output(b"LOCAL-ECHO") + child.send(b"xyz") + peer.wait_bytes(b"xyz") + child.wait_output(b"xyz") + child.quit() + assert b"abc" not in child.read(), f"remote echo duplicated locally: {child.read()!r}" + return {"wire_hex": bytes(peer.received).hex()} + finally: + child.close() + peer.close() + + def split_newlines(): + peer = Peer() + child = TTYClient(binary, peer.port, ["--echo", "off"]) + try: + peer.send(b"READY\r\n") + child.wait_output(b"READY") + child.wait_raw() + child.send(b"first\r") + peer.wait_bytes(b"first\r\n") + child.send(b"\nsecond\r") + peer.wait_bytes(b"second\r\n") + child.send(b"\x00third\n") + peer.wait_bytes(b"third\r\n") + child.quit() + want = b"first\r\nsecond\r\nthird\r\n" + assert bytes(peer.received) == want, f"wire={bytes(peer.received)!r}, want={want!r}" + return {"wire_hex": bytes(peer.received).hex()} + finally: + child.close() + peer.close() + + def pipe_half_close(): + peer = Peer() + child = subprocess.Popen([binary, "--close-write", "127.0.0.1", str(peer.port)], stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + data = b"show\r\nraw\xffbyte\r\x00tail\n" + try: + assert peer.connected.wait(3) + child.stdin.write(data) + child.stdin.close() + assert peer.eof.wait(3), "stdin EOF did not half-close TCP write" + expected = data.replace(b"\xff", b"\xff\xff") + assert bytes(peer.received) == expected, f"pipe input changed: {bytes(peer.received)!r}" + peer.send(b"\r\x00FINAL RESULT\r\n") + peer.close_remote() + code = child.wait(timeout=5) + output = child.stdout.read() + error = child.stderr.read() + assert code == 0, f"exit={code} stderr={error!r}" + assert output == b"\rFINAL RESULT\r\n", f"final output lost/changed: {output!r}" + return {"wire_hex": bytes(peer.received).hex(), "stdout_hex": output.hex(), "exit": code} + finally: + if child.poll() is None: + child.kill() + child.wait() + peer.close() + + def pipe_eof_keeps_negotiation(): + peer = Peer() + child = subprocess.Popen([binary, "127.0.0.1", str(peer.port)], stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + try: + assert peer.connected.wait(3) + child.stdin.write(b"show status\n") + child.stdin.close() + peer.wait_bytes(b"show status\n") + # An EOF notification is never expected in default mode. Waiting + # on the event allows the child to consume the input EOF before + # the server starts a later protocol negotiation. + assert not peer.eof.wait(0.2), "default stdin EOF closed the Telnet write direction" + peer.send(IAC + WILL + ECHO + b"FIRST OUTPUT\r\n") + peer.wait_bytes(IAC + DO + ECHO) + peer.send(b"FINAL OUTPUT\r\n") + peer.close_remote() + code = child.wait(timeout=5) + output = child.stdout.read() + error = child.stderr.read() + assert code == 0, f"exit={code} stderr={error!r}" + assert output == b"FIRST OUTPUT\r\nFINAL OUTPUT\r\n", f"output lost: {output!r}" + return {"wire_hex": bytes(peer.received).hex(), "stdout_hex": output.hex(), "exit": code} + finally: + if child.poll() is None: + child.kill() + child.wait() + peer.close() + + def password_signal(): + peer = Peer() + child = TTYClient(binary, peer.port, ["--auto-login", "--user", "admin", "--ask-pass"]) + try: + eventually(lambda: b"assword" in child.read(), "no password prompt") + child.process.send_signal(signal.SIGTERM) + code = child.wait_exit() + assert code != 0, "password signal interruption returned success" + return {"exit": code} + finally: + child.close() + peer.close() + + def trace_permissions(): + peer = Peer() + with tempfile.TemporaryDirectory(prefix="tclient-trace-") as folder: + path = os.path.join(folder, "session.trace") + child = TTYClient(binary, peer.port, ["--trace-file", path]) + try: + peer.send(b"READY\r\n") + child.wait_output(b"READY") + child.wait_raw() + child.quit() + mode = os.stat(path).st_mode & 0o777 + assert mode == 0o600, f"trace mode={oct(mode)}" + before = open(path, "rb").read() + refused = subprocess.run( + [binary, "--trace-file", path, "127.0.0.1", str(peer.port)], + input=b"", stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=5) + assert refused.returncode != 0, "existing trace was overwritten" + after = open(path, "rb").read() + assert after == before, "existing trace content was changed" + return {"mode": oct(mode), "existing_file_exit": refused.returncode} + finally: + child.close() + peer.close() + + def stdout_error(): + if not os.path.exists("/dev/full"): + return {"skipped": "/dev/full is unavailable on this platform"} + peer = Peer() + with open("/dev/full", "wb") as output: + child = subprocess.Popen([binary, "127.0.0.1", str(peer.port)], + stdin=subprocess.DEVNULL, stdout=output, stderr=subprocess.PIPE) + try: + peer.send(b"OUTPUT CANNOT BE WRITTEN\r\n") + peer.close_remote() + code = child.wait(timeout=5) + error = child.stderr.read() + assert code != 0, f"stdout write error returned success: {error!r}" + return {"exit": code, "stderr": error.decode(errors="replace")} + finally: + if child.poll() is None: + child.kill() + child.wait() + peer.close() + + def broken_stdout_pipe(): + peer = Peer() + child = TTYClient(binary, peer.port, stdout=subprocess.PIPE) + try: + assert peer.connected.wait(3) + child.wait_raw() + child.process.stdout.close() + peer.send(b"OUTPUT TO CLOSED PIPE\r\n") + code = child.wait_exit() + assert code > 0, f"broken pipe exit={code}; likely exited directly from SIGPIPE" + return {"exit": code} + finally: + child.close() + peer.close() + + def redirected_stdout(): + peer = Peer() + with tempfile.TemporaryFile() as output: + child = TTYClient(binary, peer.port, stdout=output) + try: + peer.send(b"REMOTE DATA ONLY\r\n") + child.wait_raw() + eventually(lambda: os.fstat(output.fileno()).st_size > 0, + "remote output did not reach redirected stdout") + child.send(b"x") + peer.wait_bytes(b"x") + child.wait_output(b"x") + child.quit() + assert b"tclient> " in child.read(), "local commands were not shown on stderr" + output.seek(0) + data = output.read() + assert data == b"REMOTE DATA ONLY\r\n", f"local UI polluted stdout: {data!r}" + return {"stdout_hex": data.hex()} + finally: + child.close() + peer.close() + + def fill_stdout_pipe(peer, child): + assert peer.connected.wait(3) + child.wait_raw() + pipe_fd = child.process.stdout.fileno() + capacity = fcntl.fcntl(pipe_fd, fcntl.F_GETPIPE_SZ) + peer.send(b"x" * (capacity + 65536)) + eventually(lambda: struct.unpack("i", fcntl.ioctl( + pipe_fd, termios.FIONREAD, struct.pack("i", 0)))[0] >= capacity, + "remote output did not fill stdout pipe") + return capacity + + def blocked_stdout_signal(): + if not hasattr(fcntl, "F_GETPIPE_SZ"): + return {"skipped": "Linux pipe capacity ioctl is unavailable"} + peer = Peer() + child = TTYClient(binary, peer.port, stdout=subprocess.PIPE) + try: + capacity = fill_stdout_pipe(peer, child) + child.process.send_signal(signal.SIGTERM) + code = child.wait_exit() + assert code > 0, f"blocked-output signal exit={code}" + return {"exit": code, "pipe_capacity": capacity} + finally: + child.close() + peer.close() + + def blocked_stdout_quit(): + if not hasattr(fcntl, "F_GETPIPE_SZ"): + return {"skipped": "Linux pipe capacity ioctl is unavailable"} + peer = Peer() + child = TTYClient(binary, peer.port, stdout=subprocess.PIPE) + try: + capacity = fill_stdout_pipe(peer, child) + child.quit() + assert b"tclient> " in child.read(), "local prompt could not bypass blocked stdout" + assert bytes(peer.received) == b"", "local quit was sent to the remote peer" + return {"exit": 0, "pipe_capacity": capacity} + finally: + child.close() + peer.close() + + def hidden_password_utf8_crlf(): + peer = Peer() + child = TTYClient(binary, peer.port, ["--auto-login", "--user", "admin", "--ask-pass"]) + try: + child.wait_output(b"Password: ") + child.wait_raw() + child.send("密钥错".encode() + b"\x7f" + "码".encode() + b"\r\n") + peer.send(b"Login:") + peer.wait_bytes(b"admin\r\n") + peer.send(b"Password:") + password = "密钥码".encode() + peer.wait_bytes(password + b"\r\n") + peer.send(b"\r\nrouter#\r\nSESSION READY\r\n") + child.wait_output(b"SESSION READY") + child.wait_raw() + child.send(b"probe\r") + peer.wait_bytes(b"probe\r\n") + child.quit() + expected = b"admin\r\n" + password + b"\r\nprobe\r\n" + assert bytes(peer.received) == expected, f"password edit or CRLF changed wire input: {bytes(peer.received)!r}" + assert "密".encode() not in child.read(), "hidden password was echoed" + assert "错".encode() not in child.read(), "deleted password character was echoed" + return {"exit": 0, "hidden": True, "utf8_backspace": True, "extra_enter": False} + finally: + child.close() + peer.close() + + for name, fn in [ + ("connect_failure_restores_terminal", failed_dial), + ("remote_eof_restores_terminal", remote_eof), + ("escape_quit_is_local", escape_quit), + ("sigterm_restores_terminal", terminated), + ("naws_initial_and_resize", naws_resize), + ("remote_local_echo_transitions", echo_transitions), + ("split_crlf_crnul_enter_mapping", split_newlines), + ("pipe_eof_drains_final_output", pipe_half_close), + ("default_pipe_eof_preserves_negotiation", pipe_eof_keeps_negotiation), + ("sigterm_during_password_restores_terminal", password_signal), + ("trace_private_and_no_overwrite", trace_permissions), + ("stdout_write_error_is_failure", stdout_error), + ("broken_stdout_pipe_restores_terminal", broken_stdout_pipe), + ("local_ui_keeps_stdout_clean", redirected_stdout), + ("sigterm_with_blocked_stdout_restores_terminal", blocked_stdout_signal), + ("quit_with_blocked_stdout_restores_terminal", blocked_stdout_quit), + ("hidden_password_utf8_edit_and_crlf", hidden_password_utf8_crlf), + ]: + run_case(name, fn, results) + + if args.out: + with open(args.out, "w") as stream: + json.dump(results, stream, indent=2) + raise SystemExit(0 if all(item["ok"] for item in results) else 1) + + +if __name__ == "__main__": + main() From a40505e972ab94c333496a903f9d5a0aeb37ee4e Mon Sep 17 00:00:00 2001 From: Eric_Lee Date: Tue, 3 Nov 2026 23:01:11 +0800 Subject: [PATCH 3/4] fix: keep Telnet sessions responsive under backpressure --- .github/workflows/test.yml | 7 +- docs/cli.md | 11 +- internal/cli/resize.go | 100 ++++++++ internal/cli/resize_linux_test.go | 200 +++++++++++++++ internal/cli/resize_test.go | 111 +++++++++ internal/cli/run.go | 25 +- internal/cli/terminal_other.go | 2 +- internal/cli/terminal_windows.go | 186 ++++++++++++++ internal/cli/terminal_windows_test.go | 343 ++++++++++++++++++++++++++ scripts/test_cli_pty.py | 24 ++ terminal_protocol_test.go | 107 ++++++++ transport.go | 45 +++- 12 files changed, 1139 insertions(+), 22 deletions(-) create mode 100644 internal/cli/resize.go create mode 100644 internal/cli/resize_linux_test.go create mode 100644 internal/cli/resize_test.go create mode 100644 internal/cli/terminal_windows.go create mode 100644 internal/cli/terminal_windows_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 488c8dd..099fb37 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,17 +10,20 @@ permissions: jobs: test: strategy: + fail-fast: false matrix: + os: ["ubuntu-latest", "windows-latest"] go-version: ["1.20.x", "stable"] - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: go-version: ${{ matrix.go-version }} - - run: go test -race ./... + - run: go test -race -timeout=3m ./... - run: go vet ./... - name: Verify interactive CLI with a real PTY and local TCP server + if: runner.os == 'Linux' run: | go build -o "$RUNNER_TEMP/tclient" ./cmd/tclient python3 scripts/test_cli_pty.py "$RUNNER_TEMP/tclient" diff --git a/docs/cli.md b/docs/cli.md index 2bf35b3..be9cd8b 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -17,9 +17,10 @@ The port defaults to 23. Options go before the host. Use `--help` for the full option list. This client supports plain TCP Telnet; it does not encrypt traffic. Interactive terminal support currently targets Unix systems. The PTY regression -suite runs on Linux; macOS can build the same Unix implementation. Other -platforms reject terminal input before changing console settings; redirected -input remains available. +suite runs on Linux; macOS can build the same Unix implementation. Windows +supports redirected input and output, with native CI tests for cancelling +blocked pipe operations. Windows rejects terminal input before changing console +settings. The default is **manual login**. Server prompts, menus, password changes, and subsequent interactions are displayed without trying to infer authentication @@ -78,7 +79,9 @@ not negotiate password echo correctly. Local raw mode is used only for terminal input and is restored before returning from the CLI, including connection errors, remote disconnects, local quit, and handled termination signals. Terminal dimensions are sent when NAWS is -negotiated; supported Unix systems also send changes after SIGWINCH. +negotiated; supported Unix systems also send changes after SIGWINCH. Dimensions +are refreshed after automatic login, and resize writes run separately from the +session loop so a slow peer cannot block local quit or disconnect handling. For redirected input, no raw terminal mode, local echo, escape commands, or Enter rewriting is applied: diff --git a/internal/cli/resize.go b/internal/cli/resize.go new file mode 100644 index 0000000..6d4f3d3 --- /dev/null +++ b/internal/cli/resize.go @@ -0,0 +1,100 @@ +package cli + +import ( + "context" + + "golang.org/x/term" +) + +type terminalSize struct { + height int + width int +} + +// watchTerminalSize subscribes before taking its first snapshot. Login may +// have taken long enough for the size captured in ClientConfig to be stale; +// refreshing here also closes the gap before the signal subscription began. +func watchTerminalSize(ctx context.Context, fd int) (<-chan terminalSize, func()) { + signals, stopSignals := watchResize() + watchCtx, cancel := context.WithCancel(ctx) + updates := make(chan terminalSize, 1) + stopped := make(chan struct{}) + refresh := func() { + width, height, err := term.GetSize(fd) + if err == nil && width > 0 && height > 0 { + queueLatestSize(updates, terminalSize{height: height, width: width}) + } + } + refresh() + go func() { + defer close(stopped) + defer close(updates) + defer stopSignals() + for { + select { + case <-watchCtx.Done(): + return + case _, ok := <-signals: + if !ok { + return + } + refresh() + } + } + }() + return updates, func() { + cancel() + <-stopped + } +} + +// Only the size watcher publishes to this one-element queue. A stalled network +// write retains the most recent pending size instead of building a backlog. +func queueLatestSize(updates chan terminalSize, size terminalSize) { + select { + case updates <- size: + return + default: + } + select { + case <-updates: + default: + } + select { + case updates <- size: + default: + } +} + +// A single worker serializes NAWS writes without occupying the session's +// control loop. Cancellation stops new work; closing the connection releases +// an in-flight write (or a wait for the transport's write mutex). +func startResizeWorker(ctx context.Context, updates <-chan terminalSize, send func(height, width int) error) (<-chan error, <-chan struct{}) { + stopped := make(chan struct{}) + if updates == nil { + close(stopped) + return nil, stopped + } + failures := make(chan error, 1) + go func() { + defer close(stopped) + for { + select { + case <-ctx.Done(): + return + case size, ok := <-updates: + if !ok || ctx.Err() != nil { + return + } + if err := send(size.height, size.width); err != nil { + select { + case failures <- err: + case <-ctx.Done(): + } + return + } + } + } + }() + return failures, stopped +} diff --git a/internal/cli/resize_linux_test.go b/internal/cli/resize_linux_test.go new file mode 100644 index 0000000..f179ddd --- /dev/null +++ b/internal/cli/resize_linux_test.go @@ -0,0 +1,200 @@ +//go:build linux + +package cli + +import ( + "bytes" + "context" + "fmt" + "io" + "net" + "os" + "sync" + "testing" + "time" + + "github.com/LeeEirc/tclientlib" + "golang.org/x/sys/unix" + "golang.org/x/term" +) + +func resizeTestPTY(t *testing.T, width, height uint16) (master, slave *os.File) { + t.Helper() + master, err := os.OpenFile("/dev/ptmx", os.O_RDWR|unix.O_NOCTTY, 0) + if err != nil { + t.Skipf("PTY unavailable: %v", err) + } + t.Cleanup(func() { master.Close() }) + if err := unix.IoctlSetPointerInt(int(master.Fd()), unix.TIOCSPTLCK, 0); err != nil { + t.Fatal(err) + } + number, err := unix.IoctlGetInt(int(master.Fd()), unix.TIOCGPTN) + if err != nil { + t.Fatal(err) + } + slave, err = os.OpenFile(fmt.Sprintf("/dev/pts/%d", number), os.O_RDWR|unix.O_NOCTTY, 0) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { slave.Close() }) + if err := unix.IoctlSetWinsize(int(slave.Fd()), unix.TIOCSWINSZ, &unix.Winsize{Col: width, Row: height}); err != nil { + t.Fatal(err) + } + state, err := term.MakeRaw(int(slave.Fd())) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { term.Restore(int(slave.Fd()), state) }) + return master, slave +} + +// resizeTestConn first requests NAWS. Its initial negotiation reply succeeds, +// then a standalone resize write can be held until Close. That reproduces a +// socket under backpressure without depending on TCP buffer sizes or sleeps. +type resizeTestConn struct { + firstRead bool + blockResize bool + resizeBlocked chan struct{} + remoteEOF chan struct{} + closed chan struct{} + writes chan []byte + blockOnce sync.Once + closeOnce sync.Once +} + +func newResizeTestConn(block bool) *resizeTestConn { + return &resizeTestConn{ + blockResize: block, resizeBlocked: make(chan struct{}), remoteEOF: make(chan struct{}), + closed: make(chan struct{}), writes: make(chan []byte, 16), + } +} + +func (c *resizeTestConn) Read(p []byte) (int, error) { + if !c.firstRead { + c.firstRead = true + return copy(p, []byte{255, 253, 31}), nil // DO NAWS + } + select { + case <-c.remoteEOF: + return 0, io.EOF + case <-c.closed: + return 0, net.ErrClosed + } +} + +func (c *resizeTestConn) Write(p []byte) (int, error) { + if c.blockResize && bytes.HasPrefix(p, []byte{255, 250, 31}) { + c.blockOnce.Do(func() { close(c.resizeBlocked) }) + <-c.closed + return 0, net.ErrClosed + } + select { + case c.writes <- append([]byte(nil), p...): + case <-c.closed: + return 0, net.ErrClosed + } + return len(p), nil +} + +func (c *resizeTestConn) Close() error { + c.closeOnce.Do(func() { close(c.closed) }) + return nil +} +func (c *resizeTestConn) LocalAddr() net.Addr { return &net.TCPAddr{} } +func (c *resizeTestConn) RemoteAddr() net.Addr { return &net.TCPAddr{} } +func (c *resizeTestConn) SetDeadline(time.Time) error { return nil } +func (c *resizeTestConn) SetReadDeadline(time.Time) error { return nil } +func (c *resizeTestConn) SetWriteDeadline(time.Time) error { return nil } + +func TestBlockedResizeDoesNotBlockSessionExit(t *testing.T) { + for _, exit := range []string{"local quit", "remote EOF"} { + t.Run(exit, func(t *testing.T) { + master, slave := resizeTestPTY(t, 110, 35) + conn := newResizeTestConn(true) + defer conn.Close() + client, err := tclientlib.NewClientConn(conn, &tclientlib.ClientConfig{Width: 80, Height: 24}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + terminal := &terminalInput{client: client, local: io.Discard, ending: []byte("\r\n"), echo: "off"} + done := make(chan error, 1) + go func() { + done <- runSession(context.Background(), options{drainTimeout: time.Second}, slave, io.Discard, conn, client, terminal, func() error { return nil }, func() {}) + }() + // Wait for the blocking NAWS write itself, rather than assuming the + // signal subscription or the negotiation completed after a delay. + ticker := time.NewTicker(5 * time.Millisecond) + defer ticker.Stop() + deadline := time.NewTimer(time.Second) + defer deadline.Stop() + waitResize: + for { + select { + case <-conn.resizeBlocked: + break waitResize + case <-ticker.C: + _ = unix.Kill(os.Getpid(), unix.SIGWINCH) + case <-deadline.C: + conn.Close() + <-done + t.Fatal("resize never reached the blocked network write") + } + } + if exit == "local quit" { + if _, err := master.Write([]byte("\x1dquit\r")); err != nil { + t.Fatal(err) + } + } else { + close(conn.remoteEOF) + } + select { + case err := <-done: + if err != nil { + t.Fatalf("%s: %v", exit, err) + } + case <-time.After(time.Second): + conn.Close() + <-done + t.Fatalf("blocked resize prevented %s from ending the session", exit) + } + }) + } +} + +func TestSessionRefreshesSizeWithoutAnotherSignal(t *testing.T) { + _, slave := resizeTestPTY(t, 109, 37) + conn := newResizeTestConn(false) + defer conn.Close() + // Model a size captured before login: the terminal already has a newer + // size when the interactive session starts, and no new SIGWINCH arrives. + client, err := tclientlib.NewClientConn(conn, &tclientlib.ClientConfig{Width: 80, Height: 24}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + terminal := &terminalInput{client: client, local: io.Discard, ending: []byte("\r\n"), echo: "off"} + done := make(chan error, 1) + go func() { + done <- runSession(context.Background(), options{drainTimeout: time.Second}, slave, io.Discard, conn, client, terminal, func() error { return nil }, func() {}) + }() + want := []byte{255, 250, 31, 0, 109, 0, 37, 255, 240} + timer := time.NewTimer(time.Second) + defer timer.Stop() + for { + select { + case packet := <-conn.writes: + if bytes.Contains(packet, want) { + close(conn.remoteEOF) + if err := <-done; err != nil { + t.Fatal(err) + } + return + } + case <-timer.C: + close(conn.remoteEOF) + <-done + t.Fatal("session kept the size captured before login until another SIGWINCH") + } + } +} diff --git a/internal/cli/resize_test.go b/internal/cli/resize_test.go new file mode 100644 index 0000000..30e43d1 --- /dev/null +++ b/internal/cli/resize_test.go @@ -0,0 +1,111 @@ +package cli + +import ( + "context" + "errors" + "testing" + "time" +) + +func TestResizeWorkerCoalescesPendingSizes(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + updates := make(chan terminalSize, 1) + started := make(chan terminalSize, 2) + release := make(chan struct{}) + var calls []terminalSize + send := func(height, width int) error { + size := terminalSize{height: height, width: width} + calls = append(calls, size) + started <- size + if len(calls) == 1 { + <-release + } + return nil + } + failures, stopped := startResizeWorker(ctx, updates, send) + first := terminalSize{height: 24, width: 80} + queueLatestSize(updates, first) + select { + case size := <-started: + if size != first { + t.Fatalf("first size = %+v", size) + } + case <-time.After(time.Second): + close(release) + t.Fatal("first resize did not start") + } + // A blocked write must not block the watcher or accumulate every obsolete + // size. Once it finishes, the next write must contain the final dimensions. + for i := 1; i <= 100; i++ { + queueLatestSize(updates, terminalSize{height: 24 + i, width: 80 + i}) + } + close(release) + wantLast := terminalSize{height: 124, width: 180} + select { + case size := <-started: + if size != wantLast { + t.Fatalf("pending size = %+v, want %+v", size, wantLast) + } + case <-time.After(time.Second): + t.Fatal("last resize did not start") + } + cancel() + <-stopped + if len(calls) != 2 { + t.Fatalf("sent %d resizes, want first and latest only", len(calls)) + } + select { + case err := <-failures: + t.Fatalf("unexpected resize error: %v", err) + default: + } +} + +func TestResizeWorkerReportsWriteError(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + updates := make(chan terminalSize, 1) + updates <- terminalSize{height: 24, width: 80} + want := errors.New("resize write failed") + failures, stopped := startResizeWorker(ctx, updates, func(int, int) error { return want }) + select { + case err := <-failures: + if !errors.Is(err, want) { + t.Fatalf("error = %v, want %v", err, want) + } + case <-time.After(time.Second): + t.Fatal("resize error was not delivered") + } + <-stopped +} + +func TestResizeWorkerDoesNotSendQueuedSizeAfterCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + updates := make(chan terminalSize, 1) + updates <- terminalSize{height: 24, width: 80} + started, release := make(chan struct{}), make(chan struct{}) + calls := 0 + _, stopped := startResizeWorker(ctx, updates, func(int, int) error { + calls++ + if calls == 1 { + close(started) + <-release + } + return nil + }) + select { + case <-started: + case <-time.After(time.Second): + close(release) + t.Fatal("resize did not start") + } + queueLatestSize(updates, terminalSize{height: 35, width: 110}) + cancel() + close(release) + <-stopped + if calls != 1 { + t.Fatalf("sent a queued resize after cancellation: %d calls", calls) + } +} diff --git a/internal/cli/run.go b/internal/cli/run.go index 2730de9..bac7e39 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -46,7 +46,7 @@ func Run(args []string, stdin *os.File, stdout, stderr io.Writer) int { return 0 } // A full diagnostic pipe must not undo cancellation after the terminal has - // already been restored. Reporting is best effort and bounded on Unix files. + // already been restored. Reporting uses the platform's cancellable outputs. if ctx.Err() == nil { diagnosticCtx, cancel := context.WithTimeout(ctx, 250*time.Millisecond) _, diagnostics, restore, outputErr := terminalOutputs(diagnosticCtx, io.Discard, stderr) @@ -168,6 +168,13 @@ func run(ctx context.Context, o options, stdin *os.File, stdout, stderr io.Write func runSession(ctx context.Context, o options, stdin *os.File, output io.Writer, conn net.Conn, client *tclientlib.Client, terminal *terminalInput, flush func() error, cancelOutput context.CancelFunc) error { sessionCtx, cancel := context.WithCancel(ctx) + var sizes <-chan terminalSize + if terminal != nil { + var stop func() + sizes, stop = watchTerminalSize(sessionCtx, int(stdin.Fd())) + defer stop() + } + resizeErrors, resizeStopped := startResizeWorker(sessionCtx, sizes, client.WindowChange) sent := make(chan error, 1) received := make(chan error, 1) inputStopped := make(chan struct{}) @@ -188,15 +195,10 @@ func runSession(ctx context.Context, o options, stdin *os.File, output io.Writer // input or negotiation may currently be blocked writing to the peer. _ = conn.Close() stopInput(stdin) + <-resizeStopped <-inputStopped <-outputStopped }() - var resize <-chan os.Signal - if terminal != nil { - var stop func() - resize, stop = watchResize() - defer stop() - } var timer *time.Timer var drained <-chan time.Time defer func() { @@ -244,13 +246,8 @@ func runSession(ctx context.Context, o options, stdin *os.File, output io.Writer drained = timer.C case <-drained: return fmt.Errorf("remote output did not finish within %s after stdin EOF", o.drainTimeout) - case <-resize: - width, height, err := term.GetSize(int(stdin.Fd())) - if err == nil { - if err := client.WindowChange(height, width); err != nil { - return fmt.Errorf("resize terminal: %w", err) - } - } + case err := <-resizeErrors: + return fmt.Errorf("resize terminal: %w", err) } } } diff --git a/internal/cli/terminal_other.go b/internal/cli/terminal_other.go index ce721bd..9eb942c 100644 --- a/internal/cli/terminal_other.go +++ b/internal/cli/terminal_other.go @@ -1,4 +1,4 @@ -//go:build !aix && !darwin && !dragonfly && !freebsd && !linux && !netbsd && !openbsd && !solaris +//go:build !aix && !darwin && !dragonfly && !freebsd && !linux && !netbsd && !openbsd && !solaris && !windows package cli diff --git a/internal/cli/terminal_windows.go b/internal/cli/terminal_windows.go new file mode 100644 index 0000000..94c3dac --- /dev/null +++ b/internal/cli/terminal_windows.go @@ -0,0 +1,186 @@ +package cli + +import ( + "context" + "io" + "os" + "runtime" + "time" + + "golang.org/x/sys/windows" +) + +func supportsTerminal() bool { return false } + +func interruptSignals() []os.Signal { return []os.Signal{os.Interrupt} } + +func watchResize() (<-chan os.Signal, func()) { return nil, func() {} } + +// Keep the caller's handle open. Closing a synchronous Windows pipe does not +// reliably interrupt a pending ReadFile or WriteFile; cancel the operation on +// its issuing thread instead. Overlapped pipe handles need CancelIoEx as well. +func stopInput(*os.File) {} + +func readInput(ctx context.Context, input *os.File, p []byte) (int, error) { + if err := ctx.Err(); err != nil { + return 0, err + } + if len(p) == 0 { + return 0, nil + } + handle, err := duplicatePipe(input) + if err != nil { + return 0, err + } + if handle == windows.InvalidHandle { + return input.Read(p) + } + defer windows.CloseHandle(handle) + n, err := pipeIO(ctx, handle, func(overlapped *windows.Overlapped, done *uint32) error { + return windows.ReadFile(handle, p, done, overlapped) + }) + runtime.KeepAlive(p) + if err == windows.ERROR_BROKEN_PIPE || err == windows.ERROR_HANDLE_EOF || n == 0 && err == nil { + err = io.EOF + } + return n, err +} + +func terminalOutputs(ctx context.Context, stdout, stderr io.Writer) (io.Writer, io.Writer, func() error, error) { + writers := []io.Writer{stdout, stderr} + var handles []windows.Handle + restore := func() error { + var first error + for _, handle := range handles { + if err := windows.CloseHandle(handle); err != nil && first == nil { + first = err + } + } + handles = nil + return first + } + for i, writer := range writers { + file, ok := writer.(*os.File) + if !ok { + continue + } + handle, err := duplicatePipe(file) + if err != nil { + _ = restore() + return nil, nil, nil, err + } + if handle != windows.InvalidHandle { + handles = append(handles, handle) + writers[i] = &pipeWriter{ctx: ctx, handle: handle} + } + } + return writers[0], writers[1], restore, nil +} + +// Use RawConn to preserve the original handle's runtime polling association. +// Disk files and consoles retain os.File's append and Unicode handling. +func duplicatePipe(file *os.File) (windows.Handle, error) { + handle := windows.InvalidHandle + raw, err := file.SyscallConn() + if err != nil { + return handle, err + } + var handleErr error + err = raw.Control(func(fd uintptr) { + var kind uint32 + kind, handleErr = windows.GetFileType(windows.Handle(fd)) + if handleErr == nil && kind == windows.FILE_TYPE_PIPE { + process := windows.CurrentProcess() + handleErr = windows.DuplicateHandle(process, windows.Handle(fd), process, &handle, 0, false, windows.DUPLICATE_SAME_ACCESS) + } + }) + if err == nil { + err = handleErr + } + return handle, err +} + +type pipeWriter struct { + ctx context.Context + handle windows.Handle +} + +func (w *pipeWriter) Write(p []byte) (int, error) { + total := 0 + for len(p) > 0 { + n, err := pipeIO(w.ctx, w.handle, func(overlapped *windows.Overlapped, done *uint32) error { + return windows.WriteFile(w.handle, p, done, overlapped) + }) + runtime.KeepAlive(p) + total += n + p = p[n:] + if err != nil { + return total, err + } + if n == 0 { + return total, io.ErrShortWrite + } + } + return total, nil +} + +var cancelSynchronousIO = windows.NewLazySystemDLL("kernel32.dll").NewProc("CancelSynchronousIo") + +// A supplied OVERLAPPED works with both synchronous inherited pipes and pipes +// opened for overlapped I/O. Its event's low bit suppresses completion packets: +// the original pipe may belong to Go's IOCP, which does not own this OVERLAPPED. +func pipeIO(ctx context.Context, handle windows.Handle, operation func(*windows.Overlapped, *uint32) error) (int, error) { + if err := ctx.Err(); err != nil { + return 0, err + } + runtime.LockOSThread() + defer runtime.UnlockOSThread() + thread, err := windows.OpenThread(windows.THREAD_TERMINATE, false, windows.GetCurrentThreadId()) + if err != nil { + return 0, err + } + defer windows.CloseHandle(thread) + event, err := windows.CreateEvent(nil, 1, 0, nil) + if err != nil { + return 0, err + } + defer windows.CloseHandle(event) + overlapped := &windows.Overlapped{HEvent: event | 1} + finished, watcherStopped := make(chan struct{}), make(chan struct{}) + go func() { + defer close(watcherStopped) + select { + case <-finished: + return + case <-ctx.Done(): + } + ticker := time.NewTicker(10 * time.Millisecond) + defer ticker.Stop() + for { + _ = windows.CancelIoEx(handle, overlapped) + _, _, _ = cancelSynchronousIO.Call(uintptr(thread)) + // Cancellation may arrive before ReadFile/WriteFile starts. Retry + // until completion so that an initial ERROR_NOT_FOUND cannot hang. + select { + case <-finished: + return + case <-ticker.C: + } + } + }() + var done uint32 + err = operation(overlapped, &done) + if err == nil || err == windows.ERROR_IO_PENDING { + // An overlapped handle's immediate-success byte count must also come + // from OVERLAPPED. Synchronous handles populate it before returning. + err = windows.GetOverlappedResult(handle, overlapped, &done, true) + } + close(finished) + // Join before reusing the thread or freeing any handle/buffer. Otherwise a + // late CancelSynchronousIo could cancel an unrelated operation on it. + <-watcherStopped + if err != nil && ctx.Err() != nil { + err = ctx.Err() + } + return int(done), err +} diff --git a/internal/cli/terminal_windows_test.go b/internal/cli/terminal_windows_test.go new file mode 100644 index 0000000..f945c56 --- /dev/null +++ b/internal/cli/terminal_windows_test.go @@ -0,0 +1,343 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "net" + "os" + "sync/atomic" + "testing" + "time" + "unsafe" + + "golang.org/x/sys/windows" +) + +var windowsPipeSequence uint64 + +var windowsPipeModes = []struct { + name string + open func(*testing.T) (*os.File, *os.File) +}{ + {"synchronous", synchronousTestPipe}, + {"overlapped", overlappedTestPipe}, + {"os-pipe", func(t *testing.T) (*os.File, *os.File) { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { r.Close(); w.Close() }) + return r, w + }}, +} + +func synchronousTestPipe(t *testing.T) (*os.File, *os.File) { + t.Helper() + var r, w windows.Handle + if err := windows.CreatePipe(&r, &w, nil, 4096); err != nil { + t.Fatal(err) + } + reader, writer := os.NewFile(uintptr(r), "pipe-reader"), os.NewFile(uintptr(w), "pipe-writer") + t.Cleanup(func() { reader.Close(); writer.Close() }) + return reader, writer +} + +func overlappedTestPipe(t *testing.T) (*os.File, *os.File) { + t.Helper() + name := fmt.Sprintf(`\\.\pipe\tclient-test-%d-%d`, os.Getpid(), atomic.AddUint64(&windowsPipeSequence, 1)) + path, err := windows.UTF16PtrFromString(name) + if err != nil { + t.Fatal(err) + } + r, err := windows.CreateNamedPipe(path, windows.PIPE_ACCESS_INBOUND|windows.FILE_FLAG_OVERLAPPED, + windows.PIPE_TYPE_BYTE|windows.PIPE_WAIT, 1, 4096, 4096, 0, nil) + if err != nil { + t.Fatal(err) + } + reader := os.NewFile(uintptr(r), "overlapped-reader") + t.Cleanup(func() { reader.Close() }) + w, err := windows.CreateFile(path, windows.GENERIC_WRITE, 0, nil, windows.OPEN_EXISTING, windows.FILE_FLAG_OVERLAPPED, 0) + if err != nil { + t.Fatal(err) + } + writer := os.NewFile(uintptr(w), "overlapped-writer") + t.Cleanup(func() { writer.Close() }) + // The client connected before ConnectNamedPipe, so no asynchronous connect + // remains outstanding (ERROR_PIPE_CONNECTED is the documented success case). + if err := windows.ConnectNamedPipe(r, &windows.Overlapped{}); err != nil && err != windows.ERROR_PIPE_CONNECTED { + t.Fatal(err) + } + return reader, writer +} + +var peekNamedPipe = windows.NewLazySystemDLL("kernel32.dll").NewProc("PeekNamedPipe") + +func pipeAvailable(t *testing.T, file *os.File) uint32 { + t.Helper() + raw, err := file.SyscallConn() + if err != nil { + t.Fatal(err) + } + var available uint32 + var peekErr error + err = raw.Control(func(fd uintptr) { + ok, _, callErr := peekNamedPipe.Call(fd, 0, 0, 0, uintptr(unsafe.Pointer(&available)), 0) + if ok == 0 { + peekErr = callErr + } + }) + if err != nil || peekErr != nil { + t.Fatalf("peek pipe: %v, %v", err, peekErr) + } + return available +} + +func waitPipeData(t *testing.T, reader *os.File) { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for pipeAvailable(t, reader) == 0 { + if time.Now().After(deadline) { + t.Fatal("output never reached the pipe") + } + time.Sleep(time.Millisecond) + } +} + +func checkPipeReusable(t *testing.T, reader, writer *os.File) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + // A canceled write may already have copied a prefix into the pipe. Drain it + // before checking that a later session can use the original handles. + buffer := make([]byte, 4096) + for available := pipeAvailable(t, reader); available > 0; available = pipeAvailable(t, reader) { + if available < uint32(len(buffer)) { + buffer = buffer[:available] + } + if _, err := readInput(ctx, reader, buffer); err != nil { + t.Fatal(err) + } + } + output, _, restore, err := terminalOutputs(ctx, writer, io.Discard) + if err != nil { + t.Fatal(err) + } + defer restore() + marker := []byte("original handles still work") + if n, err := output.Write(marker); err != nil || n != len(marker) { + t.Fatalf("reuse output: %d, %v", n, err) + } + got := make([]byte, len(marker)) + if n, err := readInput(ctx, reader, got); err != nil || n != len(marker) || !bytes.Equal(got, marker) { + t.Fatalf("reuse input: %d, %v, %q", n, err, got) + } +} + +type windowsIOResult struct { + n int + err error +} + +func TestWindowsPipeOutputCancellation(t *testing.T) { + for _, mode := range windowsPipeModes { + t.Run(mode.name, func(t *testing.T) { + reader, writer := mode.open(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + output, _, restore, err := terminalOutputs(ctx, writer, io.Discard) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { restore() }) + done, stopped := make(chan windowsIOResult, 1), make(chan struct{}) + payload := make([]byte, 8<<20) + go func() { + defer close(stopped) + n, err := output.Write(payload) + done <- windowsIOResult{n, err} + }() + t.Cleanup(func() { cancel(); reader.Close(); <-stopped }) + waitPipeData(t, reader) + select { + case result := <-done: + t.Fatalf("write did not block: %+v", result) + default: + } + cancel() + select { + case result := <-done: + if !errors.Is(result.err, context.Canceled) || result.n < 0 || result.n >= len(payload) { + t.Fatalf("canceled write: %+v", result) + } + case <-time.After(2 * time.Second): + t.Fatal("cancellation did not stop pipe output") + } + if err := restore(); err != nil { + t.Fatal(err) + } + checkPipeReusable(t, reader, writer) + }) + } +} + +func TestWindowsPipeInputCancellation(t *testing.T) { + for _, mode := range windowsPipeModes { + t.Run(mode.name, func(t *testing.T) { + reader, writer := mode.open(t) + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + done, stopped := make(chan windowsIOResult, 1), make(chan struct{}) + go func() { + defer close(stopped) + n, err := readInput(ctx, reader, make([]byte, 1)) + done <- windowsIOResult{n, err} + }() + t.Cleanup(func() { cancel(); writer.Close(); <-stopped }) + select { + case result := <-done: + if result.n != 0 || !errors.Is(result.err, context.DeadlineExceeded) { + t.Fatalf("canceled read: %+v", result) + } + case <-time.After(2 * time.Second): + t.Fatal("cancellation did not stop pipe input") + } + stopInput(reader) + checkPipeReusable(t, reader, writer) + }) + } +} + +func TestWindowsPipeCancellationBeforeSyscall(t *testing.T) { + for _, mode := range windowsPipeModes { + t.Run(mode.name, func(t *testing.T) { + reader, writer := mode.open(t) + handle, err := duplicatePipe(reader) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { windows.CloseHandle(handle) }) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + entered, release, stopped := make(chan struct{}), make(chan struct{}), make(chan struct{}) + releaseOperation := func() { + select { + case <-release: + default: + close(release) + } + } + done := make(chan windowsIOResult, 1) + go func() { + defer close(stopped) + var buffer [1]byte + n, err := pipeIO(ctx, handle, func(overlapped *windows.Overlapped, done *uint32) error { + close(entered) + <-release + return windows.ReadFile(handle, buffer[:], done, overlapped) + }) + done <- windowsIOResult{n, err} + }() + t.Cleanup(func() { cancel(); releaseOperation(); writer.Close(); <-stopped }) + select { + case <-entered: + case result := <-done: + t.Fatalf("pipe operation did not start: %+v", result) + case <-time.After(2 * time.Second): + t.Fatal("pipe operation did not start") + } + cancel() + // Let the cancellation watcher run while there is no pending I/O. + // It must retry once ReadFile starts, rather than lose cancellation. + time.Sleep(30 * time.Millisecond) + releaseOperation() + select { + case result := <-done: + if result.n != 0 || !errors.Is(result.err, context.Canceled) { + t.Fatalf("early cancellation: %+v", result) + } + case <-time.After(2 * time.Second): + t.Fatal("cancellation before ReadFile was lost") + } + checkPipeReusable(t, reader, writer) + }) + } +} + +func TestWindowsPipeEOF(t *testing.T) { + for _, mode := range windowsPipeModes { + t.Run(mode.name, func(t *testing.T) { + reader, writer := mode.open(t) + writer.Close() + if n, err := readInput(context.Background(), reader, make([]byte, 1)); n != 0 || !errors.Is(err, io.EOF) { + t.Fatalf("pipe EOF: %d, %v", n, err) + } + }) + } +} + +func TestWindowsPipeAlreadyCanceled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + n, err := pipeIO(ctx, windows.InvalidHandle, func(*windows.Overlapped, *uint32) error { + t.Fatal("operation started after cancellation") + return nil + }) + if n != 0 || !errors.Is(err, context.Canceled) { + t.Fatalf("already canceled operation: %d, %v", n, err) + } +} + +func TestWindowsRegularFileOutputUsesOriginalFile(t *testing.T) { + file := testInput(t, nil) + output, diagnostics, restore, err := terminalOutputs(context.Background(), file, file) + if err != nil { + t.Fatal(err) + } + defer restore() + if output != file || diagnostics != file { + t.Fatal("non-pipe file was replaced") + } + stopInput(file) + if _, err := io.WriteString(output, "你好\n"); err != nil { + t.Fatal(err) + } +} + +func TestWindowsDrainDeadlineCancelsBlockedOutput(t *testing.T) { + for _, mode := range windowsPipeModes { + t.Run(mode.name, func(t *testing.T) { + reader, writer := mode.open(t) + input, inputWriter := synchronousTestPipe(t) + host, port := testServer(t, func(conn net.Conn) { + _, _ = conn.Write(bytes.Repeat([]byte("x"), 8<<20)) + }) + o, err := parseOptions([]string{"--drain-timeout", "50ms", host, port}, io.Discard) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done, stopped := make(chan error, 1), make(chan struct{}) + go func() { + defer close(stopped) + done <- run(ctx, o, input, writer, io.Discard) + }() + t.Cleanup(func() { cancel(); reader.Close(); inputWriter.Close(); <-stopped }) + waitPipeData(t, reader) + inputWriter.Close() + select { + case err := <-done: + if err == nil { + t.Fatal("blocked output did not reach the drain deadline") + } + case <-time.After(2 * time.Second): + t.Fatal("drain deadline did not stop blocked output") + } + checkPipeReusable(t, reader, writer) + }) + } +} diff --git a/scripts/test_cli_pty.py b/scripts/test_cli_pty.py index 1f2f400..577cf9a 100644 --- a/scripts/test_cli_pty.py +++ b/scripts/test_cli_pty.py @@ -289,6 +289,29 @@ def echo_transitions(): child.close() peer.close() + def login_resize_refresh(): + peer = Peer() + child = TTYClient(binary, peer.port, ["--auto-login", "--user", "admin"]) + initial = IAC + b"\xfa" + NAWS + b"\x00\x50\x00\x18" + IAC + b"\xf0" + resized = IAC + b"\xfa" + NAWS + b"\x00\x6f\x00\x25" + IAC + b"\xf0" + try: + peer.send(IAC + DO + NAWS + b"Login:") + peer.wait_bytes(initial) + peer.wait_bytes(b"admin\r\n") + # Login is still waiting for the shell prompt. No second resize + # signal is sent after login: the client must refresh on subscribe. + fcntl.ioctl(child.slave, termios.TIOCSWINSZ, struct.pack("HHHH", 37, 111, 0, 0)) + child.process.send_signal(signal.SIGWINCH) + peer.send(b"\r\nrouter#\r\nSESSION READY\r\n") + child.wait_output(b"SESSION READY") + child.wait_raw() + peer.wait_bytes(resized) + child.quit() + return {"wire_hex": bytes(peer.received).hex(), "refreshed_after_login": True} + finally: + child.close() + peer.close() + def split_newlines(): peer = Peer() child = TTYClient(binary, peer.port, ["--echo", "off"]) @@ -533,6 +556,7 @@ def hidden_password_utf8_crlf(): ("escape_quit_is_local", escape_quit), ("sigterm_restores_terminal", terminated), ("naws_initial_and_resize", naws_resize), + ("resize_during_login_is_refreshed", login_resize_refresh), ("remote_local_echo_transitions", echo_transitions), ("split_crlf_crnul_enter_mapping", split_newlines), ("pipe_eof_drains_final_output", pipe_half_close), diff --git a/terminal_protocol_test.go b/terminal_protocol_test.go index 02abcfe..b6ddd86 100644 --- a/terminal_protocol_test.go +++ b/terminal_protocol_test.go @@ -195,6 +195,113 @@ func TestCommandOutputDoesNotWaitForBlockedApplicationWrite(t *testing.T) { } } +func TestNoReplyEventsDoNotWaitForBlockedApplicationWrite(t *testing.T) { + tests := []struct { + name string + setup []byte + reply []byte + event []byte + data []byte + want []byte + }{ + {"initial WONT ECHO", nil, nil, []byte{iac, wont, echoOption}, nil, nil}, + {"initial DONT NAWS", nil, nil, []byte{iac, dont, windowSize}, nil, nil}, + {"duplicate WILL ECHO", []byte{iac, will, echoOption}, []byte{iac, do, echoOption}, []byte{iac, will, echoOption}, nil, nil}, + {"duplicate WILL BINARY", []byte{iac, will, binaryOption}, []byte{iac, do, binaryOption}, []byte{iac, will, binaryOption}, []byte{'\r', 0, 'x'}, []byte{'\r', 0, 'x'}}, + {"duplicate DO NAWS", []byte{iac, do, windowSize}, append([]byte{iac, will, windowSize}, (optionPacket{command: sb, option: windowSize, parameters: []byte{0, 80, 0, 24}}).bytes()...), []byte{iac, do, windowSize}, nil, nil}, + {"duplicate DO BINARY", []byte{iac, do, binaryOption}, []byte{iac, will, binaryOption}, []byte{iac, do, binaryOption}, []byte{'\r', 0, 'x'}, []byte{'\r', 'x'}}, + {"unnegotiated SB", nil, nil, (optionPacket{command: sb, option: terminalType, parameters: []byte{1}}).bytes(), nil, nil}, + {"non SEND SB", []byte{iac, do, terminalType}, []byte{iac, will, terminalType}, (optionPacket{command: sb, option: terminalType, parameters: []byte{0}}).bytes(), nil, nil}, + {"unsupported SB", []byte{iac, do, binaryOption}, []byte{iac, will, binaryOption}, (optionPacket{command: sb, option: binaryOption, parameters: []byte{1}}).bytes(), nil, nil}, + {"malformed environment SEND", []byte{iac, do, newEnvironment}, []byte{iac, will, newEnvironment}, (optionPacket{command: sb, option: newEnvironment, parameters: []byte("\x01\x00USER\x01")}).bytes(), nil, nil}, + } + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + server, peer := net.Pipe() + defer server.Close() + deadline := time.Now().Add(5 * time.Second) + _ = server.SetDeadline(deadline) + _ = peer.SetDeadline(deadline) + conn := &protocolWriteStartedConn{Conn: peer, started: make(chan struct{})} + config, err := prepareConfig(&ClientConfig{User: "alice"}) + if err != nil { + t.Fatal(err) + } + transport := newTransport(conn, config.transportConfig()) + client := &Client{conn: conn, transport: transport, reader: transport, writer: transport} + defer client.Close() + if len(tc.setup) > 0 { + setupDone := make(chan error, 1) + go func() { + if _, err := server.Write(append(append([]byte(nil), tc.setup...), 's')); err != nil { + setupDone <- err + return + } + reply := make([]byte, len(tc.reply)) + _, err := io.ReadFull(server, reply) + if err == nil && !bytes.Equal(reply, tc.reply) { + err = fmt.Errorf("setup reply = %v, want %v", reply, tc.reply) + } + setupDone <- err + }() + if _, err := io.ReadFull(client, make([]byte, 1)); err != nil { + t.Fatal(err) + } + if err := <-setupDone; err != nil { + t.Fatal(err) + } + // Setup has finished; observe only the following application write. + conn.once = sync.Once{} + conn.started = make(chan struct{}) + } + mode := client.TerminalMode() + written := make(chan error, 1) + go func() { _, err := client.Write([]byte("input")); written <- err }() + <-conn.started // writeMu is held while the peer deliberately does not read. + data, want := tc.data, tc.want + if data == nil { + data, want = []byte("x"), []byte("x") + } + served := make(chan error, 1) + go func() { + _, err := server.Write(append(append([]byte(nil), tc.event...), data...)) + served <- err + }() + read := make(chan error, 1) + go func() { + output := make([]byte, len(want)) + _, err := io.ReadFull(client, output) + if err == nil && !bytes.Equal(output, want) { + err = fmt.Errorf("output = %v, want %v", output, want) + } + read <- err + }() + select { + case err := <-read: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("event needing no reply blocked Read behind the application Write") + } + if got := client.TerminalMode(); got != mode { + t.Fatalf("no-reply event changed mode: %+v, want %+v", got, mode) + } + if err := <-served; err != nil { + t.Fatal(err) + } + input := make([]byte, len("input")) + if _, err := io.ReadFull(server, input); err != nil || string(input) != "input" { + t.Fatalf("application write = %q, %v", input, err) + } + if err := <-written; err != nil { + t.Fatal(err) + } + }) + } +} + type protocolWriteStartedConn struct { net.Conn started chan struct{} diff --git a/transport.go b/transport.go index aa2b344..8b97d26 100644 --- a/transport.go +++ b/transport.go @@ -185,7 +185,7 @@ func (t *transport) windowChange(height, width int) error { } func (t *transport) handleEvent(event telnetEvent) error { - if event.typ != negotiationEvent && event.typ != subnegotiationEvent { + if !t.eventNeedsWrite(event) { return nil } // Keep option state changes and their replies ahead of concurrent writes. @@ -202,6 +202,49 @@ func (t *transport) handleEvent(event telnetEvent) error { } } +// Read serializes incoming option changes. WindowChange only updates dimensions, +// so this read-only check can skip no-op events without taking writeMu. Actual +// changes and replies are still handled together under writeMu -> stateMu. +// Otherwise a duplicate or ignored event could block reading behind a full +// application send buffer, even though it has nothing to send to the peer. +func (t *transport) eventNeedsWrite(event telnetEvent) bool { + if event.typ != negotiationEvent && event.typ != subnegotiationEvent { + return false + } + t.stateMu.Lock() + defer t.stateMu.Unlock() + if event.typ == negotiationEvent { + switch event.command { + case do: + return !t.supportsLocalOption(event.option) || !t.localOptions[event.option] + case dont: + return t.localOptions[event.option] + case will: + return !t.supportsRemoteOption(event.option) || !t.remoteOptions[event.option] + case wont: + return t.remoteOptions[event.option] + default: + return false + } + } + if len(event.parameters) == 0 || event.parameters[0] != 1 || !t.localOptions[event.option] { + return false + } + switch event.option { + case terminalType, terminalSpeed: + return true + case oldEnvironment, newEnvironment: + // Validate before waiting for the writer; malformed requests have no + // reply and must not block either. Do not change the OLD-ENVIRON dialect + // here: handleSubnegotiation commits it with the actual response. + params, _ := environmentResponse(event.parameters[1:], t.config.User, + event.option == oldEnvironment, t.oldEnvironmentReversed, t.config.maxSubnegotiation) + return params != nil + default: + return false + } +} + func (t *transport) handleNegotiation(command, option byte) []optionPacket { t.stateMu.Lock() defer t.stateMu.Unlock() From f57b74d9fb0f593560cad51d4bba8ad7e2ac10ac Mon Sep 17 00:00:00 2001 From: Eric_Lee Date: Tue, 3 Nov 2026 23:25:33 +0800 Subject: [PATCH 4/4] fix: decouple terminal input writes and preserve file flags --- docs/cli.md | 11 +- internal/cli/descriptor.go | 53 +++++ internal/cli/descriptor_unix_test.go | 190 ++++++++++++++++ internal/cli/input.go | 6 +- internal/cli/resize.go | 7 +- internal/cli/run.go | 71 ++++-- internal/cli/sender.go | 183 ++++++++++++++++ internal/cli/sender_linux_test.go | 56 +++++ internal/cli/sender_queue_test.go | 314 +++++++++++++++++++++++++++ internal/cli/sender_test.go | 122 +++++++++++ internal/cli/terminal_unix.go | 89 +++++--- scripts/test_cli_pty.py | 15 +- 12 files changed, 1051 insertions(+), 66 deletions(-) create mode 100644 internal/cli/descriptor.go create mode 100644 internal/cli/descriptor_unix_test.go create mode 100644 internal/cli/sender.go create mode 100644 internal/cli/sender_linux_test.go create mode 100644 internal/cli/sender_queue_test.go create mode 100644 internal/cli/sender_test.go diff --git a/docs/cli.md b/docs/cli.md index be9cd8b..d61a59a 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -78,7 +78,8 @@ not negotiate password echo correctly. Local raw mode is used only for terminal input and is restored before returning from the CLI, including connection errors, remote disconnects, local quit, and -handled termination signals. Terminal dimensions are sent when NAWS is +handled termination signals. Original input and output file flags are preserved, +including nonblocking streams. Terminal dimensions are sent when NAWS is negotiated; supported Unix systems also send changes after SIGWINCH. Dimensions are refreshed after automatic login, and resize writes run separately from the session loop so a slow peer cannot block local quit or disconnect handling. @@ -123,6 +124,14 @@ the remote endpoint. These command names are processed locally and are never sent as terminal text. `send` calls the library's existing `SendCommand` API. It does not implement TCP urgent data or Telnet SYNCH. +Interactive network writes use an ordered queue so local commands can still be +read when the peer stops accepting input. The input backlog is limited to +256 KiB, including any write in progress. If that limit is exceeded, the CLI +reports an error and closes the session; pending input may not have reached the +peer. Local `quit` also closes the connection without waiting for pending input +to be delivered. Redirected input remains a stream with backpressure and has no +limit on its total size. + ## Tracing Tracing is disabled by default. To enable it explicitly: diff --git a/internal/cli/descriptor.go b/internal/cli/descriptor.go new file mode 100644 index 0000000..9d0b6f8 --- /dev/null +++ b/internal/cli/descriptor.go @@ -0,0 +1,53 @@ +package cli + +import ( + "os" + + "golang.org/x/term" +) + +// File.Fd can switch a runtime-managed Unix descriptor to blocking mode. +// Control leaves that state unchanged and keeps the descriptor valid until the +// operation returns, even if another goroutine closes the file concurrently. +// The descriptor must not be retained for later I/O outside this callback. +func withFileDescriptor(file *os.File, operation func(uintptr) error) error { + raw, err := file.SyscallConn() + if err != nil { + return err + } + var operationErr error + if err := raw.Control(func(fd uintptr) { operationErr = operation(fd) }); err != nil { + return err + } + return operationErr +} + +func fileIsTerminal(file *os.File) (terminal bool, err error) { + err = withFileDescriptor(file, func(fd uintptr) error { + terminal = term.IsTerminal(int(fd)) + return nil + }) + return terminal, err +} + +func fileTerminalSize(file *os.File) (width, height int, err error) { + err = withFileDescriptor(file, func(fd uintptr) error { + var sizeErr error + width, height, sizeErr = term.GetSize(int(fd)) + return sizeErr + }) + return width, height, err +} + +func fileMakeRaw(file *os.File) (state *term.State, err error) { + err = withFileDescriptor(file, func(fd uintptr) error { + var rawErr error + state, rawErr = term.MakeRaw(int(fd)) + return rawErr + }) + return state, err +} + +func fileRestoreTerminal(file *os.File, state *term.State) error { + return withFileDescriptor(file, func(fd uintptr) error { return term.Restore(int(fd), state) }) +} diff --git a/internal/cli/descriptor_unix_test.go b/internal/cli/descriptor_unix_test.go new file mode 100644 index 0000000..9474c3c --- /dev/null +++ b/internal/cli/descriptor_unix_test.go @@ -0,0 +1,190 @@ +//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris + +package cli + +import ( + "bytes" + "context" + "errors" + "io" + "net" + "os" + "testing" + "time" + + "golang.org/x/sys/unix" +) + +func descriptorTestFlags(t *testing.T, file *os.File) int { + t.Helper() + raw, err := file.SyscallConn() + if err != nil { + t.Fatal(err) + } + var flags int + var controlErr error + if err := raw.Control(func(fd uintptr) { + flags, controlErr = unix.FcntlInt(fd, unix.F_GETFL, 0) + }); err != nil { + t.Fatal(err) + } + if controlErr != nil { + t.Fatal(controlErr) + } + return flags +} + +func descriptorTestPipe(t *testing.T) (*os.File, *os.File) { + t.Helper() + reader, writer, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { reader.Close(); writer.Close() }) + return reader, writer +} + +func TestTerminalOutputsPreserveManagedPipeFlags(t *testing.T) { + for _, alias := range []string{"same file", "duplicated descriptor"} { + t.Run(alias, func(t *testing.T) { + _, stdout := descriptorTestPipe(t) + stderr := stdout + if alias == "duplicated descriptor" { + raw, err := stdout.SyscallConn() + if err != nil { + t.Fatal(err) + } + var duplicate int + var duplicateErr error + if err := raw.Control(func(fd uintptr) { duplicate, duplicateErr = unix.Dup(int(fd)) }); err != nil { + t.Fatal(err) + } + if duplicateErr != nil { + t.Fatal(duplicateErr) + } + stderr = os.NewFile(uintptr(duplicate), "stderr alias") + defer stderr.Close() + } + before := descriptorTestFlags(t, stdout) + if before&unix.O_NONBLOCK == 0 { + t.Fatal("Go pipe was not nonblocking before output setup") + } + _, _, restore, err := terminalOutputs(context.Background(), stdout, stderr) + if err != nil { + t.Fatal(err) + } + if got := descriptorTestFlags(t, stdout); got != before { + _ = restore() + t.Fatalf("output setup changed pipe flags: %#x, want %#x", got, before) + } + if err := restore(); err != nil { + t.Fatal(err) + } + for _, file := range []*os.File{stdout, stderr} { + if got := descriptorTestFlags(t, file); got != before { + t.Fatalf("output restore changed pipe flags: %#x, want %#x", got, before) + } + } + }) + } +} + +func TestOutputSetupFailureDoesNotChangeEarlierDescriptor(t *testing.T) { + _, stdout := descriptorTestPipe(t) + _, stderr := descriptorTestPipe(t) + if err := stderr.Close(); err != nil { + t.Fatal(err) + } + before := descriptorTestFlags(t, stdout) + if _, _, _, err := terminalOutputs(context.Background(), stdout, stderr); err == nil { + t.Fatal("closed output descriptor was accepted") + } + if got := descriptorTestFlags(t, stdout); got != before { + t.Fatalf("failed setup changed stdout flags: %#x, want %#x", got, before) + } +} + +func TestRunConnectFailurePreservesInputFlags(t *testing.T) { + input, _ := descriptorTestPipe(t) + before := descriptorTestFlags(t, input) + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + host, port, err := net.SplitHostPort(listener.Addr().String()) + listener.Close() + if err != nil { + t.Fatal(err) + } + var diagnostics bytes.Buffer + if code := Run([]string{"--timeout", "1s", host, port}, input, io.Discard, &diagnostics); code != 1 { + t.Fatalf("connection failure exit = %d, diagnostics=%q", code, diagnostics.String()) + } + if got := descriptorTestFlags(t, input); got != before { + t.Fatalf("connection failure changed stdin flags: %#x, want %#x", got, before) + } +} + +func TestReadInputPreservesManagedPipeFlags(t *testing.T) { + for _, operation := range []string{"read", "cancel before read", "cancel while idle"} { + t.Run(operation, func(t *testing.T) { + input, writer := descriptorTestPipe(t) + before := descriptorTestFlags(t, input) + ctx, cancel := context.WithCancel(context.Background()) + if operation == "cancel while idle" { + cancel() + ctx, cancel = context.WithTimeout(context.Background(), 20*time.Millisecond) + } + defer cancel() + switch operation { + case "read": + if _, err := writer.Write([]byte("x")); err != nil { + t.Fatal(err) + } + case "cancel before read": + cancel() + } + var data [1]byte + n, err := readInput(ctx, input, data[:]) + if operation == "read" { + if n != 1 || err != nil || data[0] != 'x' { + t.Fatalf("read = %d %v %q", n, err, data) + } + } else if !errors.Is(err, ctx.Err()) || err == nil { + t.Fatalf("canceled read = %d, %v; context=%v", n, err, ctx.Err()) + } + if got := descriptorTestFlags(t, input); got != before { + t.Fatalf("input operation changed pipe flags: %#x, want %#x", got, before) + } + }) + } +} + +func TestTerminalProbesPreserveManagedPipeFlags(t *testing.T) { + input, _ := descriptorTestPipe(t) + before := descriptorTestFlags(t, input) + if terminal, err := fileIsTerminal(input); err != nil || terminal { + t.Fatalf("pipe terminal probe = %t, %v", terminal, err) + } + if _, _, err := fileTerminalSize(input); err == nil { + t.Fatal("pipe unexpectedly has a terminal size") + } + if _, err := fileMakeRaw(input); err == nil { + t.Fatal("pipe unexpectedly accepted raw terminal mode") + } + if got := descriptorTestFlags(t, input); got != before { + t.Fatalf("terminal probes changed pipe flags: %#x, want %#x", got, before) + } +} + +func TestDescriptorCallbackDoesNotUseClosedFile(t *testing.T) { + input, _ := descriptorTestPipe(t) + if err := input.Close(); err != nil { + t.Fatal(err) + } + called := false + err := withFileDescriptor(input, func(uintptr) error { called = true; return nil }) + if err == nil || called { + t.Fatalf("closed descriptor callback called=%t, error=%v", called, err) + } +} diff --git a/internal/cli/input.go b/internal/cli/input.go index 09ad2fa..fd87bfd 100644 --- a/internal/cli/input.go +++ b/internal/cli/input.go @@ -10,7 +10,6 @@ import ( "unicode/utf8" "github.com/LeeEirc/tclientlib" - "golang.org/x/term" ) const escapeKey = 0x1d // Ctrl+] @@ -214,13 +213,12 @@ func writeFull(writer io.Writer, data []byte) error { } func readPassword(ctx context.Context, input *os.File, output io.Writer) (password string, afterCR bool, retErr error) { - fd := int(input.Fd()) - state, err := term.MakeRaw(fd) + state, err := fileMakeRaw(input) if err != nil { return "", false, err } defer func() { - if err := term.Restore(fd, state); retErr == nil { + if err := fileRestoreTerminal(input, state); retErr == nil { retErr = err } _, _ = io.WriteString(output, "\r\n") diff --git a/internal/cli/resize.go b/internal/cli/resize.go index 6d4f3d3..bd4a16c 100644 --- a/internal/cli/resize.go +++ b/internal/cli/resize.go @@ -2,8 +2,7 @@ package cli import ( "context" - - "golang.org/x/term" + "os" ) type terminalSize struct { @@ -14,13 +13,13 @@ type terminalSize struct { // watchTerminalSize subscribes before taking its first snapshot. Login may // have taken long enough for the size captured in ClientConfig to be stale; // refreshing here also closes the gap before the signal subscription began. -func watchTerminalSize(ctx context.Context, fd int) (<-chan terminalSize, func()) { +func watchTerminalSize(ctx context.Context, file *os.File) (<-chan terminalSize, func()) { signals, stopSignals := watchResize() watchCtx, cancel := context.WithCancel(ctx) updates := make(chan terminalSize, 1) stopped := make(chan struct{}) refresh := func() { - width, height, err := term.GetSize(fd) + width, height, err := fileTerminalSize(file) if err == nil && width > 0 && height > 0 { queueLatestSize(updates, terminalSize{height: height, width: width}) } diff --git a/internal/cli/run.go b/internal/cli/run.go index bac7e39..8e834ca 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -14,7 +14,6 @@ import ( "time" "github.com/LeeEirc/tclientlib" - "golang.org/x/term" "golang.org/x/text/transform" ) @@ -31,11 +30,16 @@ func Run(args []string, stdin *os.File, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, "tclient:", err) return 2 } - if o.askPass && !term.IsTerminal(int(stdin.Fd())) { + isTTY, err := fileIsTerminal(stdin) + if err != nil { + fmt.Fprintln(stderr, "tclient: inspect stdin:", err) + return 1 + } + if o.askPass && !isTTY { fmt.Fprintln(stderr, "tclient: --ask-pass requires a terminal on stdin") return 2 } - if term.IsTerminal(int(stdin.Fd())) && !supportsTerminal() { + if isTTY && !supportsTerminal() { fmt.Fprintln(stderr, "tclient: interactive terminals are currently supported on Unix; redirected input remains available") return 2 } @@ -83,8 +87,10 @@ func run(ctx context.Context, o options, stdin *os.File, stdout, stderr io.Write } stdout = &lockedWriter{mu: outputMu, writer: stdout} stderr = &lockedWriter{mu: localMu, writer: stderr} - fd := int(stdin.Fd()) - isTTY := term.IsTerminal(fd) + isTTY, err := fileIsTerminal(stdin) + if err != nil { + return fmt.Errorf("inspect stdin: %w", err) + } password := "" passwordCR := false if o.askPass { @@ -133,7 +139,7 @@ func run(ctx context.Context, o options, stdin *os.File, stdout, stderr io.Write config.User, config.Password = o.user, password } if isTTY { - if width, height, err := term.GetSize(fd); err == nil { + if width, height, err := fileTerminalSize(stdin); err == nil { config.Width, config.Height = width, height } } @@ -152,12 +158,12 @@ func run(ctx context.Context, o options, stdin *os.File, stdout, stderr io.Write defer client.Close() var terminal *terminalInput if isTTY { - state, err := term.MakeRaw(fd) + state, err := fileMakeRaw(stdin) if err != nil { return fmt.Errorf("enter terminal raw mode: %w", err) } defer func() { - if err := term.Restore(fd, state); retErr == nil { + if err := fileRestoreTerminal(stdin, state); retErr == nil { retErr = err } }() @@ -168,10 +174,32 @@ func run(ctx context.Context, o options, stdin *os.File, stdout, stderr io.Write func runSession(ctx context.Context, o options, stdin *os.File, output io.Writer, conn net.Conn, client *tclientlib.Client, terminal *terminalInput, flush func() error, cancelOutput context.CancelFunc) error { sessionCtx, cancel := context.WithCancel(ctx) + finishInput := func() error { + if err := flush(); err != nil { + return fmt.Errorf("flush input: %w", err) + } + if o.closeWrite { + halfCloser, ok := conn.(interface{ CloseWrite() error }) + if !ok { + return fmt.Errorf("connection does not support closing the input direction") + } + if err := halfCloser.CloseWrite(); err != nil { + return fmt.Errorf("close input direction: %w", err) + } + } + return nil + } + var sender *inputSender + var sendResult <-chan error + if terminal != nil { + sender = newInputSender(sessionCtx, client, finishInput) + terminal.client = sender + sendResult = sender.result + } var sizes <-chan terminalSize if terminal != nil { var stop func() - sizes, stop = watchTerminalSize(sessionCtx, int(stdin.Fd())) + sizes, stop = watchTerminalSize(sessionCtx, stdin) defer stop() } resizeErrors, resizeStopped := startResizeWorker(sessionCtx, sizes, client.WindowChange) @@ -195,6 +223,9 @@ func runSession(ctx context.Context, o options, stdin *os.File, output io.Writer // input or negotiation may currently be blocked writing to the peer. _ = conn.Close() stopInput(stdin) + if sender != nil { + <-sender.stopped + } <-resizeStopped <-inputStopped <-outputStopped @@ -229,21 +260,19 @@ func runSession(ctx context.Context, o options, stdin *os.File, output io.Writer if err := conn.SetDeadline(deadline); err != nil { return fmt.Errorf("set EOF drain deadline: %w", err) } - if err := flush(); err != nil { - return fmt.Errorf("flush input: %w", err) - } - if o.closeWrite { - halfCloser, ok := conn.(interface{ CloseWrite() error }) - if !ok { - return fmt.Errorf("connection does not support closing the input direction") - } - if err := halfCloser.CloseWrite(); err != nil { - return fmt.Errorf("close input direction: %w", err) - } - } sent = nil timer = time.NewTimer(time.Until(deadline)) drained = timer.C + if sender != nil { + sender.finishInput() + } else if err := finishInput(); err != nil { + return err + } + case err := <-sendResult: + if err != nil { + return fmt.Errorf("send input: %w", err) + } + sendResult = nil case <-drained: return fmt.Errorf("remote output did not finish within %s after stdin EOF", o.drainTimeout) case err := <-resizeErrors: diff --git a/internal/cli/sender.go b/internal/cli/sender.go new file mode 100644 index 0000000..5dc9fec --- /dev/null +++ b/internal/cli/sender.go @@ -0,0 +1,183 @@ +package cli + +import ( + "context" + "errors" + "io" + "sync" + + "github.com/LeeEirc/tclientlib" +) + +const ( + maxPendingInputBytes = 256 * 1024 + maxInputChunk = 4096 +) + +var errInputBacklog = errors.New("pending terminal input exceeded 256 KiB; closing connection (queued input may not have reached the peer)") + +type sendOperation struct { + data []byte + command tclientlib.Command +} + +func (op sendOperation) size() int { + if op.command != 0 { + return 2 // IAC and the standalone command. + } + return len(op.data) +} + +// inputSender accepts terminal input without waiting for network writes. A +// single worker preserves the order of application data and Telnet commands. +// Its byte budget includes the current write as well as queued operations. +// Saturation is an explicit session error: never block local quit parsing or +// silently drop characters and continue with an incomplete command stream. +type inputSender struct { + ctx context.Context + client interactiveClient + finish func() error + + mu sync.Mutex + queue []sendOperation + pendingBytes int + sealed bool + failure error + ready chan struct{} + result chan error + stopped chan struct{} +} + +func newInputSender(ctx context.Context, client interactiveClient, finish func() error) *inputSender { + s := &inputSender{ + ctx: ctx, client: client, finish: finish, + ready: make(chan struct{}, 1), result: make(chan error, 1), stopped: make(chan struct{}), + } + go s.run() + return s +} + +func (s *inputSender) TerminalMode() tclientlib.TerminalMode { return s.client.TerminalMode() } + +func (s *inputSender) Write(p []byte) (int, error) { + total := 0 + for len(p) > 0 { + n := len(p) + if n > maxInputChunk { + n = maxInputChunk + } + if err := s.enqueue(sendOperation{data: p[:n]}); err != nil { + return total, err + } + total += n + p = p[n:] + } + return total, nil +} + +func (s *inputSender) SendCommand(command tclientlib.Command) error { + return s.enqueue(sendOperation{command: command}) +} + +func (s *inputSender) enqueue(op sendOperation) error { + s.mu.Lock() + defer s.mu.Unlock() + if err := s.ctx.Err(); err != nil { + return err + } + if s.failure != nil { + return s.failure + } + if s.sealed { + return io.ErrClosedPipe + } + if op.size() > maxPendingInputBytes-s.pendingBytes { + return errInputBacklog + } + // Adjacent keystrokes can share a pending data chunk. Never append to the + // in-flight operation, or coalesce across a Telnet command boundary. + last := len(s.queue) - 1 + if op.command == 0 && last >= 0 && s.queue[last].command == 0 && len(s.queue[last].data)+len(op.data) <= maxInputChunk { + s.queue[last].data = append(s.queue[last].data, op.data...) + } else { + op.data = append([]byte(nil), op.data...) + s.queue = append(s.queue, op) + } + s.pendingBytes += op.size() + s.wake() + return nil +} + +func (s *inputSender) wake() { + select { + case s.ready <- struct{}{}: + default: + } +} + +// Called only after the input reader reports EOF and the main loop sets the +// drain deadline. The worker flushes/half-closes after every accepted operation. +func (s *inputSender) finishInput() { + s.mu.Lock() + s.sealed = true + s.mu.Unlock() + s.wake() +} + +func (s *inputSender) run() { + defer close(s.stopped) + for { + if s.ctx.Err() != nil { + return + } + s.mu.Lock() + if len(s.queue) == 0 { + sealed := s.sealed + s.mu.Unlock() + if sealed { + if s.ctx.Err() == nil { + s.complete(s.finish()) + } + return + } + select { + case <-s.ctx.Done(): + return + case <-s.ready: + } + continue + } + op := s.queue[0] + s.queue[0] = sendOperation{} + s.queue = s.queue[1:] + if len(s.queue) == 0 { + s.queue = nil + } + s.mu.Unlock() + // Quit cancels pending work. Closing the connection releases any write + // already in flight; do not start another queued operation afterward. + if s.ctx.Err() != nil { + return + } + var err error + if op.command != 0 { + err = s.client.SendCommand(op.command) + } else { + err = writeFull(s.client, op.data) + } + if err != nil { + s.complete(err) + return + } + s.mu.Lock() + s.pendingBytes -= op.size() + s.mu.Unlock() + } +} + +func (s *inputSender) complete(err error) { + s.mu.Lock() + s.failure = err + s.mu.Unlock() + s.result <- err +} diff --git a/internal/cli/sender_linux_test.go b/internal/cli/sender_linux_test.go new file mode 100644 index 0000000..8054e88 --- /dev/null +++ b/internal/cli/sender_linux_test.go @@ -0,0 +1,56 @@ +//go:build linux + +package cli + +import ( + "context" + "io" + "net" + "testing" + "time" + + "github.com/LeeEirc/tclientlib" +) + +func TestBlockedInputWriteDoesNotBlockLocalQuit(t *testing.T) { + master, slave := resizeTestPTY(t, 80, 24) + left, right := net.Pipe() + defer left.Close() + defer right.Close() + conn := &observedWriteConn{Conn: left, writeStarted: make(chan struct{})} + client, err := tclientlib.NewClientConn(conn, &tclientlib.ClientConfig{}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + terminal := &terminalInput{client: client, local: io.Discard, ending: []byte("\r\n"), echo: "off"} + done := make(chan error, 1) + go func() { + done <- runSession(context.Background(), options{drainTimeout: time.Second}, slave, io.Discard, conn, client, terminal, func() error { return nil }, func() {}) + }() + if _, err := master.Write([]byte("x")); err != nil { + t.Fatal(err) + } + select { + case <-conn.writeStarted: + case <-time.After(time.Second): + conn.Close() + <-done + t.Fatal("input did not reach network Write") + } + // This is a distinct stdin read, made only after Write has begun. The + // unresponsive peer never reads, so only closing the session releases it. + if _, err := master.Write([]byte("\x1dquit\r")); err != nil { + t.Fatal(err) + } + select { + case err := <-done: + if err != nil { + t.Fatalf("local quit: %v", err) + } + case <-time.After(time.Second): + conn.Close() + <-done + t.Fatal("blocked input Write prevented local quit from being read") + } +} diff --git a/internal/cli/sender_queue_test.go b/internal/cli/sender_queue_test.go new file mode 100644 index 0000000..7480e9b --- /dev/null +++ b/internal/cli/sender_queue_test.go @@ -0,0 +1,314 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "io" + "reflect" + "sync" + "testing" + "time" + + "github.com/LeeEirc/tclientlib" +) + +type senderQueueEvent struct { + data string + command tclientlib.Command + finish bool +} + +// The first operation waits before reading its data, so tests can fill the +// queue and reuse input buffers while the network writer is still blocked. +// Only the sender worker records events; assertions read them after it stops. +type senderQueueClient struct { + started chan struct{} + release chan struct{} + finishCalled chan struct{} + first sync.Once + events []senderQueueEvent + writeErr error + commandErr error + finishErr error + zeroWrite bool +} + +func (c *senderQueueClient) wait() { + c.first.Do(func() { close(c.started); <-c.release }) +} + +func (c *senderQueueClient) Write(p []byte) (int, error) { + c.wait() + last := len(c.events) - 1 + if last >= 0 && c.events[last].command == 0 && !c.events[last].finish { + c.events[last].data += string(p) + } else { + c.events = append(c.events, senderQueueEvent{data: string(p)}) + } + if c.writeErr != nil || c.zeroWrite { + return 0, c.writeErr + } + return len(p), nil +} + +func (c *senderQueueClient) SendCommand(command tclientlib.Command) error { + c.wait() + c.events = append(c.events, senderQueueEvent{command: command}) + return c.commandErr +} + +func (c *senderQueueClient) TerminalMode() tclientlib.TerminalMode { + return tclientlib.TerminalMode{} +} + +func (c *senderQueueClient) finish() error { + c.events = append(c.events, senderQueueEvent{finish: true}) + close(c.finishCalled) + return c.finishErr +} + +func newSenderQueueTest(t *testing.T, client *senderQueueClient) (*inputSender, context.CancelFunc, func()) { + t.Helper() + client.started, client.release, client.finishCalled = make(chan struct{}), make(chan struct{}), make(chan struct{}) + ctx, cancel := context.WithCancel(context.Background()) + sender := newInputSender(ctx, client, client.finish) + var once sync.Once + release := func() { once.Do(func() { close(client.release) }) } + t.Cleanup(func() { + cancel() + release() + select { + case <-sender.stopped: + case <-time.After(2 * time.Second): + t.Error("sender did not stop during cleanup") + } + }) + return sender, cancel, release +} + +func waitSenderQueueStarted(t *testing.T, client *senderQueueClient) { + t.Helper() + select { + case <-client.started: + case <-time.After(2 * time.Second): + t.Fatal("sender did not start its first operation") + } +} + +func waitSenderQueueStopped(t *testing.T, sender *inputSender) { + t.Helper() + select { + case <-sender.stopped: + case <-time.After(2 * time.Second): + t.Fatal("sender did not stop") + } +} + +func senderQueueResult(t *testing.T, sender *inputSender) error { + t.Helper() + select { + case err := <-sender.result: + waitSenderQueueStopped(t, sender) + return err + case <-time.After(2 * time.Second): + t.Fatal("sender did not report its result") + return nil + } +} + +func queueInput(t *testing.T, sender *inputSender, data []byte) { + t.Helper() + if n, err := sender.Write(data); n != len(data) || err != nil { + t.Fatalf("queue input: accepted %d of %d bytes, error %v", n, len(data), err) + } +} + +func TestSenderQueuePreservesDataCommandAndFinishOrder(t *testing.T) { + client := new(senderQueueClient) + sender, _, release := newSenderQueueTest(t, client) + queueInput(t, sender, []byte("first")) + waitSenderQueueStarted(t, client) + // A multi-chunk Write may be divided or coalesced internally, but its data + // must stay before the command that follows it. + chunked := bytes.Repeat([]byte("a"), maxInputChunk+7) + queueInput(t, sender, chunked) + if err := sender.SendCommand(tclientlib.CommandNoOperation); err != nil { + t.Fatal(err) + } + queueInput(t, sender, []byte("middle")) + if err := sender.SendCommand(tclientlib.CommandAreYouThere); err != nil { + t.Fatal(err) + } + queueInput(t, sender, []byte("last")) + sender.finishInput() + select { + case <-client.finishCalled: + t.Fatal("finish ran before pending input was sent") + default: + } + if n, err := sender.Write([]byte("after EOF")); n != 0 || !errors.Is(err, io.ErrClosedPipe) { + t.Fatalf("input after EOF: %d, %v", n, err) + } + release() + if err := senderQueueResult(t, sender); err != nil { + t.Fatal(err) + } + want := []senderQueueEvent{ + {data: "first" + string(chunked)}, + {command: tclientlib.CommandNoOperation}, + {data: "middle"}, + {command: tclientlib.CommandAreYouThere}, + {data: "last"}, + {finish: true}, + } + if !reflect.DeepEqual(client.events, want) { + t.Fatalf("send order = %#v, want %#v", client.events, want) + } +} + +func TestSenderQueueLimitIncludesInflightData(t *testing.T) { + client := new(senderQueueClient) + sender, _, _ := newSenderQueueTest(t, client) + queueInput(t, sender, make([]byte, maxInputChunk)) + waitSenderQueueStarted(t, client) + remaining := maxPendingInputBytes - maxInputChunk + n, err := sender.Write(make([]byte, remaining+17)) + if n != remaining || !errors.Is(err, errInputBacklog) { + t.Fatalf("overflow: accepted %d bytes, error %v; want %d and errInputBacklog", n, err, remaining) + } + if n, err := sender.Write([]byte("x")); n != 0 || !errors.Is(err, errInputBacklog) { + t.Fatalf("full queue accepted input: %d, %v", n, err) + } + if err := sender.SendCommand(tclientlib.CommandNoOperation); !errors.Is(err, errInputBacklog) { + t.Fatalf("full queue accepted command: %v", err) + } +} + +func TestSenderQueueCommandsUseByteBudget(t *testing.T) { + client := new(senderQueueClient) + sender, _, _ := newSenderQueueTest(t, client) + queueInput(t, sender, make([]byte, maxInputChunk)) + waitSenderQueueStarted(t, client) + queueInput(t, sender, make([]byte, maxPendingInputBytes-maxInputChunk-3)) + if err := sender.SendCommand(tclientlib.CommandNoOperation); err != nil { + t.Fatalf("command should fit with three bytes available: %v", err) + } + if err := sender.SendCommand(tclientlib.CommandAreYouThere); !errors.Is(err, errInputBacklog) { + t.Fatalf("two-byte command fit into one remaining byte: %v", err) + } + queueInput(t, sender, []byte("x")) + if n, err := sender.Write([]byte("y")); n != 0 || !errors.Is(err, errInputBacklog) { + t.Fatalf("command bytes were not counted: %d, %v", n, err) + } +} + +func TestSenderQueueCopiesInputBeforeReturning(t *testing.T) { + client := new(senderQueueClient) + sender, _, release := newSenderQueueTest(t, client) + first := []byte("inflight") + queueInput(t, sender, first) + waitSenderQueueStarted(t, client) + queued := make([]byte, len("queued"), 64) + copy(queued, "queued") + queueInput(t, sender, queued) + suffix := []byte("suffix") + queueInput(t, sender, suffix) + for _, data := range [][]byte{first, queued, suffix} { + for i := range data { + data[i] = 'X' + } + } + sender.finishInput() + release() + if err := senderQueueResult(t, sender); err != nil { + t.Fatal(err) + } + want := []senderQueueEvent{{data: "inflightqueuedsuffix"}, {finish: true}} + if !reflect.DeepEqual(client.events, want) { + t.Fatalf("reusing caller buffers changed queued input: %#v", client.events) + } +} + +func TestSenderQueueCancellationDoesNotStartPendingOperations(t *testing.T) { + client := new(senderQueueClient) + sender, cancel, release := newSenderQueueTest(t, client) + queueInput(t, sender, []byte("inflight")) + waitSenderQueueStarted(t, client) + if err := sender.SendCommand(tclientlib.CommandNoOperation); err != nil { + t.Fatal(err) + } + queueInput(t, sender, []byte("must not start")) + sender.finishInput() + cancel() + if n, err := sender.Write([]byte("after cancel")); n != 0 || !errors.Is(err, context.Canceled) { + t.Fatalf("write after cancellation: %d, %v", n, err) + } + release() + waitSenderQueueStopped(t, sender) + want := []senderQueueEvent{{data: "inflight"}} + if !reflect.DeepEqual(client.events, want) { + t.Fatalf("canceled sender started pending work: %#v", client.events) + } +} + +func TestSenderQueuePropagatesFailures(t *testing.T) { + wantErr := errors.New("send failed") + for _, test := range []struct { + name string + client *senderQueueClient + startCommand bool + want error + }{ + {name: "write", client: &senderQueueClient{writeErr: wantErr}, want: wantErr}, + {name: "command", client: &senderQueueClient{commandErr: wantErr}, startCommand: true, want: wantErr}, + {name: "finish", client: &senderQueueClient{finishErr: wantErr}, want: wantErr}, + {name: "zero-write", client: &senderQueueClient{zeroWrite: true}, want: io.ErrShortWrite}, + } { + t.Run(test.name, func(t *testing.T) { + client := test.client + sender, _, release := newSenderQueueTest(t, client) + if test.startCommand { + if err := sender.SendCommand(tclientlib.CommandNoOperation); err != nil { + t.Fatal(err) + } + } else { + queueInput(t, sender, []byte("first")) + } + waitSenderQueueStarted(t, client) + queueInput(t, sender, []byte("later")) + sender.finishInput() + release() + if err := senderQueueResult(t, sender); !errors.Is(err, test.want) { + t.Fatalf("sender result = %v, want %v", err, test.want) + } + if n, err := sender.Write([]byte("after failure")); n != 0 || !errors.Is(err, test.want) { + t.Fatalf("write after failure: %d, %v", n, err) + } + if err := sender.SendCommand(tclientlib.CommandAreYouThere); !errors.Is(err, test.want) { + t.Fatalf("command after failure: %v", err) + } + if test.name != "finish" && len(client.events) != 1 { + t.Fatalf("sender continued after failure: %#v", client.events) + } + }) + } +} + +func TestSenderQueueSaturationLeavesLocalQuitAvailable(t *testing.T) { + client := new(senderQueueClient) + sender, _, _ := newSenderQueueTest(t, client) + queueInput(t, sender, make([]byte, maxPendingInputBytes)) + waitSenderQueueStarted(t, client) + var local bytes.Buffer + terminal := terminalInput{client: sender, local: &local, ending: []byte("\r\n"), echo: "on"} + if err := terminal.feed([]byte("x")); !errors.Is(err, errInputBacklog) { + t.Fatalf("saturated input: %v", err) + } + if local.Len() != 0 { + t.Fatalf("rejected character was echoed: %q", local.String()) + } + if err := terminal.feed([]byte("\x1dquit\r")); !errors.Is(err, errQuit) { + t.Fatalf("saturated sender prevented local quit: %v", err) + } +} diff --git a/internal/cli/sender_test.go b/internal/cli/sender_test.go new file mode 100644 index 0000000..0514133 --- /dev/null +++ b/internal/cli/sender_test.go @@ -0,0 +1,122 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "io" + "net" + "strings" + "sync" + "testing" + "time" + + "github.com/LeeEirc/tclientlib" +) + +type observedWriteConn struct { + net.Conn + writeStarted chan struct{} + startOnce sync.Once +} + +func (c *observedWriteConn) Write(p []byte) (int, error) { + c.startOnce.Do(func() { close(c.writeStarted) }) + return c.Conn.Write(p) +} + +func TestInputEOFStartsDrainDeadlineBeforeSendCompletes(t *testing.T) { + left, right := net.Pipe() + defer left.Close() + defer right.Close() + // The peer deliberately never reads. In the terminal parsing path, EOF + // must still reach the control loop while application output is blocked. + client, err := tclientlib.NewClientConn(left, &tclientlib.ClientConfig{}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + terminal := &terminalInput{client: client, local: io.Discard, ending: []byte("\r\n"), echo: "off"} + input := testInput(t, []byte("pending input")) + done := make(chan error, 1) + go func() { + done <- runSession(context.Background(), options{drainTimeout: 50 * time.Millisecond}, input, io.Discard, left, client, terminal, func() error { return nil }, func() {}) + }() + select { + case err := <-done: + if err == nil { + t.Fatal("blocked pending input unexpectedly completed successfully") + } + case <-time.After(time.Second): + left.Close() + <-done + t.Fatal("stdin EOF could not start its deadline while network Write was blocked") + } +} + +func TestTerminalInputOverflowFailsSessionExplicitly(t *testing.T) { + left, right := net.Pipe() + defer left.Close() + defer right.Close() + client, err := tclientlib.NewClientConn(left, &tclientlib.ClientConfig{}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + // The writer is blocked, so the entire accepted budget remains pending, + // including whatever chunk the sender already took from the queue. + input := testInput(t, bytes.Repeat([]byte{'x'}, maxPendingInputBytes+1)) + terminal := &terminalInput{client: client, local: io.Discard, ending: []byte("\r\n"), echo: "off"} + done := make(chan error, 1) + go func() { + done <- runSession(context.Background(), options{drainTimeout: time.Second}, input, io.Discard, left, client, terminal, func() error { return nil }, func() {}) + }() + select { + case err := <-done: + if !errors.Is(err, errInputBacklog) || !strings.Contains(err.Error(), "queued input may not have reached the peer") { + t.Fatalf("overflow did not report incomplete delivery: %v", err) + } + case <-time.After(3 * time.Second): + left.Close() + <-done + t.Fatal("terminal queue saturation blocked instead of failing explicitly") + } +} + +func TestTerminalEOFDrainsDataAndCommandsBeforeFlushAndCloseWrite(t *testing.T) { + wire := make(chan []byte, 1) + host, port := testServer(t, func(conn net.Conn) { + data, err := io.ReadAll(conn) + if err != nil { + t.Errorf("server read: %v", err) + } + wire <- data + _, _ = conn.Write([]byte("final output")) + }) + conn, err := net.Dial("tcp", net.JoinHostPort(host, port)) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + client, err := tclientlib.NewClientConn(conn, &tclientlib.ClientConfig{}) + if err != nil { + t.Fatal(err) + } + defer client.Close() + terminal := &terminalInput{client: client, local: io.Discard, ending: []byte("\r\n"), echo: "off"} + input := testInput(t, []byte("one\r\x1dsend nop\rcontinue\rtwo\r")) + var output bytes.Buffer + // Model an encoder's final buffered bytes. The server cannot return its + // final output until both these bytes and TCP CloseWrite have occurred. + flush := func() error { return writeFull(client, []byte("[flush]")) } + o := options{drainTimeout: time.Second, closeWrite: true} + if err := runSession(context.Background(), o, input, &output, conn, client, terminal, flush, func() {}); err != nil { + t.Fatal(err) + } + if got, want := <-wire, []byte("one\r\n\xff\xf1two\r\n[flush]"); !bytes.Equal(got, want) { + t.Fatalf("wire order got %q, want %q", got, want) + } + if output.String() != "final output" { + t.Fatalf("final output was lost: %q", output.String()) + } +} diff --git a/internal/cli/terminal_unix.go b/internal/cli/terminal_unix.go index f71703b..64d4feb 100644 --- a/internal/cli/terminal_unix.go +++ b/internal/cli/terminal_unix.go @@ -27,30 +27,37 @@ func watchResize() (<-chan os.Signal, func()) { } func readInput(ctx context.Context, input *os.File, p []byte) (int, error) { - fd := int(input.Fd()) for { if err := ctx.Err(); err != nil { return 0, err } - poll := []unix.PollFd{{Fd: int32(fd), Events: unix.POLLIN}} - _, err := unix.Poll(poll, 100) - if err == unix.EINTR { - continue - } - if err != nil { - return 0, err - } - if poll[0].Revents == 0 { - continue + if len(p) == 0 { + return 0, nil } - if poll[0].Revents&unix.POLLNVAL != 0 { - return 0, os.ErrClosed - } - n, err := unix.Read(fd, p) + var n int + var ready bool + err := withFileDescriptor(input, func(fd uintptr) error { + poll := []unix.PollFd{{Fd: int32(fd), Events: unix.POLLIN}} + if _, err := unix.Poll(poll, 100); err != nil { + return err + } + if poll[0].Revents == 0 { + return nil + } + if poll[0].Revents&unix.POLLNVAL != 0 { + return os.ErrClosed + } + ready = true + var readErr error + n, readErr = unix.Read(int(fd), p) + return readErr + }) if err == unix.EINTR || err == unix.EAGAIN { continue } - return n, err + if err != nil || ready { + return n, err + } } } @@ -58,7 +65,7 @@ func readInput(ctx context.Context, input *os.File, p []byte) (int, error) { func stopInput(*os.File) {} type descriptorFlags struct { - fd uintptr + file *os.File flags int } @@ -76,29 +83,41 @@ func terminalOutputs(ctx context.Context, stdout, stderr io.Writer) (io.Writer, if !ok { continue } - fd := file.Fd() - if !seen[fd] { - flags, err := unix.FcntlInt(fd, unix.F_GETFL, 0) - if err != nil { - return nil, nil, nil, err + if err := withFileDescriptor(file, func(fd uintptr) error { + if !seen[fd] { + flags, err := unix.FcntlInt(fd, unix.F_GETFL, 0) + if err != nil { + return err + } + saved = append(saved, descriptorFlags{file: file, flags: flags}) + seen[fd] = true } - saved = append(saved, descriptorFlags{fd: fd, flags: flags}) - seen[fd] = true + return nil + }); err != nil { + return nil, nil, nil, err } - writers[i] = &pollWriter{ctx: ctx, fd: int(fd)} + writers[i] = &pollWriter{ctx: ctx, file: file} } restore := func() error { var first error for i := len(saved) - 1; i >= 0; i-- { entry := saved[i] - if _, err := unix.FcntlInt(entry.fd, unix.F_SETFL, entry.flags); err != nil && first == nil { + err := withFileDescriptor(entry.file, func(fd uintptr) error { + _, err := unix.FcntlInt(fd, unix.F_SETFL, entry.flags) + return err + }) + if err != nil && first == nil { first = err } } return first } for _, entry := range saved { - if _, err := unix.FcntlInt(entry.fd, unix.F_SETFL, entry.flags|unix.O_NONBLOCK); err != nil { + err := withFileDescriptor(entry.file, func(fd uintptr) error { + _, err := unix.FcntlInt(fd, unix.F_SETFL, entry.flags|unix.O_NONBLOCK) + return err + }) + if err != nil { _ = restore() return nil, nil, nil, err } @@ -107,8 +126,8 @@ func terminalOutputs(ctx context.Context, stdout, stderr io.Writer) (io.Writer, } type pollWriter struct { - ctx context.Context - fd int + ctx context.Context + file *os.File } func (w *pollWriter) Write(p []byte) (int, error) { @@ -117,7 +136,12 @@ func (w *pollWriter) Write(p []byte) (int, error) { if err := w.ctx.Err(); err != nil { return total, err } - n, err := unix.Write(w.fd, p) + var n int + err := withFileDescriptor(w.file, func(fd uintptr) error { + var writeErr error + n, writeErr = unix.Write(int(fd), p) + return writeErr + }) if n > 0 { total += n p = p[n:] @@ -134,7 +158,10 @@ func (w *pollWriter) Write(p []byte) (int, error) { } continue } - _, err = unix.Poll([]unix.PollFd{{Fd: int32(w.fd), Events: unix.POLLOUT}}, 100) + err = withFileDescriptor(w.file, func(fd uintptr) error { + _, err := unix.Poll([]unix.PollFd{{Fd: int32(fd), Events: unix.POLLOUT}}, 100) + return err + }) if err != nil && err != unix.EINTR { return total, err } diff --git a/scripts/test_cli_pty.py b/scripts/test_cli_pty.py index 577cf9a..2e9fdad 100644 --- a/scripts/test_cli_pty.py +++ b/scripts/test_cli_pty.py @@ -114,9 +114,12 @@ def close(self): class TTYClient: - def __init__(self, binary, port, extra=(), stdout=None): + def __init__(self, binary, port, extra=(), stdout=None, nonblocking=False): self.master, self.slave = pty.openpty() fcntl.ioctl(self.slave, termios.TIOCSWINSZ, struct.pack("HHHH", 24, 80, 0, 0)) + if nonblocking: + flags = fcntl.fcntl(self.slave, fcntl.F_GETFL) + fcntl.fcntl(self.slave, fcntl.F_SETFL, flags | os.O_NONBLOCK) self.original = termios.tcgetattr(self.slave) self.original_flags = fcntl.fcntl(self.slave, fcntl.F_GETFL) self.output = bytearray() @@ -193,11 +196,11 @@ def main(): binary = os.path.abspath(args.binary) results = [] - def failed_dial(): + def failed_dial(nonblocking=False): reserve = socket.socket() reserve.bind(("127.0.0.1", 0)) port = reserve.getsockname()[1] - child = TTYClient(binary, port, ["--timeout", "100ms"]) + child = TTYClient(binary, port, ["--timeout", "100ms"], nonblocking=nonblocking) try: code = child.wait_exit() assert code != 0, "connection failure returned success" @@ -220,9 +223,9 @@ def remote_eof(): child.close() peer.close() - def escape_quit(): + def escape_quit(nonblocking=False): peer = Peer() - child = TTYClient(binary, peer.port) + child = TTYClient(binary, peer.port, nonblocking=nonblocking) try: peer.send(b"READY\r\n") child.wait_output(b"READY") @@ -552,8 +555,10 @@ def hidden_password_utf8_crlf(): for name, fn in [ ("connect_failure_restores_terminal", failed_dial), + ("connect_failure_preserves_nonblocking_terminal", lambda: failed_dial(True)), ("remote_eof_restores_terminal", remote_eof), ("escape_quit_is_local", escape_quit), + ("quit_preserves_nonblocking_terminal", lambda: escape_quit(True)), ("sigterm_restores_terminal", terminated), ("naws_initial_and_resize", naws_resize), ("resize_during_login_is_refreshed", login_resize_refresh),