diff --git a/FmiSrl.FtpServer.Server/Commands/PasvCommand.cs b/FmiSrl.FtpServer.Server/Commands/PasvCommand.cs index d188c63..bbdd793 100644 --- a/FmiSrl.FtpServer.Server/Commands/PasvCommand.cs +++ b/FmiSrl.FtpServer.Server/Commands/PasvCommand.cs @@ -31,6 +31,7 @@ private static async Task PrepareDataConnectionAsync(FtpCommandContext context) } var clientIp = (context.Session.RemoteEndPoint as IPEndPoint)?.Address.ToString() ?? ""; + Console.WriteLine($"[PASV-V2] PasvCommand: ListeningIp='{context.Configuration.ListeningIp}', clientIp='{clientIp}', portRange={context.Configuration.PasvMinPort}-{context.Configuration.PasvMaxPort}"); var dataConnection = new MikuPassiveDataConnection( context.Configuration.ListeningIp, @@ -50,6 +51,7 @@ private static async Task SendPassiveModeResponseAsync(FtpCommandContext context var p1 = port / 256; var p2 = port % 256; + Console.WriteLine($"[PASV-V2] PasvCommand: advertising 227 with IP={localIp}, port={port} ({p1},{p2})"); await context.Session.SendResponseAsync(227, $"Entering Passive Mode ({localIp},{p1},{p2})."); } diff --git a/FmiSrl.FtpServer.Server/FtpServer.cs b/FmiSrl.FtpServer.Server/FtpServer.cs index 265656a..90ec803 100644 --- a/FmiSrl.FtpServer.Server/FtpServer.cs +++ b/FmiSrl.FtpServer.Server/FtpServer.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Text; using Miku.Core; @@ -40,7 +41,7 @@ public class FtpServer( private readonly ILogger _logger = logger ?? NullLogger.Instance; private readonly FtpCommandHandler _commandHandler = new(middlewares); - private readonly Dictionary _sessions = []; + private readonly ConcurrentDictionary _sessions = new(); private NetServer? _netServer; @@ -121,9 +122,16 @@ private async Task HandleClientDataReceivedAsync(NetClient client, ReadOnlyMemor } var ftpSession = (FtpSession)session; - using (await ftpSession.LockSessionAsync()) + try { - await ProcessClientDataAsync(client, ftpSession, data); + using (await ftpSession.LockSessionAsync()) + { + await ProcessClientDataAsync(client, ftpSession, data); + } + } + catch (Exception ex) + { + _logger.LogError(ex, "Unhandled exception while processing data for client {Id}", client.Id); } } @@ -137,7 +145,7 @@ private async Task ProcessClientDataAsync(NetClient client, FtpSession session, { _logger.LogWarning("Command buffer exceeded maximum length for client {Id}. Disconnecting.", client.Id); await session.SendResponseAsync(500, "Line too long."); - HandleExplicitQuit(client); + await HandleExplicitQuitAsync(client, session); return; } @@ -187,7 +195,7 @@ private async Task ExecuteCommandAsync(NetClient client, FtpSession session, str if (verb == "QUIT") { - HandleExplicitQuit(client); + await HandleExplicitQuitAsync(client, session); } } @@ -199,17 +207,53 @@ private static (string Verb, string Args) ParseRequest(string rawRequest) return (verb, args); } - private void HandleExplicitQuit(NetClient client) + private async Task HandleExplicitQuitAsync(NetClient client, FtpSession session) { _logger.LogInformation("Client disconnected explicitly: {Id}", client.Id); - _sessions.Remove(client.Id); + _sessions.TryRemove(client.Id, out _); + await DisposeDataConnectionAsync(session, client.Id); client.Stop(); } - private void HandleClientDisconnected(NetClient client, string reason) + private async void HandleClientDisconnected(NetClient client, string reason) { _logger.LogInformation("Client disconnected: {Id}. Reason: {Reason}", client.Id, reason); - _sessions.Remove(client.Id); + if (!_sessions.TryRemove(client.Id, out var session)) + { + return; + } + + var ftpSession = (FtpSession)session; + try + { + using (await ftpSession.LockSessionAsync()) + { + await DisposeDataConnectionAsync(ftpSession, client.Id); + } + } + catch (Exception ex) + { + _logger.LogError(ex, "Error during session cleanup for client {Id}", client.Id); + } + } + + private async Task DisposeDataConnectionAsync(FtpSession session, int clientId) + { + var dataConnection = session.DataConnection; + if (dataConnection is null) + { + return; + } + + session.DataConnection = null; + try + { + await dataConnection.DisposeAsync(); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to dispose data connection for session {Id}", clientId); + } } private void HandleError(Exception exception) @@ -222,19 +266,42 @@ private void HandleError(Exception exception) /// /// A task that represents the asynchronous operation of stopping the server. /// - /// Stops the underlying and clears the server state. + /// Stops the underlying , disposes any passive data connections + /// still associated with live sessions, and clears the server state. /// - public Task StopAsync() + public async Task StopAsync() { - if (_netServer is null) + var netServer = _netServer; + if (netServer is null) { - return Task.CompletedTask; + return; } - - _netServer.Stop(); _netServer = null; - _logger.LogInformation("FTP Server stopped."); - return Task.CompletedTask; + // Snapshot sessions before Miku's Stop can start racing OnClientDisconnected callbacks + // against us. Data-connection DisposeAsync is idempotent, so double-disposal is safe. + var sessions = _sessions.Values.ToList(); + + netServer.Stop(); + + foreach (var session in sessions) + { + if (session is not FtpSession ftpSession) + { + continue; + } + + try + { + await DisposeDataConnectionAsync(ftpSession, 0); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Error disposing data connection during shutdown for session {Id}", ftpSession.Id); + } + } + + _sessions.Clear(); + _logger.LogInformation("FTP Server stopped."); } } diff --git a/FmiSrl.FtpServer.Server/Infrastructure/MikuPassiveDataConnection.cs b/FmiSrl.FtpServer.Server/Infrastructure/MikuPassiveDataConnection.cs index cc6ad85..141cf93 100644 --- a/FmiSrl.FtpServer.Server/Infrastructure/MikuPassiveDataConnection.cs +++ b/FmiSrl.FtpServer.Server/Infrastructure/MikuPassiveDataConnection.cs @@ -1,97 +1,146 @@ using System.Net; +using System.Net.NetworkInformation; using System.Net.Sockets; -using Miku.Core; using FmiSrl.FtpServer.Server.Abstractions; namespace FmiSrl.FtpServer.Server.Infrastructure; /// -/// Provides an implementation of that uses a passive connection. +/// Provides an implementation of that listens for a passive +/// data connection using a plain and exposes the accepted socket +/// as a . The name is retained for backwards compatibility. /// public class MikuPassiveDataConnection : IFtpDataConnection { - private NetServer? _dataServer; - private MikuClientStream? _stream; - private readonly TaskCompletionSource _streamTcs = new(); + private readonly TcpListener _listener; + private readonly string _authorizedClientIp; + private readonly CancellationTokenSource _cts = new(); + private TcpClient? _acceptedClient; + private NetworkStream? _networkStream; + private bool _disposed; /// - /// Gets the port used for the passive data connection. + /// Gets the port the data listener is bound to. /// - /// The port number as an . public int Port { get; } /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class and + /// binds a listener on the first available port in the configured range. /// - /// The IP address to bind the data server to. - /// The minimum port number in the range of allowed ports. - /// The maximum port number in the range of allowed ports. - /// The IP address authorized to connect to this data port. - public MikuPassiveDataConnection( - string ip, - int minPort, - int maxPort, - string authorizedClientIp - ) + /// The IP address to bind the data listener to. + /// Minimum port in the passive range. + /// Maximum port in the passive range. + /// The only client IP that may complete the data connection. + public MikuPassiveDataConnection(string ip, int minPort, int maxPort, string authorizedClientIp) { - // Try to find a free port in the specified range - var bound = false; + _authorizedClientIp = authorizedClientIp; + var bindAddress = IPAddress.TryParse(ip, out var parsed) ? parsed : IPAddress.Any; - for (var portToTry = minPort; portToTry <= maxPort; portToTry++) + _listener = BindInRange(bindAddress, minPort, maxPort); + + // If anything below throws, release the bound listener before the exception escapes; + // otherwise the partially-initialised instance leaks a socket that only closes on GC + // (and TcpListener has no finalizer that will close it for us). + try + { + Port = ((IPEndPoint)_listener.LocalEndpoint).Port; + Console.WriteLine($"[PASV-V2] TcpListener bound on {bindAddress}:{Port} (authorized={authorizedClientIp})"); + } + catch + { + try { _listener.Stop(); } catch { /* best effort */ } + throw; + } + } + + private static TcpListener BindInRange(IPAddress address, int minPort, int maxPort) + { + // On Windows, a specific-IP listener (e.g. 127.0.0.1:port) shadows a wildcard + // listener (0.0.0.0:port) for connections to that specific IP. `TcpListener.Start()` + // still succeeds on the wildcard, but the client's connection is delivered to the + // more-specific socket. Enumerate all active listeners and skip any port that + // already has a binding on any IP. + var busyPorts = new HashSet( + IPGlobalProperties.GetIPGlobalProperties() + .GetActiveTcpListeners() + .Select(ep => ep.Port)); + + for (var port = minPort; port <= maxPort; port++) { + if (busyPorts.Contains(port)) + { + continue; + } + try { - var listener = new TcpListener(IPAddress.Any, portToTry); + var listener = new TcpListener(address, port); listener.Start(); - Port = ((IPEndPoint)listener.LocalEndpoint).Port; - listener.Stop(); - bound = true; - break; + return listener; } catch (SocketException) { - // Port is likely in use, try the next one + // Port became busy between the enumeration and Start(); try the next. } } - if (!bound) + // Range exhausted — fall back to an ephemeral port. This is a warning-level event: + // sustained fallbacks indicate the configured PASV range is too small or held by + // other processes, and eventually the OS-wide dynamic port range will be pressured too. + var fallback = new TcpListener(address, 0); + fallback.Start(); + var fallbackPort = ((IPEndPoint)fallback.LocalEndpoint).Port; + Console.WriteLine($"[PASV-V2] WARNING: configured range {minPort}-{maxPort} exhausted, fell back to ephemeral port {fallbackPort}"); + return fallback; + } + + /// + public async Task GetStreamAsync() + { + Console.WriteLine($"[PASV-V2] Awaiting client accept on port {Port}"); + _acceptedClient = await _listener.AcceptTcpClientAsync(_cts.Token).ConfigureAwait(false); + + var remoteEndpoint = (IPEndPoint?)_acceptedClient.Client.RemoteEndPoint; + var clientIp = remoteEndpoint?.Address.ToString() ?? ""; + Console.WriteLine($"[PASV-V2] Accepted client from {clientIp} (port {Port}, authorized={_authorizedClientIp})"); + + if (!AreIpAddressesEqual(clientIp, _authorizedClientIp)) { - // Fallback to random ephemeral port if the range is exhausted or invalid - var listener = new TcpListener(IPAddress.Any, 0); - listener.Start(); - Port = ((IPEndPoint)listener.LocalEndpoint).Port; - listener.Stop(); + Console.WriteLine($"[PASV-V2] REJECTED: {clientIp} != {_authorizedClientIp}"); + _acceptedClient.Close(); + _acceptedClient = null; + throw new UnauthorizedAccessException( + $"Data connection from {clientIp} rejected. Expected {_authorizedClientIp}."); } - _dataServer = new NetServer(); - _dataServer.OnClientConnected += c => - { - if (!AreIpAddressesEqual(c.Ip, authorizedClientIp)) - { - // Unblock GetStreamAsync with an exception to prevent hanging the control connection - _streamTcs.TrySetException( - new UnauthorizedAccessException( - $"Data connection from {c.Ip} rejected. Expected {authorizedClientIp}.")); - - // Stop the client asynchronously to prevent potential library crashes - Task.Run(() => c.Stop()); - return; - } - _stream = new MikuClientStream(c); - _streamTcs.TrySetResult(_stream); - }; - _dataServer.OnClientDataReceived += (c, data) => + _networkStream = _acceptedClient.GetStream(); + Console.WriteLine($"[PASV-V2] NetworkStream ready (port {Port})"); + return _networkStream; + } + + /// + public async ValueTask DisposeAsync() + { + if (_disposed) { - if (AreIpAddressesEqual(c.Ip, authorizedClientIp)) - { - _stream?.EnqueueData(data); - } - }; - _dataServer.OnClientDisconnected += (c, reason) => + return; + } + _disposed = true; + Console.WriteLine($"[PASV-V2] Disposing (port {Port})"); + + try { _cts.Cancel(); } catch { /* best effort */ } + + if (_networkStream is not null) { - _stream?.Complete(); - }; - _dataServer.Start(ip, Port); + try { await _networkStream.DisposeAsync().ConfigureAwait(false); } catch { /* best effort */ } + } + + try { _acceptedClient?.Close(); } catch { /* best effort */ } + try { _listener.Stop(); } catch { /* best effort */ } + + _cts.Dispose(); + Console.WriteLine($"[PASV-V2] Disposed (port {Port})"); } private static bool AreIpAddressesEqual(string ip1, string ip2) @@ -112,21 +161,10 @@ private static bool AreIpAddressesEqual(string ip1, string ip2) return true; } - // Normalize to IPv6 for comparison to handle IPv4-mapped IPv6 addresses + // Normalize IPv4-mapped IPv6 addresses. return addr1.MapToIPv6().Equals(addr2.MapToIPv6()); } return false; } - - /// - public Task GetStreamAsync() => _streamTcs.Task; - - /// - public ValueTask DisposeAsync() - { - _dataServer?.Stop(); - _dataServer = null; - return ValueTask.CompletedTask; - } }