From c043b4edd2f5602b535982c5cef85aa1db7357d0 Mon Sep 17 00:00:00 2001 From: dirtywater97 Date: Mon, 27 Jul 2026 18:15:27 -0400 Subject: [PATCH] fix(mobile): detect half-open relay sockets with client-side ping/pong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The relay pings every 30s and closes after 3 missed pongs, which protects the relay from dead clients but does nothing for the client. Mobile networks silently drop idle TCP flows without sending a FIN, so a dead socket still reports as open: `onDone`/`onError` never fire, the session notifier is never told to reconnect, and the connection can stay dead until some later network event or an app restart. `WebSocketChannel.connect` resolves to a platform-adaptive implementation that exposes no ping configuration. `IOWebSocketChannel` does: an unanswered ping closes the socket with `goingAway`, which reaches the existing `onDone` handler and drives the normal reconnect path, so no additional lifecycle handling is required. The mobile app targets Android and iOS only, so binding to the `dart:io` implementation is safe. `dart:io` never has two pings outstanding, so detection takes up to two intervals — about 40s at 20s, keeping foreground recovery under a minute. The session already disconnects 5s after backgrounding, so the added probe traffic does not run while the app is suspended. The connector is isolated in its own file so `relay_socket.dart` changes by one call site plus an import. Test covers the real failure: a server that completes the WebSocket handshake then goes silent, never answering pings and never sending a close frame. It asserts the socket closes cleanly through `onDone` with close code 1001 rather than erroring. Verified to fail against `WebSocketChannel.connect` (socket stays open indefinitely) and pass with ping/pong enabled. Refs #3224 Signed-off-by: dirtywater97 --- mobile/lib/shared/relay/relay_socket.dart | 3 +- .../relay/relay_websocket_connector.dart | 41 +++++ .../relay/relay_websocket_connector_test.dart | 146 ++++++++++++++++++ 3 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 mobile/lib/shared/relay/relay_websocket_connector.dart create mode 100644 mobile/test/shared/relay/relay_websocket_connector_test.dart diff --git a/mobile/lib/shared/relay/relay_socket.dart b/mobile/lib/shared/relay/relay_socket.dart index 267b030391..289e2c90ef 100644 --- a/mobile/lib/shared/relay/relay_socket.dart +++ b/mobile/lib/shared/relay/relay_socket.dart @@ -6,6 +6,7 @@ import 'package:nostr/nostr.dart' as nostr; import 'package:web_socket_channel/web_socket_channel.dart'; import 'nostr_models.dart'; +import 'relay_websocket_connector.dart'; /// Low-level websocket connection with NIP-42 authentication. /// @@ -63,7 +64,7 @@ class RelaySocket { _state = SocketState.connecting; try { - _channel = WebSocketChannel.connect(Uri.parse(_wsUrl)); + _channel = connectRelayWebSocket(Uri.parse(_wsUrl)); await _channel!.ready; } catch (e) { _state = SocketState.disconnected; diff --git a/mobile/lib/shared/relay/relay_websocket_connector.dart b/mobile/lib/shared/relay/relay_websocket_connector.dart new file mode 100644 index 0000000000..b3eae60496 --- /dev/null +++ b/mobile/lib/shared/relay/relay_websocket_connector.dart @@ -0,0 +1,41 @@ +import 'package:web_socket_channel/io.dart'; +import 'package:web_socket_channel/web_socket_channel.dart'; + +/// How often to send a WebSocket ping frame, and how long to wait for the +/// matching pong before declaring the socket dead. +/// +/// The relay pings every 30s and closes after 3 missed pongs +/// (`crates/buzz-relay/src/connection.rs`). That protects the relay from dead +/// clients; it does nothing for the client. Mobile networks — cellular in +/// particular — silently drop idle TCP flows without sending a FIN, so an +/// unused socket can stay dead while the client still reports it as open, and +/// nothing surfaces it until some later network event or an app restart. +/// +/// `dart:io` never has two pings outstanding: it waits one interval, sends a +/// ping, then allows one more interval for the pong. Detection therefore takes +/// up to `2 * relayPingInterval` — about 40s at this value, which keeps +/// recovery under a minute while the app is foregrounded. The cost is three +/// extra ping/pong exchanges per idle foreground minute; the session already +/// disconnects 5s after being backgrounded +/// ([RelaySessionNotifier], `relay_session.dart`), so this does not run while +/// the app is suspended. Raising it to 30s would halve the probe traffic and +/// roughly double time-to-detection. +const relayPingInterval = Duration(seconds: 20); + +/// Opens the relay WebSocket with client-side liveness probing enabled. +/// +/// [WebSocketChannel.connect] resolves to a platform-adaptive implementation +/// that exposes no ping configuration, so a half-open socket never surfaces as +/// a close event. [IOWebSocketChannel] does support it: when a ping goes +/// unanswered for [relayPingInterval] the socket closes with `goingAway`, +/// which reaches the existing `onDone` handler and drives the normal reconnect +/// path — no additional lifecycle handling required. +/// +/// The mobile app targets Android and iOS only (`mobile/` has no web target), +/// so binding to the `dart:io` implementation is safe here. +WebSocketChannel connectRelayWebSocket( + Uri uri, { + Duration pingInterval = relayPingInterval, +}) { + return IOWebSocketChannel.connect(uri, pingInterval: pingInterval); +} diff --git a/mobile/test/shared/relay/relay_websocket_connector_test.dart b/mobile/test/shared/relay/relay_websocket_connector_test.dart new file mode 100644 index 0000000000..01966f9505 --- /dev/null +++ b/mobile/test/shared/relay/relay_websocket_connector_test.dart @@ -0,0 +1,146 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:pointycastle/digests/sha1.dart'; +import 'package:buzz/shared/relay/relay_websocket_connector.dart'; + +/// A WebSocket server that completes the handshake and then goes silent — +/// it never answers a ping, and never sends a close frame. +/// +/// This is the shape of the failure being fixed: a carrier NAT or a dozing +/// radio drops the flow without either peer sending a FIN, so the socket looks +/// open to the client forever. It is built on a raw [ServerSocket] because +/// `dart:io`'s own `WebSocket` auto-replies to pings, which is precisely the +/// behaviour we need to withhold. +class _SilentWebSocketServer { + _SilentWebSocketServer._(this._server); + + final ServerSocket _server; + final List _clients = []; + + static const _handshakeGuid = '258EAFA5-E914-47DA-95CA-C5AB0DC85B11'; + + int get port => _server.port; + + Uri get uri => Uri.parse('ws://127.0.0.1:$port'); + + static Future<_SilentWebSocketServer> start() async { + final server = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0); + final instance = _SilentWebSocketServer._(server); + server.listen(instance._handleClient); + return instance; + } + + void _handleClient(Socket socket) { + _clients.add(socket); + final buffer = StringBuffer(); + var upgraded = false; + + socket.listen( + (data) { + // Once upgraded, every inbound frame — including pings — is discarded. + if (upgraded) return; + + buffer.write(latin1.decode(data)); + final request = buffer.toString(); + if (!request.contains('\r\n\r\n')) return; + + final key = RegExp( + r'sec-websocket-key:\s*(\S+)', + caseSensitive: false, + ).firstMatch(request)?.group(1); + if (key == null) { + socket.destroy(); + return; + } + + final accept = base64.encode( + SHA1Digest().process(latin1.encode('$key$_handshakeGuid')), + ); + socket.add( + latin1.encode( + 'HTTP/1.1 101 Switching Protocols\r\n' + 'Upgrade: websocket\r\n' + 'Connection: Upgrade\r\n' + 'Sec-WebSocket-Accept: $accept\r\n' + '\r\n', + ), + ); + upgraded = true; + }, + onError: (_) {}, + cancelOnError: true, + ); + } + + Future close() async { + for (final client in _clients) { + client.destroy(); + } + await _server.close(); + } +} + +void main() { + test( + 'closes a silent socket once a ping goes unanswered', + () async { + final server = await _SilentWebSocketServer.start(); + addTearDown(server.close); + + final channel = connectRelayWebSocket( + server.uri, + pingInterval: const Duration(milliseconds: 200), + ); + await channel.ready; + + final done = Completer(); + Object? streamError; + channel.stream.listen( + (_) {}, + onError: (Object error) { + streamError = error; + if (!done.isCompleted) done.complete(); + }, + onDone: () { + if (!done.isCompleted) done.complete(); + }, + ); + + // The socket is healthy at the TCP level and the server never hangs up, + // so without client-side ping/pong this never completes. + await done.future.timeout( + const Duration(seconds: 5), + onTimeout: () => fail( + 'socket stayed open with no pong — client-side liveness probing is ' + 'not active', + ), + ); + + // RelaySocket distinguishes an unanswered ping from a transport failure: + // the former must arrive as a clean close through `onDone`, which is the + // path that drives reconnection. + expect( + streamError, + isNull, + reason: 'unanswered ping should close cleanly, not raise onError', + ); + expect( + channel.closeCode, + WebSocketStatus.goingAway, + reason: 'dart:io closes a ping-timed-out socket with goingAway (1001)', + ); + }, + // Guards against a regression silently reintroducing an unbounded wait. + timeout: const Timeout(Duration(seconds: 20)), + ); + + test('defaults to a ping interval inside the relay heartbeat window', () { + // The relay closes after 3 missed 30s pongs; probing must be frequent + // enough to detect a dead socket well before that. + expect(relayPingInterval, lessThan(const Duration(seconds: 30))); + expect(relayPingInterval, greaterThan(Duration.zero)); + }); +}