diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c8d1ff..ec54907 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## Unreleased +- Added `SSHClient.disconnect`, which sends `SSH_MSG_DISCONNECT` (BY_APPLICATION) before closing the socket, so the peer sees a protocol-level disconnect instead of inferring the teardown from TCP alone. RFC 4253 §11.1 says implementations SHOULD send this message, and OpenSSH logs a `BY_APPLICATION` teardown as info rather than an error-level connection loss, so a deliberate client-side close stops showing up on the server as an unexplained drop. `close()` is unchanged and remains the right call on error paths [#250]. + ## [4.1.0] - 2026-09-04 - Added `SSHClient.pipelineChannelRequests`, off by default, which sends all of a session's channel requests before reading any reply instead of waiting for each one in turn. `execute` and `shell` send `env`, agent forwarding, `pty-req` and `x11-req` ahead of `exec` or `shell`, and each of them cost a round trip: against a server 40 ms away, `execute` with a pty measured 246 ms before and 206 ms after. RFC 4254 §5.4 permits sending further messages without waiting and §4 requires the peer to answer a channel's requests in the order it received them, which is what `ssh(1)` relies on when it does the same thing. Setting it also adopts OpenSSH's reporting, because it has to: the command is on the wire before a refusal can come back, so a refused `pty-req`, `env`, agent forwarding or `x11-req` is reported through `printDebug` and the command runs, the way `ssh(1)` prints `PTY allocation request failed on channel 0` and carries on, rather than throwing an error that means the command has already run. Only a refused `exec` or `shell` still throws `SSHChannelRequestError`, because nothing has run when that one fails. Leaving it unset changes nothing, down to the order the requests go out and the message of every error [#243]. - Added a `dartssh2-nopty` account to the interop server, which `PermitTTY no` applies to, so both sides of a refused `pty-req` are exercised against a real OpenSSH: by default the command does not run, and with pipelining it does [#243]. diff --git a/lib/src/ssh_client.dart b/lib/src/ssh_client.dart index bd344b4..fd755db 100644 --- a/lib/src/ssh_client.dart +++ b/lib/src/ssh_client.dart @@ -20,6 +20,7 @@ import 'package:dartssh2/src/utils/auth_methods.dart'; import 'package:dartssh2/src/utils/pending_requests.dart'; import 'package:dartssh2/src/utils/terminal_state.dart'; import 'package:dartssh2/src/message/msg_channel.dart'; +import 'package:dartssh2/src/message/msg_disconnect.dart'; import 'package:dartssh2/src/message/msg_request.dart'; import 'package:dartssh2/src/message/msg_service.dart'; import 'package:dartssh2/src/message/msg_userauth.dart'; @@ -902,6 +903,33 @@ class SSHClient { await _transport.close(); } + /// Sends `SSH_MSG_DISCONNECT` (BY_APPLICATION) and then closes, so the peer + /// sees a protocol-level disconnect instead of inferring the teardown from + /// TCP alone. + /// + /// Prefer this over [close] when the connection is ending by deliberate + /// client choice (user closes the tab, app shuts down) rather than by + /// failure: RFC 4253 §11.1 asks for this message, and OpenSSH logs the + /// teardown as info (`BY_APPLICATION`) instead of an error-level connection + /// loss. [close] remains appropriate for error paths, where tearing the + /// socket down without a farewell is fine. + /// + /// Fire-and-forget safe: a transport that already died falls through to + /// [close]. + Future disconnect() async { + try { + if (!_transport.isClosed) { + _sendMessage( + SSH_Message_Disconnect.fromReason(SSHDisconnectReason.byApplication), + ); + await _transport.flush(); + } + } on Exception { + // The transport may already be tearing down; the close below is enough. + } + await close(); + } + /// Force flush any buffered outgoing data to the socket. Future flush() async { await _transport.flush(); diff --git a/test/src/ssh_client_disconnect_test.dart b/test/src/ssh_client_disconnect_test.dart new file mode 100644 index 0000000..4fb7d47 --- /dev/null +++ b/test/src/ssh_client_disconnect_test.dart @@ -0,0 +1,120 @@ +import 'dart:async'; +import 'dart:typed_data'; + +import 'package:dartssh2/dartssh2.dart'; +import 'package:dartssh2/src/message/msg_disconnect.dart'; +import 'package:test/test.dart'; + +void main() { + group('SSHClient.disconnect', () { + test('sends SSH_MSG_DISCONNECT before closing', () async { + final socket = _CaptureSSHSocket(); + final client = SSHClient(socket, username: 'test'); + + await client.disconnect(); + + expect(client.isClosed, isTrue); + final message = _findDisconnectMessage(socket.packets); + expect(message, isNotNull, + reason: 'a disconnect packet must reach the wire before the close'); + expect(message!.reasonCode, SSHDisconnectReason.byApplication.code); + expect( + message.description, SSHDisconnectReason.byApplication.description); + }); + + test('is a no-op on an already closed client', () async { + final socket = _CaptureSSHSocket(); + final client = SSHClient(socket, username: 'test'); + await client.close(); + + await client.disconnect(); + + expect(client.isClosed, isTrue); + }); + }); +} + +/// Scans raw socket writes for an unencrypted `SSH_MSG_DISCONNECT` packet. +/// +/// Before the first key exchange packets are sent in the clear, so the wire +/// format is directly parseable: 4-byte length, 1-byte padding length, then +/// the payload. Writes that are not binary packets (the version line) are +/// skipped by the length sanity check. +SSH_Message_Disconnect? _findDisconnectMessage(List writes) { + for (final write in writes) { + if (write.length < 6) continue; + final payloadLength = + write[0] << 24 | write[1] << 16 | write[2] << 8 | write[3]; + if (payloadLength < 1 || payloadLength + 4 > write.length) continue; + final payload = write.sublist(5, 4 + payloadLength); + if (payload.isEmpty || payload[0] != SSH_Message_Disconnect.messageId) { + continue; + } + return SSH_Message_Disconnect.decode( + Uint8List.fromList( + [SSH_Message_Disconnect.messageId, ...payload.sublist(1)]), + ); + } + return null; +} + +class _CaptureSSHSocket implements SSHSocket { + final _inputController = StreamController(); + final _doneCompleter = Completer(); + final packets = []; + + @override + Stream get stream => _inputController.stream; + + @override + StreamSink> get sink => _CaptureSink(packets); + + @override + Future get done => _doneCompleter.future; + + @override + Future close() async { + if (!_doneCompleter.isCompleted) { + _doneCompleter.complete(); + } + await _inputController.close(); + } + + @override + void destroy() { + if (!_doneCompleter.isCompleted) { + _doneCompleter.complete(); + } + unawaited(_inputController.close()); + } + + @override + Future flush() async {} +} + +class _CaptureSink implements StreamSink> { + _CaptureSink(this._packets); + + final List _packets; + + @override + void add(List data) { + _packets.add(Uint8List.fromList(data)); + } + + @override + Future addStream(Stream> stream) async { + await for (final chunk in stream) { + add(chunk); + } + } + + @override + void addError(Object error, [StackTrace? stackTrace]) {} + + @override + Future close() async {} + + @override + Future get done async {} +}