Remove unused code for keep-alive socks requests and fix socks requests timing out
What changed, and why it matters
This commit removes a reused SOCKS connection pool and forces every network request to use a fresh connection that closes immediately after use. It also extends a connection test timeout from 10 to 20 seconds and cleans up unused proxy-port variables. The stated goal is to fix SOCKS requests that were timing out. There is no direct evidence in the commit of a security vulnerability being patched, but the change does remove a class of risks that connection pools can introduce, such as accidentally sending sensitive request data on a connection previously authenticated under different wallet state.
Treat as a routine reliability/refactoring change unless independent testing shows the old pool could be coerced into cross-request state leakage. Review whether SOCKSSocket.send closes the underlying socket after use, since the new code no longer explicitly closes non-pooled sockets in a finally block. Verify the 20-second timeout is acceptable for Tor and does not mask a deeper hang. No urgent security patch is indicated by the commit alone.
Security signals we found
Removal of connection pooling for SOCKS5 proxy connections
HTTP requests now explicitly use Connection: close instead of keep-alive
Each request gets a fresh socket rather than reusing a potentially stale pooled socket
Timeout for connection test increased from 10 to 20 seconds
Unused proxy-port variables removed from connection settings form
Evidence from the diff
The diff deletes SocksConnectionPool and _PooledConnection, removes the keepAlive parameter from getRawHttpRequestString (now hard-coded to Connection: close), and rewrites makeSocksHttpRequest to always create, connect, send, parse, and presumably close a new SOCKSSocket. It also changes the connection-test timeout from 10s to 20s and simplifies connection_settings_form.dart by dropping the unused torProxyPort and proxyPort variables in favor of customProxyPort. The change is framed as a bug fix for SOCKS timeouts, not as a security fix. No CVE, advisory, or researcher attribution is present in the commit materials.
Changed components
lib/util/socks_http.dartlib/widgets/connection_settings_form.dartSOCKS5 HTTP request pathWallet connection-test logicInspect captured patch +16 / −182
diff --git a/lib/util/socks_http.dart b/lib/util/socks_http.dart
index 5990922..47cfb27 100644
--- a/lib/util/socks_http.dart
+++ b/lib/util/socks_http.dart
@@ -1,114 +1,8 @@
-import 'dart:async';
import 'dart:convert';
import 'dart:io';
-import 'package:skylight_wallet/util/logging.dart';
import 'package:skylight_wallet/util/socks_socket.dart';
-/// Manages a pool of SOCKS connections for reuse.
-///
-/// Connections are keyed by destination (host:port) and proxy info.
-/// This improves performance by avoiding the overhead of establishing
-/// new SOCKS5 connections for each request.
-class SocksConnectionPool {
- static final SocksConnectionPool instance = SocksConnectionPool._();
- SocksConnectionPool._();
-
- final Map<String, _PooledConnection> _connections = {};
-
- /// Maximum idle time before a connection is considered stale
- static const Duration maxIdleTime = Duration(seconds: 30);
-
- String _makeKey(String host, int port, String proxyHost, int proxyPort, bool ssl) {
- return '$proxyHost:$proxyPort->$host:$port:${ssl ? 'ssl' : 'plain'}';
- }
-
- /// Get or create a connection for the given destination.
- Future<SOCKSSocket> getConnection({
- required String host,
- required int port,
- required String proxyHost,
- required int proxyPort,
- required bool sslEnabled,
- }) async {
- final key = _makeKey(host, port, proxyHost, proxyPort, sslEnabled);
-
- // Check if we have a valid existing connection
- final existing = _connections[key];
- if (existing != null && !existing.isStale) {
- existing.lastUsed = DateTime.now();
- return existing.socket;
- }
-
- // Close stale connection if exists
- if (existing != null) {
- log(LogLevel.info, 'Closing stale SOCKS connection to $host:$port');
- await _closeConnection(key);
- }
-
- // Create new connection
- log(LogLevel.info, 'Creating new SOCKS connection to $host:$port');
- final socket = await SOCKSSocket.create(
- proxyHost: proxyHost,
- proxyPort: proxyPort,
- sslEnabled: sslEnabled,
- );
-
- await socket.connect();
- await socket.connectTo(host, port);
-
- _connections[key] = _PooledConnection(socket: socket);
- return socket;
- }
-
- /// Remove a connection from the pool (e.g., on error).
- Future<void> removeConnection({
- required String host,
- required int port,
- required String proxyHost,
- required int proxyPort,
- required bool sslEnabled,
- }) async {
- final key = _makeKey(host, port, proxyHost, proxyPort, sslEnabled);
- await _closeConnection(key);
- }
-
- Future<void> _closeConnection(String key) async {
- final conn = _connections.remove(key);
- if (conn != null) {
- try {
- await conn.socket.close();
- } catch (_) {}
- }
- }
-
- /// Close all connections in the pool.
- Future<void> closeAll() async {
- for (final key in _connections.keys.toList()) {
- await _closeConnection(key);
- }
- }
-
- /// Clean up stale connections.
- Future<void> cleanup() async {
- final staleKeys = _connections.entries.where((e) => e.value.isStale).map((e) => e.key).toList();
-
- for (final key in staleKeys) {
- log(LogLevel.info, 'Cleaning up stale connection: $key');
- await _closeConnection(key);
- }
- }
-}
-
-class _PooledConnection {
- final SOCKSSocket socket;
- DateTime lastUsed;
-
- _PooledConnection({required this.socket}) : lastUsed = DateTime.now();
-
- bool get isStale => DateTime.now().difference(lastUsed) > SocksConnectionPool.maxIdleTime;
-}
-
class ParsedHttpResponse {
final String httpVersion;
final int statusCode;
@@ -140,12 +34,7 @@ Decoded JSON: $jsonBody
}
}
-String getRawHttpRequestString(
- String method,
- String url, {
- Object? jsonBody,
- bool keepAlive = true,
-}) {
+String getRawHttpRequestString(String method, String url, {Object? jsonBody}) {
final uri = Uri.parse(url);
final host = uri.host;
@@ -157,7 +46,7 @@ String getRawHttpRequestString(
request.write('${method.toUpperCase()} $fullPath HTTP/1.1\r\n');
request.write('Host: $host\r\n');
- request.write('Connection: ${keepAlive ? 'keep-alive' : 'close'}\r\n');
+ request.write('Connection: close\r\n');
request.write('Accept: */*\r\n');
final jsonBodyStr = jsonBody is Object ? jsonBody.toString() : null;
@@ -230,73 +119,21 @@ Future<ParsedHttpResponse> makeSocksHttpRequest(
String url,
({InternetAddress host, int port}) proxyInfo, {
Object? body,
- bool? usePool,
}) async {
final uri = Uri.parse(url);
- final pool = SocksConnectionPool.instance;
- final host = uri.host;
- final port = uri.port;
- final proxyHost = proxyInfo.host.address;
- final proxyPort = proxyInfo.port;
- final sslEnabled = uri.scheme == 'https';
- // Only use connection pooling on iOS by default (helps with iOS networking limitations)
- final shouldUsePool = usePool ?? Platform.isIOS;
+ final socket = await SOCKSSocket.create(
+ proxyHost: proxyInfo.host.address,
+ proxyPort: proxyInfo.port,
+ sslEnabled: uri.scheme == 'https',
+ );
- SOCKSSocket? socket;
- bool fromPool = false;
+ await socket.connect();
+ await socket.connectTo(uri.host, uri.port);
- try {
- if (shouldUsePool) {
- socket = await pool.getConnection(
- host: host,
- port: port,
- proxyHost: proxyHost,
- proxyPort: proxyPort,
- sslEnabled: sslEnabled,
- );
- fromPool = true;
- } else {
- socket = await SOCKSSocket.create(
- proxyHost: proxyHost,
- proxyPort: proxyPort,
- sslEnabled: sslEnabled,
- );
- await socket.connect();
- await socket.connectTo(host, port);
- }
+ final rawRequest = getRawHttpRequestString(method, url, jsonBody: body);
+ final rawResponse = await socket.send(rawRequest);
+ final parsedResponse = parseHttpResponse(rawResponse);
- final rawRequest = getRawHttpRequestString(
- method,
- url,
- jsonBody: body,
- keepAlive: shouldUsePool,
- );
- final rawResponse = await socket.sendHttpRequest(rawRequest);
- final parsedResponse = parseHttpResponse(rawResponse);
-
- return parsedResponse;
- } catch (e) {
- log(LogLevel.error, 'makeSocksHttpRequest error: $e');
- // Remove the connection from pool on error
- if (fromPool) {
- await pool.removeConnection(
- host: host,
- port: port,
- proxyHost: proxyHost,
- proxyPort: proxyPort,
- sslEnabled: sslEnabled,
- );
- }
- rethrow;
- } finally {
- // Only close if not using pool
- if (!shouldUsePool && socket != null) {
- try {
- await socket.close();
- } catch (e) {
- log(LogLevel.error, 'Error closing socket: $e');
- }
- }
- }
+ return parsedResponse;
}
diff --git a/lib/widgets/connection_settings_form.dart b/lib/widgets/connection_settings_form.dart
index 79877b9..50f1b16 100644
--- a/lib/widgets/connection_settings_form.dart
+++ b/lib/widgets/connection_settings_form.dart
@@ -162,7 +162,6 @@ class _ConnectionSettingsFormState extends State<ConnectionSettingsForm> {
final proto = _useSsl ? 'https' : 'http';
final daemonAddress = cleanAddress(_addressController.text);
final customProxyPort = _customProxyPortController.text;
- String torProxyPort = '';
// Handle demo mode
if (isDemoMode) {
@@ -175,8 +174,6 @@ class _ConnectionSettingsFormState extends State<ConnectionSettingsForm> {
}
}
- String proxyPort = torProxyPort != '' ? torProxyPort : customProxyPort;
-
setState(() {
_hasTested = true;
_connectionTestIsLoading = true;
@@ -206,7 +203,7 @@ class _ConnectionSettingsFormState extends State<ConnectionSettingsForm> {
'POST',
url,
proxyInfo!,
- ).timeout(Duration(seconds: 10));
+ ).timeout(Duration(seconds: 20));
setState(() {
_connectionSuccess = response.statusCode == HttpStatus.internalServerError;
@@ -214,10 +211,10 @@ class _ConnectionSettingsFormState extends State<ConnectionSettingsForm> {
} else {
var httpClient = HttpClient();
- if (proxyPort != '') {
+ if (customProxyPort != '') {
httpClient = httpClient
..findProxy = (uri) {
- return "PROXY localhost:$proxyPort";
+ return "PROXY localhost:$customProxyPort";
};
}
Why this scored 32/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.