From 3cc0c6376df8c7678bae0e2b9a97f785e8b224bc Mon Sep 17 00:00:00 2001 From: Nick Briscoe Date: Sun, 1 Mar 2026 01:28:37 +0000 Subject: [PATCH 1/4] Add Active FTP mode support (Issue #43) - Add FtpDataConnectionType enum (AutoPassive, Active) - Add PORT and EPRT to FtpCommand enum - Add DataConnectionType and ActiveExternalIp config properties - Refactor ConnectDataStreamAsync into Passive/Active branches - Active mode: binds TcpListener, sends PORT, returns lazy-accept ActiveDataStream - ActiveDataStream defers AcceptTcpClient until first Read to avoid deadlock - Add WrapDataStreamAsync and LocalIpAddress to FtpControlStream - Add 2 integration tests using in-process fake FTP server - All 23 tests pass, no breaking changes --- src/CoreFtp/Enum/FtpCommand.cs | 4 +- src/CoreFtp/Enum/FtpDataConnectionType.cs | 21 ++ src/CoreFtp/FtpClient.cs | 63 ++++- src/CoreFtp/FtpClientConfiguration.cs | 14 ++ .../Infrastructure/Stream/ActiveDataStream.cs | 116 ++++++++++ .../Infrastructure/Stream/FtpControlStream.cs | 35 +++ .../FtpClientTests/ActiveModeTests.cs | 219 ++++++++++++++++++ 7 files changed, 469 insertions(+), 3 deletions(-) create mode 100644 src/CoreFtp/Enum/FtpDataConnectionType.cs create mode 100644 src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs create mode 100644 tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs diff --git a/src/CoreFtp/Enum/FtpCommand.cs b/src/CoreFtp/Enum/FtpCommand.cs index b35e317..6486139 100644 --- a/src/CoreFtp/Enum/FtpCommand.cs +++ b/src/CoreFtp/Enum/FtpCommand.cs @@ -25,6 +25,8 @@ public enum FtpCommand TYPE, FEAT, PBSZ, - PROT + PROT, + PORT, + EPRT } } \ No newline at end of file diff --git a/src/CoreFtp/Enum/FtpDataConnectionType.cs b/src/CoreFtp/Enum/FtpDataConnectionType.cs new file mode 100644 index 0000000..a18fcf0 --- /dev/null +++ b/src/CoreFtp/Enum/FtpDataConnectionType.cs @@ -0,0 +1,21 @@ +namespace CoreFtp.Enum +{ + /// + /// Specifies the FTP data connection type used for file transfers and directory listings + /// + public enum FtpDataConnectionType + { + /// + /// Default. Use Extended Passive (EPSV) first, fallback to Passive (PASV). + /// Client connects to the server's data port. + /// + AutoPassive, + + /// + /// Use Active mode (PORT/EPRT). + /// Client listens on a local port and the server connects back to it. + /// Requires the client to be reachable from the server (not behind NAT without configuration). + /// + Active, + } +} diff --git a/src/CoreFtp/FtpClient.cs b/src/CoreFtp/FtpClient.cs index 0effb4b..cab46ab 100644 --- a/src/CoreFtp/FtpClient.cs +++ b/src/CoreFtp/FtpClient.cs @@ -630,12 +630,23 @@ private void EnsureLoggedIn() } /// - /// Produces a data socket using Passive (PASV) or Extended Passive (EPSV) mode + /// Produces a data socket using Passive (PASV/EPSV) or Active (PORT/EPRT) mode /// /// internal async Task ConnectDataStreamAsync() { - Logger?.LogTrace( "[FtpClient] Connecting to a data socket" ); + if ( Configuration.DataConnectionType == FtpDataConnectionType.Active ) + return await ConnectActiveDataStreamAsync(); + + return await ConnectPassiveDataStreamAsync(); + } + + /// + /// Produces a data socket using Passive (PASV) or Extended Passive (EPSV) mode + /// + private async Task ConnectPassiveDataStreamAsync() + { + Logger?.LogTrace( "[FtpClient] Connecting passive data socket" ); var epsvResult = await ControlStream.SendCommandAsync( FtpCommand.EPSV ); @@ -660,6 +671,54 @@ internal async Task ConnectDataStreamAsync() return await ControlStream.OpenDataStreamAsync( Configuration.Host, passivePortNumber.Value, CancellationToken.None ); } + /// + /// Produces a data socket using Active (PORT/EPRT) mode. + /// Binds a local TcpListener, sends PORT, and returns a lazy-accept stream that + /// accepts the server's inbound connection on the first read (after LIST/STOR is sent). + /// + private async Task ConnectActiveDataStreamAsync() + { + Logger?.LogTrace( "[FtpClient] Connecting active data socket" ); + + var localIp = Configuration.ActiveExternalIp ?? ControlStream.LocalIpAddress; + if ( string.IsNullOrEmpty( localIp ) ) + throw new FtpException( "Could not determine local IP address for Active mode. Set ActiveExternalIp in configuration." ); + + var listener = new System.Net.Sockets.TcpListener( System.Net.IPAddress.Any, 0 ); + listener.Start(); + + int localPort = ( (System.Net.IPEndPoint) listener.LocalEndpoint ).Port; + + // Send PORT command: PORT h1,h2,h3,h4,p1,p2 + string portCommand = FormatPortCommand( localIp, localPort ); + var portResult = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + { + FtpCommand = FtpCommand.PORT, + Data = portCommand + } ); + + if ( portResult.FtpStatusCode != FtpStatusCode.CommandOK ) + { + listener.Stop(); + throw new FtpException( "PORT command failed: " + portResult.ResponseMessage ); + } + + // Return a lazy-accept stream — the actual accept happens when the caller + // first reads from the stream (i.e. after LIST/STOR has been sent) + return new Infrastructure.Stream.ActiveDataStream( listener, ControlStream, Logger ); + } + + /// + /// Formats an IP and port into PORT command arguments: h1,h2,h3,h4,p1,p2 + /// + private static string FormatPortCommand( string ip, int port ) + { + string ipPart = ip.Replace( '.', ',' ); + int highByte = port / 256; + int lowByte = port % 256; + return $"{ipPart},{highByte},{lowByte}"; + } + /// /// Throws an exception if the server response is not one of the given acceptable codes /// diff --git a/src/CoreFtp/FtpClientConfiguration.cs b/src/CoreFtp/FtpClientConfiguration.cs index aaeb111..2aa6d78 100644 --- a/src/CoreFtp/FtpClientConfiguration.cs +++ b/src/CoreFtp/FtpClientConfiguration.cs @@ -47,5 +47,19 @@ public class FtpClientConfiguration /// and the automatic detection of the LIST output format fails (e.g. force FtpFileSystemType.Windows). /// public FtpFileSystemType? ForceFileSystem { get; set; } + + /// + /// Specifies the FTP data connection type. Default is + /// which uses EPSV/PASV (client connects to server's data port). + /// Set to to use PORT/EPRT (server connects back to client). + /// + public FtpDataConnectionType DataConnectionType { get; set; } = FtpDataConnectionType.AutoPassive; + + /// + /// The external IP address to advertise in Active mode PORT/EPRT commands. + /// Required when the client is behind NAT and the server cannot reach the client's local IP. + /// If null, the local IP address of the control connection socket is used. + /// + public string ActiveExternalIp { get; set; } } } diff --git a/src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs b/src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs new file mode 100644 index 0000000..ff62d87 --- /dev/null +++ b/src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs @@ -0,0 +1,116 @@ +namespace CoreFtp.Infrastructure.Stream +{ + using System; + using System.IO; + using System.Net.Sockets; + using System.Threading; + using System.Threading.Tasks; + using Microsoft.Extensions.Logging; + + /// + /// A wrapper stream for Active FTP mode that lazily accepts the server's + /// inbound data connection on the first read operation. This defers the + /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST) + /// has been sent on the control channel. + /// + internal class ActiveDataStream : Stream + { + private readonly TcpListener listener; + private readonly FtpControlStream controlStream; + private readonly ILogger logger; + private Stream innerStream; + private TcpClient acceptedClient; + private bool accepted; + private bool disposed; + + public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger ) + { + this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) ); + this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) ); + this.logger = logger; + } + + private async Task EnsureAcceptedAsync() + { + if ( accepted ) + return; + + logger?.LogDebug( "[ActiveDataStream] Accepting inbound data connection from server" ); + acceptedClient = await listener.AcceptTcpClientAsync(); + innerStream = await controlStream.WrapDataStreamAsync( acceptedClient ); + accepted = true; + logger?.LogDebug( "[ActiveDataStream] Data connection accepted" ); + } + + private void EnsureAccepted() + { + if ( accepted ) + return; + + EnsureAcceptedAsync().GetAwaiter().GetResult(); + } + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => true; + public override long Length => innerStream?.Length ?? 0; + + public override long Position + { + get => innerStream?.Position ?? 0; + set => throw new NotSupportedException(); + } + + public override int Read( byte[] buffer, int offset, int count ) + { + EnsureAccepted(); + return innerStream.Read( buffer, offset, count ); + } + + public override async Task ReadAsync( byte[] buffer, int offset, int count, CancellationToken cancellationToken ) + { + await EnsureAcceptedAsync(); + return await innerStream.ReadAsync( buffer, offset, count, cancellationToken ); + } + + public override void Write( byte[] buffer, int offset, int count ) + { + EnsureAccepted(); + innerStream.Write( buffer, offset, count ); + } + + public override async Task WriteAsync( byte[] buffer, int offset, int count, CancellationToken cancellationToken ) + { + await EnsureAcceptedAsync(); + await innerStream.WriteAsync( buffer, offset, count, cancellationToken ); + } + + public override void Flush() + { + innerStream?.Flush(); + } + + public override long Seek( long offset, SeekOrigin origin ) + { + throw new NotSupportedException(); + } + + public override void SetLength( long value ) + { + throw new NotSupportedException(); + } + + protected override void Dispose( bool disposing ) + { + if ( !disposed && disposing ) + { + innerStream?.Dispose(); + acceptedClient?.Dispose(); + listener?.Stop(); + disposed = true; + } + + base.Dispose( disposing ); + } + } +} diff --git a/src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs b/src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs index 79a5a0b..85d527f 100644 --- a/src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs +++ b/src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs @@ -36,6 +36,20 @@ public class FtpControlStream : Stream internal bool IsDataConnection { get; set; } + /// + /// Returns the local IP address of the control connection socket. + /// Used by Active mode to determine the IP to advertise in PORT commands. + /// + internal string LocalIpAddress + { + get + { + if ( Socket?.LocalEndPoint is System.Net.IPEndPoint localEndPoint ) + return localEndPoint.Address.ToString(); + return null; + } + } + internal void SetTimeouts(int milliseconds) { BaseStream.ReadTimeout = milliseconds; @@ -345,6 +359,27 @@ public async Task OpenDataStreamAsync(string host, int port, Cancellatio return socketStream; } + /// + /// Wraps an accepted TcpClient (from Active mode) into a data connection stream. + /// Applies TLS if the control connection is encrypted. + /// + public async Task WrapDataStreamAsync( System.Net.Sockets.TcpClient acceptedClient ) + { + Logger?.LogDebug( "[FtpSocketStream] Wrapping accepted data connection" ); + var socketStream = new FtpControlStream( Configuration, dnsResolver ) + { Logger = Logger, IsDataConnection = true }; + + socketStream.Socket = acceptedClient.Client; + socketStream.BaseStream = new NetworkStream( acceptedClient.Client ); + + if ( IsEncrypted ) + { + await socketStream.ActivateEncryptionAsync(); + } + + return socketStream; + } + protected async Task ConnectStreamAsync(CancellationToken token) { await ConnectStreamAsync(Configuration.Host, Configuration.Port, token); diff --git a/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs b/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs new file mode 100644 index 0000000..7502e4e --- /dev/null +++ b/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs @@ -0,0 +1,219 @@ +namespace CoreFtp.Tests.Integration.FtpClientTests +{ + using System; + using System.IO; + using System.Linq; + using System.Net; + using System.Net.Sockets; + using System.Text; + using System.Threading.Tasks; + using Enum; + using FluentAssertions; + using Microsoft.Extensions.Logging; + using Shared; + using Xunit; + using Xunit.Abstractions; + + public class ActiveModeTests : IDisposable + { + private readonly ILogger logger; + private readonly TcpListener controlListener; + private readonly int controlPort; + + public ActiveModeTests( ITestOutputHelper outputHelper ) + { + var factory = LoggerFactory.Create( builder => + builder.AddProvider( new XunitLoggerProvider( outputHelper ) ) ); + logger = factory.CreateLogger(); + + controlListener = new TcpListener( IPAddress.Loopback, 0 ); + controlListener.Start(); + controlPort = ( (IPEndPoint) controlListener.LocalEndpoint ).Port; + } + + public void Dispose() + { + controlListener.Stop(); + } + + private async Task RunFakeFtpServer( Func handler ) + { + using ( var client = await controlListener.AcceptTcpClientAsync() ) + using ( var stream = client.GetStream() ) + using ( var reader = new StreamReader( stream, Encoding.ASCII ) ) + using ( var writer = new StreamWriter( stream, Encoding.ASCII ) { AutoFlush = true } ) + { + await handler( reader, writer ); + } + } + + private static (string ip, int port) ParsePortCommand( string portArgs ) + { + var parts = portArgs.Split( ',' ); + string ip = $"{parts[ 0 ]}.{parts[ 1 ]}.{parts[ 2 ]}.{parts[ 3 ]}"; + int port = int.Parse( parts[ 4 ] ) * 256 + int.Parse( parts[ 5 ] ); + return (ip, port); + } + + [Fact] + public async Task ListAllAsync_ActiveMode_ReturnsDirectoryListing() + { + var serverTask = RunFakeFtpServer( async ( reader, writer ) => + { + // Greeting + await writer.WriteLineAsync( "220 Welcome" ); + + // USER + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "331 Password required" ); + + // PASS + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "230 Logged in" ); + + // FEAT + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "211-Features:" ); + await writer.WriteLineAsync( " UTF8" ); + await writer.WriteLineAsync( "211 End" ); + + // OPTS UTF8 ON + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "200 OK" ); + + // TYPE I + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "200 OK" ); + + // CWD / + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "250 CWD OK" ); + // PWD + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "257 \"/\"" ); + + // PORT command + var portLine = await reader.ReadLineAsync(); + var portArgs = portLine.Substring( "PORT ".Length ); + var (ip, port) = ParsePortCommand( portArgs ); + await writer.WriteLineAsync( "200 PORT command successful" ); + + // LIST + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "150 Opening data connection" ); + + // Server connects back to client's data listener + using ( var dataClient = new TcpClient() ) + { + await dataClient.ConnectAsync( ip, port ); + using ( var dataStream = dataClient.GetStream() ) + using ( var dataWriter = new StreamWriter( dataStream, Encoding.ASCII ) { AutoFlush = true } ) + { + await dataWriter.WriteLineAsync( "-rw-r--r-- 1 user group 1024 Jan 01 00:00 file1.txt" ); + await dataWriter.WriteLineAsync( "drwxr-xr-x 2 user group 4096 Jan 01 00:00 subdir" ); + await dataWriter.WriteLineAsync( "-rw-r--r-- 1 user group 2048 Jan 01 00:00 file2.log" ); + } + } + + await writer.WriteLineAsync( "226 Transfer complete" ); + } ); + + var config = new FtpClientConfiguration + { + Host = "localhost", + Port = controlPort, + Username = "test", + Password = "pwd", + DataConnectionType = FtpDataConnectionType.Active, + ActiveExternalIp = "127.0.0.1" + }; + + using ( var client = new FtpClient( config ) { Logger = logger } ) + { + await client.LoginAsync(); + + var nodes = await client.ListAllAsync(); + + nodes.Should().HaveCount( 3 ); + nodes.Count( n => n.NodeType == FtpNodeType.File ).Should().Be( 2 ); + nodes.Count( n => n.NodeType == FtpNodeType.Directory ).Should().Be( 1 ); + nodes.Any( n => n.Name == "file1.txt" ).Should().BeTrue(); + nodes.Any( n => n.Name == "subdir" ).Should().BeTrue(); + nodes.Any( n => n.Name == "file2.log" ).Should().BeTrue(); + } + + await serverTask; + } + + [Fact] + public async Task ListFilesAsync_ActiveMode_ReturnsOnlyFiles() + { + var serverTask = RunFakeFtpServer( async ( reader, writer ) => + { + await writer.WriteLineAsync( "220 Welcome" ); + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "331 Password required" ); + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "230 Logged in" ); + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "211-Features:" ); + await writer.WriteLineAsync( " UTF8" ); + await writer.WriteLineAsync( "211 End" ); + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "200 OK" ); + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "200 OK" ); + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "250 CWD OK" ); + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "257 \"/\"" ); + + // PORT command + var portLine = await reader.ReadLineAsync(); + var portArgs = portLine.Substring( "PORT ".Length ); + var (ip, port) = ParsePortCommand( portArgs ); + await writer.WriteLineAsync( "200 PORT command successful" ); + + // LIST + await reader.ReadLineAsync(); + await writer.WriteLineAsync( "150 Opening data connection" ); + + using ( var dataClient = new TcpClient() ) + { + await dataClient.ConnectAsync( ip, port ); + using ( var dataStream = dataClient.GetStream() ) + using ( var dataWriter = new StreamWriter( dataStream, Encoding.ASCII ) { AutoFlush = true } ) + { + await dataWriter.WriteLineAsync( "-rw-r--r-- 1 user group 1024 Jan 01 00:00 readme.md" ); + await dataWriter.WriteLineAsync( "drwxr-xr-x 2 user group 4096 Jan 01 00:00 docs" ); + } + } + + await writer.WriteLineAsync( "226 Transfer complete" ); + } ); + + var config = new FtpClientConfiguration + { + Host = "localhost", + Port = controlPort, + Username = "test", + Password = "pwd", + DataConnectionType = FtpDataConnectionType.Active, + ActiveExternalIp = "127.0.0.1" + }; + + using ( var client = new FtpClient( config ) { Logger = logger } ) + { + await client.LoginAsync(); + + var files = await client.ListFilesAsync(); + + files.Should().HaveCount( 1 ); + files[ 0 ].Name.Should().Be( "readme.md" ); + files[ 0 ].NodeType.Should().Be( FtpNodeType.File ); + } + + await serverTask; + } + } +} From a2de5bfc2fff74450aac82a4ed62d0a8f18f541c Mon Sep 17 00:00:00 2001 From: Nick Briscoe Date: Sun, 1 Mar 2026 14:23:02 +0000 Subject: [PATCH 2/4] Address PR feedback for Active FTP mode - Validate active data peer IP matches control connection IP - Fix sync-over-async in ActiveDataStream by properly overriding Memory/Span APIs and disallowing sync I/O - Add cancellation/timeout support to ActiveDataStream accept logic - Bind active mode listener explicitly to resolved IPv4 address instead of IPAddress.Any - WrapDataStreamAsync applies LingerState and ReceiveTimeout correctly - Refactored RetrieveDirectoryListing to be async instead of blocking byte-by-byte reads - Ensure WrapDataStreamAsync API is internal - Update test field naming conventions --- .../spark_NICK-PC_2026-03-01_14_20_01.trx | 72 +++ issues_comments.json | 1 + pulls_comments.json | 1 + .../DirectoryListing/DirectoryProviderBase.cs | 35 +- .../DirectoryListing/ListDirectoryProvider.cs | 51 +- .../DirectoryListing/MlsdDirectoryProvider.cs | 36 +- src/CoreFtp/FtpClient.cs | 442 +++++++++--------- .../Infrastructure/Stream/ActiveDataStream.cs | 111 +++-- .../Infrastructure/Stream/FtpControlStream.cs | 21 +- .../FtpClientTests/ActiveModeTests.cs | 162 +++---- 10 files changed, 531 insertions(+), 401 deletions(-) create mode 100644 TestResults/spark_NICK-PC_2026-03-01_14_20_01.trx create mode 100644 issues_comments.json create mode 100644 pulls_comments.json diff --git a/TestResults/spark_NICK-PC_2026-03-01_14_20_01.trx b/TestResults/spark_NICK-PC_2026-03-01_14_20_01.trx new file mode 100644 index 0000000..adf0ea1 --- /dev/null +++ b/TestResults/spark_NICK-PC_2026-03-01_14_20_01.trx @@ -0,0 +1,72 @@ + + + + + + + + + + Error sending QUIT command during logout + + System.NotSupportedException : Synchronous operations are not supported on this stream. Use ReadAsync instead. + at CoreFtp.Infrastructure.Stream.ActiveDataStream.Read(Byte[] buffer, Int32 offset, Int32 count) in D:\code\oss\CoreFTP\src\CoreFtp\Infrastructure\Stream\ActiveDataStream.cs:line 82 + at CoreFtp.Components.DirectoryListing.DirectoryProviderBase.ReadLine(Encoding encoding) in D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\DirectoryProviderBase.cs:line 38 + at CoreFtp.Components.DirectoryListing.DirectoryProviderBase.RetrieveDirectoryListing()+MoveNext() in D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\DirectoryProviderBase.cs:line 22 + at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection) + at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source) + at CoreFtp.Components.DirectoryListing.ListDirectoryProvider.ListNodesAsync(Nullable`1 ftpNodeType) in D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\ListDirectoryProvider.cs:line 108 + at CoreFtp.Components.DirectoryListing.ListDirectoryProvider.ListAllAsync() in D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\ListDirectoryProvider.cs:line 50 + at CoreFtp.FtpClient.ListAllAsync() in D:\code\oss\CoreFTP\src\CoreFtp\FtpClient.cs:line 386 + at CoreFtp.FtpClient.ListAllAsync() in D:\code\oss\CoreFTP\src\CoreFtp\FtpClient.cs:line 391 + at CoreFtp.Tests.Integration.FtpClientTests.ActiveModeTests.ListAllAsync_ActiveMode_ReturnsDirectoryListing() in D:\code\oss\CoreFTP\tests\CoreFtp.Tests.Integration\FtpClientTests\ActiveModeTests.cs:line 135 +--- End of stack trace from previous location --- + + + + + + + + + + + + + + + + + + + + + [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v3.0.0+e341b939fe (64-bit .NET 10.0.3) +[xUnit.net 00:00:00.04] Discovering: CoreFtp.Tests.Integration +[xUnit.net 00:00:00.06] Discovered: CoreFtp.Tests.Integration +[xUnit.net 00:00:00.07] Starting: CoreFtp.Tests.Integration +[xUnit.net 00:00:00.16] System.NotSupportedException : Synchronous operations are not supported on this stream. Use ReadAsync instead. +[xUnit.net 00:00:00.16] Stack Trace: +[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Infrastructure\Stream\ActiveDataStream.cs(82,0): at CoreFtp.Infrastructure.Stream.ActiveDataStream.Read(Byte[] buffer, Int32 offset, Int32 count) +[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\DirectoryProviderBase.cs(38,0): at CoreFtp.Components.DirectoryListing.DirectoryProviderBase.ReadLine(Encoding encoding) +[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\DirectoryProviderBase.cs(22,0): at CoreFtp.Components.DirectoryListing.DirectoryProviderBase.RetrieveDirectoryListing()+MoveNext() +[xUnit.net 00:00:00.16] at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection) +[xUnit.net 00:00:00.16] at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source) +[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\ListDirectoryProvider.cs(108,0): at CoreFtp.Components.DirectoryListing.ListDirectoryProvider.ListNodesAsync(Nullable`1 ftpNodeType) +[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\ListDirectoryProvider.cs(50,0): at CoreFtp.Components.DirectoryListing.ListDirectoryProvider.ListAllAsync() +[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\FtpClient.cs(386,0): at CoreFtp.FtpClient.ListAllAsync() +[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\FtpClient.cs(391,0): at CoreFtp.FtpClient.ListAllAsync() +[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\tests\CoreFtp.Tests.Integration\FtpClientTests\ActiveModeTests.cs(135,0): at CoreFtp.Tests.Integration.FtpClientTests.ActiveModeTests.ListAllAsync_ActiveMode_ReturnsDirectoryListing() +[xUnit.net 00:00:00.16] --- End of stack trace from previous location --- +[xUnit.net 00:00:00.16] Output: +[xUnit.net 00:00:00.16] Error sending QUIT command during logout +[xUnit.net 00:00:00.16] Finished: CoreFtp.Tests.Integration + + + + + [xUnit.net 00:00:00.16] CoreFtp.Tests.Integration.FtpClientTests.ActiveModeTests.ListAllAsync_ActiveMode_ReturnsDirectoryListing [FAIL] + + + + \ No newline at end of file diff --git a/issues_comments.json b/issues_comments.json new file mode 100644 index 0000000..ea71526 --- /dev/null +++ b/issues_comments.json @@ -0,0 +1 @@ +[{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/issues/comments/3978804597","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#issuecomment-3978804597","issue_url":"https://api.github.com/repos/sparkeh9/CoreFTP/issues/55","id":3978804597,"node_id":"IC_kwDOBBPGOs7tJ711","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"created_at":"2026-03-01T01:29:28Z","updated_at":"2026-03-01T01:29:28Z","body":"## Summary of Changes\n\nHello, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!\n\nThis pull request significantly enhances the `CoreFtp` library by integrating full support for Active FTP mode. This addresses a critical limitation, allowing the client to connect to FTP servers that exclusively operate in active mode, thereby broadening the library's compatibility. The implementation includes new configuration options and a clever lazy-accept mechanism to manage the asynchronous nature of active data connections, all while maintaining existing passive mode functionality and ensuring backward compatibility.\n\n### Highlights\n\n* **Active FTP Mode Support**: Added comprehensive support for Active FTP mode (PORT/EPRT), enabling the library to connect to servers that do not support passive mode.\n* **New Configuration Options**: Introduced a new `FtpDataConnectionType` enum and `ActiveExternalIp` property in `FtpClientConfiguration` to allow explicit selection and configuration of active mode.\n* **Lazy Data Stream Acceptance**: Implemented `ActiveDataStream`, a wrapper stream that defers the acceptance of the server's inbound data connection until the first read operation, effectively solving the PORT/LIST sequencing deadlock.\n* **Refactored Data Connection Logic**: Refactored `FtpClient` to separate and manage passive and active data stream connection logic, including a new helper for formatting PORT commands.\n* **Control Stream Enhancements**: Enhanced `FtpControlStream` with a `LocalIpAddress` property and a `WrapDataStreamAsync` method to facilitate the wrapping of accepted data streams, including TLS support.\n* **Integration Tests**: Included new integration tests (`ActiveModeTests.cs`) to thoroughly verify the functionality and correctness of the Active FTP mode implementation.\n* **Backward Compatibility**: Ensured full backward compatibility, with `AutoPassive` remaining the default data connection type and no breaking changes introduced to existing functionality.\n\n
\nChangelog\n\n* **src/CoreFtp/Enum/FtpCommand.cs**\n * Added `PORT` and `EPRT` commands to the `FtpCommand` enumeration.\n* **src/CoreFtp/Enum/FtpDataConnectionType.cs**\n * Added a new enumeration `FtpDataConnectionType` with `AutoPassive` and `Active` options.\n* **src/CoreFtp/FtpClient.cs**\n * Refactored `ConnectDataStreamAsync` to delegate to `ConnectActiveDataStreamAsync` or `ConnectPassiveDataStreamAsync` based on configuration.\n * Implemented `ConnectActiveDataStreamAsync` to handle the setup for active mode data connections, including `TcpListener` and `PORT` command sending.\n * Added a private static helper method `FormatPortCommand` to construct the `PORT` command argument string.\n* **src/CoreFtp/FtpClientConfiguration.cs**\n * Added `DataConnectionType` property to specify the desired FTP data connection mode.\n * Added `ActiveExternalIp` property to allow explicit setting of the external IP for active mode, crucial for NAT scenarios.\n* **src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs**\n * Added a new `ActiveDataStream` class, which is a `Stream` wrapper that lazily accepts an inbound `TcpClient` connection on the first read/write operation.\n* **src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs**\n * Added a `LocalIpAddress` property to retrieve the local IP of the control connection.\n * Added a `WrapDataStreamAsync` method to encapsulate an accepted `TcpClient` into a data stream, applying TLS if the control connection is encrypted.\n* **tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs**\n * Added a new test class `ActiveModeTests` containing integration tests for `ListAllAsync` and `ListFilesAsync` in active mode, using a fake FTP server.\n
\n\n
\nActivity\n\n* No human activity (comments, reviews, etc.) has been recorded for this pull request yet.\n
\n\n
\nUsing Gemini Code Assist\n
\n\nThe full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips.\n\nInvoking Gemini\n\nYou can request assistance from Gemini at any point by creating a comment using either `/gemini ` or `@gemini-code-assist `. Below is a summary of the supported commands on the current page.\n\nFeature | Command | Description\n--- | --- | ---\nCode Review | `/gemini review` | Performs a code review for the current pull request in its current state.\nPull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state.\nComment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments.\nHelp | `/gemini help` | Displays a list of available commands.\n\nCustomization\n\nTo customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github).\n\nLimitations & Feedback\n\nGemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG).\n\nYou can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the [Gemini Code Assist IDE Extension](https://cloud.google.com/products/gemini/code-assist).\n
\n\n\n[^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).\n","author_association":"NONE","pin":null,"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/issues/comments/3978804597/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"performed_via_github_app":{"id":956858,"client_id":"Iv23ctcJt9oWoM6OPLsw","slug":"gemini-code-assist","node_id":"A_kwHOABR6NM4ADpm6","owner":{"login":"google","id":1342004,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEzNDIwMDQ=","avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=4","gravatar_id":"","url":"https://api.github.com/users/google","html_url":"https://github.com/google","followers_url":"https://api.github.com/users/google/followers","following_url":"https://api.github.com/users/google/following{/other_user}","gists_url":"https://api.github.com/users/google/gists{/gist_id}","starred_url":"https://api.github.com/users/google/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/google/subscriptions","organizations_url":"https://api.github.com/users/google/orgs","repos_url":"https://api.github.com/users/google/repos","events_url":"https://api.github.com/users/google/events{/privacy}","received_events_url":"https://api.github.com/users/google/received_events","type":"Organization","user_view_type":"public","site_admin":false},"name":"Gemini Code Assist","description":"**Accelerate code reviews and improve code quality**\r\n\r\nBring the power of [Gemini](https://gemini.google.com/) directly to GitHub, providing AI-powered code reviews on your pull requests, with automatic pull request summaries, ready-to-commit code suggestions and the ability to invoke Gemini for assistance at any point on the PR.\r\n","external_url":"https://developers.google.com/gemini-code-assist/docs/review-github-code","html_url":"https://github.com/apps/gemini-code-assist","created_at":"2024-07-29T20:55:46Z","updated_at":"2025-09-23T20:29:15Z","permissions":{"contents":"read","issues":"write","members":"read","metadata":"read","pull_requests":"write"},"events":["commit_comment","issue_comment","pull_request","pull_request_review","pull_request_review_comment","pull_request_review_thread","push"]}}] diff --git a/pulls_comments.json b/pulls_comments.json new file mode 100644 index 0000000..024bdec --- /dev/null +++ b/pulls_comments.json @@ -0,0 +1 @@ +[{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151692","pull_request_review_id":3871046696,"id":2868151692,"node_id":"PRRC_kwDOBBPGOs6q9IWM","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![security-high](https://www.gstatic.com/codereviewagent/security-high-priority.svg) ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)\n\nIn Active FTP mode, the client opens a local port and waits for the server to connect. The current implementation of `ActiveDataStream.EnsureAcceptedAsync` accepts the first TCP connection that arrives on the listening port without verifying the source IP address. An attacker who can connect to the client's listening port (e.g., by scanning or if the client is on a shared network) can intercept the data stream before the legitimate FTP server connects. This allows the attacker to either steal sensitive data being uploaded or inject malicious data into a download (e.g., a fake directory listing or a malicious file).","created_at":"2026-03-01T01:31:26Z","updated_at":"2026-03-01T01:31:27Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151692","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151692"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151692"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151692/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":null,"original_start_line":null,"start_side":null,"line":39,"original_line":39,"side":"RIGHT","author_association":"NONE","original_position":39,"position":39,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151694","pull_request_review_id":3871046696,"id":2868151694,"node_id":"PRRC_kwDOBBPGOs6q9IWO","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;\n+ logger?.LogDebug( \"[ActiveDataStream] Data connection accepted\" );\n+ }\n+\n+ private void EnsureAccepted()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ EnsureAcceptedAsync().GetAwaiter().GetResult();\n+ }\n+\n+ public override bool CanRead => true;\n+ public override bool CanSeek => false;\n+ public override bool CanWrite => true;\n+ public override long Length => innerStream?.Length ?? 0;\n+\n+ public override long Position\n+ {\n+ get => innerStream?.Position ?? 0;\n+ set => throw new NotSupportedException();\n+ }\n+\n+ public override int Read( byte[] buffer, int offset, int count )\n+ {\n+ EnsureAccepted();\n+ return innerStream.Read( buffer, offset, count );\n+ }","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![high](https://www.gstatic.com/codereviewagent/high-priority.svg)\n\nThis method uses `EnsureAccepted()`, which in turn calls `EnsureAcceptedAsync().GetAwaiter().GetResult()`. This sync-over-async pattern is dangerous and can lead to deadlocks in certain environments (like UI or ASP.NET applications). To make the stream safer to use, it's better to prevent synchronous operations if a fully synchronous implementation isn't feasible. I recommend throwing `NotSupportedException` to enforce async-only usage.\n\n```c#\n public override int Read( byte[] buffer, int offset, int count )\n {\n throw new NotSupportedException(\"Synchronous operations are not supported on this stream. Use ReadAsync instead.\");\n }\n```","created_at":"2026-03-01T01:31:26Z","updated_at":"2026-03-01T01:31:27Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151694","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151694"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151694"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151694/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":64,"original_start_line":64,"start_side":"RIGHT","line":68,"original_line":68,"side":"RIGHT","author_association":"NONE","original_position":68,"position":68,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151695","pull_request_review_id":3871046696,"id":2868151695,"node_id":"PRRC_kwDOBBPGOs6q9IWP","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;\n+ logger?.LogDebug( \"[ActiveDataStream] Data connection accepted\" );\n+ }\n+\n+ private void EnsureAccepted()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ EnsureAcceptedAsync().GetAwaiter().GetResult();\n+ }\n+\n+ public override bool CanRead => true;\n+ public override bool CanSeek => false;\n+ public override bool CanWrite => true;\n+ public override long Length => innerStream?.Length ?? 0;\n+\n+ public override long Position\n+ {\n+ get => innerStream?.Position ?? 0;\n+ set => throw new NotSupportedException();\n+ }\n+\n+ public override int Read( byte[] buffer, int offset, int count )\n+ {\n+ EnsureAccepted();\n+ return innerStream.Read( buffer, offset, count );\n+ }\n+\n+ public override async Task ReadAsync( byte[] buffer, int offset, int count, CancellationToken cancellationToken )\n+ {\n+ await EnsureAcceptedAsync();\n+ return await innerStream.ReadAsync( buffer, offset, count, cancellationToken );\n+ }\n+\n+ public override void Write( byte[] buffer, int offset, int count )\n+ {\n+ EnsureAccepted();\n+ innerStream.Write( buffer, offset, count );\n+ }","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![high](https://www.gstatic.com/codereviewagent/high-priority.svg)\n\nSimilar to the `Read` method, this synchronous `Write` method relies on a sync-over-async call via `EnsureAccepted()`, which can cause deadlocks. To ensure the safety and reliability of the library, this synchronous method should be disallowed. Throwing `NotSupportedException` will guide users to the correct asynchronous API.\n\n```c#\n public override void Write( byte[] buffer, int offset, int count )\n {\n throw new NotSupportedException(\"Synchronous operations are not supported on this stream. Use WriteAsync instead.\");\n }\n```","created_at":"2026-03-01T01:31:27Z","updated_at":"2026-03-01T01:31:27Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151695","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151695"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151695"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151695/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":76,"original_start_line":76,"start_side":"RIGHT","line":80,"original_line":80,"side":"RIGHT","author_association":"NONE","original_position":80,"position":80,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151697","pull_request_review_id":3871046696,"id":2868151697,"node_id":"PRRC_kwDOBBPGOs6q9IWR","diff_hunk":"@@ -660,6 +671,54 @@ internal async Task ConnectDataStreamAsync()\n return await ControlStream.OpenDataStreamAsync( Configuration.Host, passivePortNumber.Value, CancellationToken.None );\n }\n \n+ /// \n+ /// Produces a data socket using Active (PORT/EPRT) mode.\n+ /// Binds a local TcpListener, sends PORT, and returns a lazy-accept stream that\n+ /// accepts the server's inbound connection on the first read (after LIST/STOR is sent).\n+ /// \n+ private async Task ConnectActiveDataStreamAsync()\n+ {\n+ Logger?.LogTrace( \"[FtpClient] Connecting active data socket\" );\n+\n+ var localIp = Configuration.ActiveExternalIp ?? ControlStream.LocalIpAddress;\n+ if ( string.IsNullOrEmpty( localIp ) )\n+ throw new FtpException( \"Could not determine local IP address for Active mode. Set ActiveExternalIp in configuration.\" );\n+\n+ var listener = new System.Net.Sockets.TcpListener( System.Net.IPAddress.Any, 0 );","path":"src/CoreFtp/FtpClient.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![security-medium](https://www.gstatic.com/codereviewagent/security-medium-priority.svg) ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)\n\nThe `TcpListener` is currently bound to `IPAddress.Any`, exposing the listening port on all network interfaces and increasing the attack surface. It is recommended to bind it specifically to the local IP address used for the control connection to mitigate this security risk. Furthermore, while addressing this, consider enhancing the active mode implementation within this method to support `EPRT` for IPv6 compatibility, as the current `PORT` command is limited to IPv4.","created_at":"2026-03-01T01:31:27Z","updated_at":"2026-03-01T01:31:27Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151697","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151697"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151697"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151697/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":null,"original_start_line":null,"start_side":null,"line":687,"original_line":687,"side":"RIGHT","author_association":"NONE","original_position":43,"position":43,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154896","pull_request_review_id":3871050263,"id":2868154896,"node_id":"PRRC_kwDOBBPGOs6q9JIQ","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"chatgpt-codex-connector[bot]","id":199175422,"node_id":"BOT_kgDOC98s_g","avatar_url":"https://avatars.githubusercontent.com/in/1144995?v=4","gravatar_id":"","url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D","html_url":"https://github.com/apps/chatgpt-codex-connector","followers_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/followers","following_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/repos","events_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"**![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat) Bound active accept wait to transfer timeout**\n\n`EnsureAcceptedAsync` waits on `AcceptTcpClientAsync()` with no cancellation or timeout, so when Active mode is misconfigured (e.g., wrong `ActiveExternalIp`, NAT/firewall blocks callback, or server never opens data connection) any LIST/RETR/STOR read/write can hang indefinitely and never surface the FTP error response from the control channel. This effectively ignores `TimeoutSeconds` for the most failure-prone part of Active mode and can stall callers permanently.\n\nUseful? React with ­ƒæì┬á/ ­ƒæÄ.","created_at":"2026-03-01T01:32:37Z","updated_at":"2026-03-01T01:32:37Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868154896","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154896"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868154896"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154896/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":null,"original_start_line":null,"start_side":null,"line":39,"original_line":39,"side":"RIGHT","author_association":"NONE","original_position":39,"position":39,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154898","pull_request_review_id":3871050263,"id":2868154898,"node_id":"PRRC_kwDOBBPGOs6q9JIS","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"chatgpt-codex-connector[bot]","id":199175422,"node_id":"BOT_kgDOC98s_g","avatar_url":"https://avatars.githubusercontent.com/in/1144995?v=4","gravatar_id":"","url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D","html_url":"https://github.com/apps/chatgpt-codex-connector","followers_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/followers","following_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/repos","events_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Validate active data peer before wrapping socket**\n\nThe first inbound connection is accepted and used as the transfer stream without checking that it comes from the same server as the control connection. In environments where another host can reach the client listener (especially plain FTP where `PORT` is visible on-path), a race connection can be accepted and treated as trusted data, allowing transfer corruption or data exfiltration.\n\nUseful? React with ­ƒæì┬á/ ­ƒæÄ.","created_at":"2026-03-01T01:32:37Z","updated_at":"2026-03-01T01:32:37Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868154898","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154898"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868154898"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154898/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":39,"original_start_line":39,"start_side":"RIGHT","line":40,"original_line":40,"side":"RIGHT","author_association":"NONE","original_position":40,"position":40,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157022","pull_request_review_id":3871052843,"id":2868157022,"node_id":"PRRC_kwDOBBPGOs6q9Jpe","diff_hunk":"@@ -345,6 +359,27 @@ public async Task OpenDataStreamAsync(string host, int port, Cancellatio\n return socketStream;\n }\n \n+ /// \n+ /// Wraps an accepted TcpClient (from Active mode) into a data connection stream.\n+ /// Applies TLS if the control connection is encrypted.\n+ /// \n+ public async Task WrapDataStreamAsync( System.Net.Sockets.TcpClient acceptedClient )\n+ {\n+ Logger?.LogDebug( \"[FtpSocketStream] Wrapping accepted data connection\" );\n+ var socketStream = new FtpControlStream( Configuration, dnsResolver )\n+ { Logger = Logger, IsDataConnection = true };\n+\n+ socketStream.Socket = acceptedClient.Client;\n+ socketStream.BaseStream = new NetworkStream( acceptedClient.Client );\n+\n+ if ( IsEncrypted )\n+ {\n+ await socketStream.ActivateEncryptionAsync();\n+ }\n+\n+ return socketStream;","path":"src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"WrapDataStreamAsync() builds a data-connection stream from an accepted socket but doesnÔÇÖt apply the same socket configuration as OpenDataStreamAsync()/ConnectSocketAsync (e.g., ReceiveTimeout / linger / any other options), which can change timeout behavior for Active mode transfers and potentially hang indefinitely. Consider mirroring the data-socket setup performed for passive connections before returning the wrapped stream.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157022","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157022"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157022"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157022/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":366,"original_start_line":366,"start_side":"RIGHT","line":380,"original_line":380,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":43,"position":43,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157030","pull_request_review_id":3871052843,"id":2868157030,"node_id":"PRRC_kwDOBBPGOs6q9Jpm","diff_hunk":"@@ -660,6 +671,54 @@ internal async Task ConnectDataStreamAsync()\n return await ControlStream.OpenDataStreamAsync( Configuration.Host, passivePortNumber.Value, CancellationToken.None );\n }\n \n+ /// \n+ /// Produces a data socket using Active (PORT/EPRT) mode.\n+ /// Binds a local TcpListener, sends PORT, and returns a lazy-accept stream that\n+ /// accepts the server's inbound connection on the first read (after LIST/STOR is sent).\n+ /// \n+ private async Task ConnectActiveDataStreamAsync()\n+ {\n+ Logger?.LogTrace( \"[FtpClient] Connecting active data socket\" );\n+\n+ var localIp = Configuration.ActiveExternalIp ?? ControlStream.LocalIpAddress;\n+ if ( string.IsNullOrEmpty( localIp ) )\n+ throw new FtpException( \"Could not determine local IP address for Active mode. Set ActiveExternalIp in configuration.\" );\n+\n+ var listener = new System.Net.Sockets.TcpListener( System.Net.IPAddress.Any, 0 );\n+ listener.Start();\n+\n+ int localPort = ( (System.Net.IPEndPoint) listener.LocalEndpoint ).Port;\n+\n+ // Send PORT command: PORT h1,h2,h3,h4,p1,p2\n+ string portCommand = FormatPortCommand( localIp, localPort );\n+ var portResult = await ControlStream.SendCommandAsync( new FtpCommandEnvelope","path":"src/CoreFtp/FtpClient.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"ConnectActiveDataStreamAsync always uses PORT and FormatPortCommand assumes an IPv4 dotted-quad address. If ActiveExternalIp/LocalIpAddress is IPv6 (contains ':'), the generated command will be invalid and the listener is also IPv4-only (IPAddress.Any). Consider either implementing EPRT for IPv6 (and/or preferring EPRT with fallback), or explicitly validating and throwing a clear exception when a non-IPv4 address is supplied.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157030","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157030"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157030"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157030/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":683,"original_start_line":683,"start_side":"RIGHT","line":694,"original_line":694,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":50,"position":50,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157033","pull_request_review_id":3871052843,"id":2868157033,"node_id":"PRRC_kwDOBBPGOs6q9Jpp","diff_hunk":"@@ -660,6 +671,54 @@ internal async Task ConnectDataStreamAsync()\n return await ControlStream.OpenDataStreamAsync( Configuration.Host, passivePortNumber.Value, CancellationToken.None );\n }\n \n+ /// \n+ /// Produces a data socket using Active (PORT/EPRT) mode.\n+ /// Binds a local TcpListener, sends PORT, and returns a lazy-accept stream that\n+ /// accepts the server's inbound connection on the first read (after LIST/STOR is sent).\n+ /// \n+ private async Task ConnectActiveDataStreamAsync()\n+ {\n+ Logger?.LogTrace( \"[FtpClient] Connecting active data socket\" );\n+\n+ var localIp = Configuration.ActiveExternalIp ?? ControlStream.LocalIpAddress;\n+ if ( string.IsNullOrEmpty( localIp ) )\n+ throw new FtpException( \"Could not determine local IP address for Active mode. Set ActiveExternalIp in configuration.\" );\n+\n+ var listener = new System.Net.Sockets.TcpListener( System.Net.IPAddress.Any, 0 );\n+ listener.Start();\n+\n+ int localPort = ( (System.Net.IPEndPoint) listener.LocalEndpoint ).Port;\n+\n+ // Send PORT command: PORT h1,h2,h3,h4,p1,p2\n+ string portCommand = FormatPortCommand( localIp, localPort );\n+ var portResult = await ControlStream.SendCommandAsync( new FtpCommandEnvelope\n+ {\n+ FtpCommand = FtpCommand.PORT,\n+ Data = portCommand\n+ } );\n+\n+ if ( portResult.FtpStatusCode != FtpStatusCode.CommandOK )\n+ {\n+ listener.Stop();\n+ throw new FtpException( \"PORT command failed: \" + portResult.ResponseMessage );\n+ }\n+\n+ // Return a lazy-accept stream ÔÇö the actual accept happens when the caller\n+ // first reads from the stream (i.e. after LIST/STOR has been sent)\n+ return new Infrastructure.Stream.ActiveDataStream( listener, ControlStream, Logger );\n+ }\n+\n+ /// \n+ /// Formats an IP and port into PORT command arguments: h1,h2,h3,h4,p1,p2\n+ /// \n+ private static string FormatPortCommand( string ip, int port )\n+ {\n+ string ipPart = ip.Replace( '.', ',' );\n+ int highByte = port / 256;\n+ int lowByte = port % 256;\n+ return $\"{ipPart},{highByte},{lowByte}\";\n+ }","path":"src/CoreFtp/FtpClient.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"FormatPortCommand accepts a free-form string and does a simple '.'ÔåÆ',' replace; this will silently produce malformed PORT arguments if ActiveExternalIp is a hostname, contains whitespace, or includes an IPv6 literal. Consider parsing/validating the input with IPAddress.TryParse (and rejecting non-IPv4 for PORT), so configuration mistakes fail fast with a clear message.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157033","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157033"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157033"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157033/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":714,"original_start_line":714,"start_side":"RIGHT","line":720,"original_line":720,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":76,"position":76,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157041","pull_request_review_id":3871052843,"id":2868157041,"node_id":"PRRC_kwDOBBPGOs6q9Jpx","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;\n+ logger?.LogDebug( \"[ActiveDataStream] Data connection accepted\" );\n+ }\n+\n+ private void EnsureAccepted()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ EnsureAcceptedAsync().GetAwaiter().GetResult();\n+ }","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"EnsureAccepted() blocks on EnsureAcceptedAsync().GetAwaiter().GetResult(), which is sync-over-async and can deadlock under a SynchronizationContext. For the synchronous Read/Write paths, prefer TcpListener.AcceptTcpClient() (fully sync) or start the accept in advance and wait on it, rather than blocking on an async method.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157041","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157041"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157041"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157041/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":45,"original_start_line":45,"start_side":"RIGHT","line":51,"original_line":51,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":51,"position":51,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157042","pull_request_review_id":3871052843,"id":2868157042,"node_id":"PRRC_kwDOBBPGOs6q9Jpy","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;\n+ logger?.LogDebug( \"[ActiveDataStream] Data connection accepted\" );\n+ }","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"EnsureAcceptedAsync() uses AcceptTcpClientAsync() without any cancellation support, so ReadAsync/WriteAsync cancellation tokens wonÔÇÖt be able to abort a hung accept if the server never connects. Consider wiring cancellation through (e.g., AcceptTcpClientAsync(token) on newer TFMs, or Task.WhenAny with token + listener.Stop()), and ensure listener shutdown correctly unblocks the accept.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157042","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157042"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157042"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157042/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":33,"original_start_line":33,"start_side":"RIGHT","line":43,"original_line":43,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":43,"position":43,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157045","pull_request_review_id":3871052843,"id":2868157045,"node_id":"PRRC_kwDOBBPGOs6q9Jp1","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"Active mode currently accepts the first inbound connection to the listener without verifying itÔÇÖs from the expected FTP server. This allows a different host to connect first and inject/consume data. Consider validating acceptedClient.RemoteEndPoint against the control connectionÔÇÖs remote address (or resolved server address) and rejecting unexpected peers before wrapping the stream.","created_at":"2026-03-01T01:33:39Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157045","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157045"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157045"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157045/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":38,"original_start_line":38,"start_side":"RIGHT","line":41,"original_line":41,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":41,"position":41,"subject_type":"line"}] diff --git a/src/CoreFtp/Components/DirectoryListing/DirectoryProviderBase.cs b/src/CoreFtp/Components/DirectoryListing/DirectoryProviderBase.cs index 69283f5..24dbef2 100644 --- a/src/CoreFtp/Components/DirectoryListing/DirectoryProviderBase.cs +++ b/src/CoreFtp/Components/DirectoryListing/DirectoryProviderBase.cs @@ -16,35 +16,20 @@ internal abstract class DirectoryProviderBase : IDirectoryProvider protected ILogger logger; protected Stream stream; - protected IEnumerable RetrieveDirectoryListing() + protected async Task> RetrieveDirectoryListingAsync() { - string line; - while ( ( line = ReadLine( ftpClient.ControlStream.Encoding ) ) != null ) + var lines = new List(); + using (var reader = new StreamReader(stream, ftpClient.ControlStream.Encoding)) { - logger?.LogDebug( line ); - yield return line; - } - } - - protected string ReadLine( Encoding encoding ) - { - if ( encoding == null ) - throw new ArgumentNullException( nameof( encoding ) ); - - var data = new List(); - var buf = new byte[1]; - string line = null; - - while ( stream.Read( buf, 0, buf.Length ) > 0 ) - { - data.Add( buf[ 0 ] ); - if ( (char) buf[ 0 ] != '\n' ) - continue; - line = encoding.GetString( data.ToArray() ).Trim( '\r', '\n' ); - break; + string line; + while ((line = await reader.ReadLineAsync()) != null) + { + logger?.LogDebug(line); + lines.Add(line); + } } - return line; + return lines; } public virtual Task> ListAllAsync() diff --git a/src/CoreFtp/Components/DirectoryListing/ListDirectoryProvider.cs b/src/CoreFtp/Components/DirectoryListing/ListDirectoryProvider.cs index 17d84e1..05bdb31 100644 --- a/src/CoreFtp/Components/DirectoryListing/ListDirectoryProvider.cs +++ b/src/CoreFtp/Components/DirectoryListing/ListDirectoryProvider.cs @@ -13,7 +13,7 @@ internal class ListDirectoryProvider : DirectoryProviderBase { private readonly List directoryParsers; - public ListDirectoryProvider( FtpClient ftpClient, ILogger logger, FtpClientConfiguration configuration ) + public ListDirectoryProvider(FtpClient ftpClient, ILogger logger, FtpClientConfiguration configuration) { this.ftpClient = ftpClient; this.logger = logger; @@ -21,8 +21,8 @@ public ListDirectoryProvider( FtpClient ftpClient, ILogger logger, FtpClientConf directoryParsers = new List { - new UnixDirectoryParser( logger ), - new DosDirectoryParser( logger ), + new UnixDirectoryParser(logger), + new DosDirectoryParser(logger), }; } @@ -38,8 +38,8 @@ internal void AddParser(IListDirectoryParser parser) private void EnsureLoggedIn() { - if ( !ftpClient.IsConnected || !ftpClient.IsAuthenticated ) - throw new FtpException( "User must be logged in" ); + if (!ftpClient.IsConnected || !ftpClient.IsAuthenticated) + throw new FtpException("User must be logged in"); } public override async Task> ListAllAsync() @@ -60,7 +60,7 @@ public override async Task> ListFilesAsyn try { await ftpClient.dataSocketSemaphore.WaitAsync(); - return await ListNodesAsync( FtpNodeType.File ); + return await ListNodesAsync(FtpNodeType.File); } finally { @@ -73,7 +73,7 @@ public override async Task> ListDirectori try { await ftpClient.dataSocketSemaphore.WaitAsync(); - return await ListNodesAsync( FtpNodeType.Directory ); + return await ListNodesAsync(FtpNodeType.Directory); } finally { @@ -86,27 +86,28 @@ public override async Task> ListDirectori /// /// /// - private async Task> ListNodesAsync( FtpNodeType? ftpNodeType = null ) + private async Task> ListNodesAsync(FtpNodeType? ftpNodeType = null) { EnsureLoggedIn(); - logger?.LogDebug( $"[ListDirectoryProvider] Listing {ftpNodeType}" ); + logger?.LogDebug($"[ListDirectoryProvider] Listing {ftpNodeType}"); try { stream = await ftpClient.ConnectDataStreamAsync(); - var result = await ftpClient.ControlStream.SendCommandAsync( new FtpCommandEnvelope + var result = await ftpClient.ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.LIST - } ); + }); - if ( ( result.FtpStatusCode != FtpStatusCode.DataAlreadyOpen ) && ( result.FtpStatusCode != FtpStatusCode.OpeningData ) ) - throw new FtpException( "Could not retrieve directory listing " + result.ResponseMessage ); + if ((result.FtpStatusCode != FtpStatusCode.DataAlreadyOpen) && + (result.FtpStatusCode != FtpStatusCode.OpeningData)) + throw new FtpException("Could not retrieve directory listing " + result.ResponseMessage); - var directoryListing = RetrieveDirectoryListing(); + var directoryListing = await RetrieveDirectoryListingAsync(); - var nodes = ParseLines( directoryListing.ToList().AsReadOnly() ) - .Where( x => !ftpNodeType.HasValue || x.NodeType == ftpNodeType ) + var nodes = ParseLines(directoryListing.AsReadOnly()) + .Where(x => !ftpNodeType.HasValue || x.NodeType == ftpNodeType) .ToList(); return nodes.AsReadOnly(); @@ -117,23 +118,23 @@ private async Task> ListNodesAsync( FtpNo } } - private IEnumerable ParseLines( IReadOnlyList lines ) + private IEnumerable ParseLines(IReadOnlyList lines) { - if ( !lines.Any() ) + if (!lines.Any()) yield break; - var parser = directoryParsers.Count == 1 - ? directoryParsers[ 0 ] - : directoryParsers.FirstOrDefault( x => x.Test( lines[ 0 ] ) ); + var parser = directoryParsers.Count == 1 + ? directoryParsers[0] + : directoryParsers.FirstOrDefault(x => x.Test(lines[0])); - if ( parser == null ) + if (parser == null) yield break; - foreach ( string line in lines ) + foreach (string line in lines) { - var parsed = parser.Parse( line ); + var parsed = parser.Parse(line); - if ( parsed != null ) + if (parsed != null) yield return parsed; } } diff --git a/src/CoreFtp/Components/DirectoryListing/MlsdDirectoryProvider.cs b/src/CoreFtp/Components/DirectoryListing/MlsdDirectoryProvider.cs index 38b257f..9d93be2 100644 --- a/src/CoreFtp/Components/DirectoryListing/MlsdDirectoryProvider.cs +++ b/src/CoreFtp/Components/DirectoryListing/MlsdDirectoryProvider.cs @@ -10,7 +10,7 @@ internal class MlsdDirectoryProvider : DirectoryProviderBase { - public MlsdDirectoryProvider( FtpClient ftpClient, ILogger logger, FtpClientConfiguration configuration ) + public MlsdDirectoryProvider(FtpClient ftpClient, ILogger logger, FtpClientConfiguration configuration) { this.ftpClient = ftpClient; this.configuration = configuration; @@ -19,8 +19,8 @@ public MlsdDirectoryProvider( FtpClient ftpClient, ILogger logger, FtpClientConf private void EnsureLoggedIn() { - if ( !ftpClient.IsConnected || !ftpClient.IsAuthenticated ) - throw new FtpException( "User must be logged in" ); + if (!ftpClient.IsConnected || !ftpClient.IsAuthenticated) + throw new FtpException("User must be logged in"); } public override async Task> ListAllAsync() @@ -41,7 +41,7 @@ public override async Task> ListFilesAsyn try { await ftpClient.dataSocketSemaphore.WaitAsync(); - return await ListNodeTypeAsync( FtpNodeType.File ); + return await ListNodeTypeAsync(FtpNodeType.File); } finally { @@ -54,7 +54,7 @@ public override async Task> ListDirectori try { await ftpClient.dataSocketSemaphore.WaitAsync(); - return await ListNodeTypeAsync( FtpNodeType.Directory ); + return await ListNodeTypeAsync(FtpNodeType.Directory); } finally { @@ -67,7 +67,7 @@ public override async Task> ListDirectori /// /// /// - private async Task> ListNodeTypeAsync( FtpNodeType? ftpNodeType = null ) + private async Task> ListNodeTypeAsync(FtpNodeType? ftpNodeType = null) { string nodeTypeString = !ftpNodeType.HasValue ? "all" @@ -75,26 +75,28 @@ private async Task> ListNodeTypeAsync( Ft ? "file" : "dir"; - logger?.LogDebug( $"[MlsdDirectoryProvider] Listing {ftpNodeType}" ); + logger?.LogDebug($"[MlsdDirectoryProvider] Listing {ftpNodeType}"); EnsureLoggedIn(); try { stream = await ftpClient.ConnectDataStreamAsync(); - if ( stream == null ) - throw new FtpException( "Could not establish a data connection" ); + if (stream == null) + throw new FtpException("Could not establish a data connection"); - var result = await ftpClient.ControlStream.SendCommandAsync( FtpCommand.MLSD ); - if ( ( result.FtpStatusCode != FtpStatusCode.DataAlreadyOpen ) && ( result.FtpStatusCode != FtpStatusCode.OpeningData ) && ( result.FtpStatusCode != FtpStatusCode.ClosingData ) ) - throw new FtpException( "Could not retrieve directory listing " + result.ResponseMessage ); + var result = await ftpClient.ControlStream.SendCommandAsync(FtpCommand.MLSD); + if ((result.FtpStatusCode != FtpStatusCode.DataAlreadyOpen) && + (result.FtpStatusCode != FtpStatusCode.OpeningData) && + (result.FtpStatusCode != FtpStatusCode.ClosingData)) + throw new FtpException("Could not retrieve directory listing " + result.ResponseMessage); - var directoryListing = RetrieveDirectoryListing().ToList(); + var directoryListing = await RetrieveDirectoryListingAsync(); - var nodes = ( from node in directoryListing - where !node.IsNullOrWhiteSpace() - where !ftpNodeType.HasValue || node.Contains( $"type={nodeTypeString}" ) - select node.ToFtpNode() ) + var nodes = (from node in directoryListing + where !node.IsNullOrWhiteSpace() + where !ftpNodeType.HasValue || node.Contains($"type={nodeTypeString}") + select node.ToFtpNode()) .ToList(); diff --git a/src/CoreFtp/FtpClient.cs b/src/CoreFtp/FtpClient.cs index cab46ab..25d98b9 100644 --- a/src/CoreFtp/FtpClient.cs +++ b/src/CoreFtp/FtpClient.cs @@ -21,7 +21,7 @@ public class FtpClient : IFtpClient private IDirectoryProvider directoryProvider; private ILogger logger; private Stream dataStream; - internal readonly SemaphoreSlim dataSocketSemaphore = new SemaphoreSlim( 1, 1 ); + internal readonly SemaphoreSlim dataSocketSemaphore = new SemaphoreSlim(1, 1); public FtpClientConfiguration Configuration { get; private set; } internal IEnumerable Features { get; private set; } @@ -41,30 +41,32 @@ public ILogger Logger } } - public FtpClient() { } + public FtpClient() + { + } - public FtpClient( FtpClientConfiguration configuration ) + public FtpClient(FtpClientConfiguration configuration) { - Configure( configuration ); + Configure(configuration); } - public void Configure( FtpClientConfiguration configuration ) + public void Configure(FtpClientConfiguration configuration) { Configuration = configuration; - if ( configuration.Host == null ) - throw new ArgumentNullException( nameof( configuration.Host ) ); + if (configuration.Host == null) + throw new ArgumentNullException(nameof(configuration.Host)); - if ( Uri.IsWellFormedUriString( configuration.Host, UriKind.Absolute ) ) + if (Uri.IsWellFormedUriString(configuration.Host, UriKind.Absolute)) { - configuration.Host = new Uri( configuration.Host ).Host; + configuration.Host = new Uri(configuration.Host).Host; } - ControlStream = new FtpControlStream( Configuration, new DnsResolver() ) + ControlStream = new FtpControlStream(Configuration, new DnsResolver()) { Encoding = Configuration.BaseEncoding }; - Configuration.BaseDirectory = $"/{Configuration.BaseDirectory.TrimStart( '/' )}"; + Configuration.BaseDirectory = $"/{Configuration.BaseDirectory.TrimStart('/')}"; } /// @@ -73,7 +75,7 @@ public void Configure( FtpClientConfiguration configuration ) /// public async Task LoginAsync() { - if ( IsConnected ) + if (IsConnected) await LogOutAsync(); string username = Configuration.Username.IsNullOrWhiteSpace() @@ -82,49 +84,50 @@ public async Task LoginAsync() await ControlStream.ConnectAsync(); - var usrResponse = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + var usrResponse = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.USER, Data = username - } ); + }); - await BailIfResponseNotAsync( usrResponse, FtpStatusCode.SendUserCommand, FtpStatusCode.SendPasswordCommand, FtpStatusCode.LoggedInProceed ); + await BailIfResponseNotAsync(usrResponse, FtpStatusCode.SendUserCommand, FtpStatusCode.SendPasswordCommand, + FtpStatusCode.LoggedInProceed); - var passResponse = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + var passResponse = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.PASS, Data = username != Constants.ANONYMOUS_USER ? Configuration.Password : string.Empty - } ); + }); - await BailIfResponseNotAsync( passResponse, FtpStatusCode.LoggedInProceed ); + await BailIfResponseNotAsync(passResponse, FtpStatusCode.LoggedInProceed); IsAuthenticated = true; - if ( ControlStream.IsEncrypted ) + if (ControlStream.IsEncrypted) { - await ControlStream.SendCommandAsync( new FtpCommandEnvelope + await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.PBSZ, Data = "0" - } ); + }); - await ControlStream.SendCommandAsync( new FtpCommandEnvelope + await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.PROT, Data = "P" - } ); + }); } Features = await DetermineFeaturesAsync(); directoryProvider = DetermineDirectoryProvider(); await EnableUTF8IfPossible(); - await SetTransferMode( Configuration.Mode, Configuration.ModeSecondType ); + await SetTransferMode(Configuration.Mode, Configuration.ModeSecondType); - if ( Configuration.BaseDirectory != "/" ) + if (Configuration.BaseDirectory != "/") { - await CreateDirectoryAsync( Configuration.BaseDirectory ); + await CreateDirectoryAsync(Configuration.BaseDirectory); } - await ChangeWorkingDirectoryAsync( Configuration.BaseDirectory ); + await ChangeWorkingDirectoryAsync(Configuration.BaseDirectory); } /// @@ -133,19 +136,19 @@ public async Task LoginAsync() public async Task LogOutAsync() { await IgnoreStaleData(); - if ( !IsConnected ) + if (!IsConnected) return; - Logger?.LogTrace( "[FtpClient] Logging out" ); - try + Logger?.LogTrace("[FtpClient] Logging out"); + try { - await ControlStream.SendCommandAsync( FtpCommand.QUIT ); - } - catch (Exception e) + await ControlStream.SendCommandAsync(FtpCommand.QUIT); + } + catch (Exception e) { - Logger?.LogWarning( 0, e, "Error sending QUIT command during logout" ); + Logger?.LogWarning(0, e, "Error sending QUIT command during logout"); } - finally + finally { ControlStream.Disconnect(); IsAuthenticated = false; @@ -157,29 +160,29 @@ public async Task LogOutAsync() /// /// /// - public async Task ChangeWorkingDirectoryAsync( string directory ) + public async Task ChangeWorkingDirectoryAsync(string directory) { - Logger?.LogTrace( $"[FtpClient] changing directory to {directory}" ); - if ( directory.IsNullOrWhiteSpace() || directory.Equals( "." ) ) - throw new ArgumentOutOfRangeException( nameof( directory ), "Directory supplied was incorrect" ); + Logger?.LogTrace($"[FtpClient] changing directory to {directory}"); + if (directory.IsNullOrWhiteSpace() || directory.Equals(".")) + throw new ArgumentOutOfRangeException(nameof(directory), "Directory supplied was incorrect"); EnsureLoggedIn(); - var response = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + var response = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.CWD, Data = directory - } ); + }); - if ( !response.IsSuccess ) - throw new FtpException( response.ResponseMessage ); + if (!response.IsSuccess) + throw new FtpException(response.ResponseMessage); - var pwdResponse = await ControlStream.SendCommandAsync( FtpCommand.PWD ); + var pwdResponse = await ControlStream.SendCommandAsync(FtpCommand.PWD); - if ( !pwdResponse.IsSuccess ) - throw new FtpException( pwdResponse.ResponseMessage ); + if (!pwdResponse.IsSuccess) + throw new FtpException(pwdResponse.ResponseMessage); - WorkingDirectory = ParseWorkingDirectory( pwdResponse.ResponseMessage ); + WorkingDirectory = ParseWorkingDirectory(pwdResponse.ResponseMessage); } private string ParseWorkingDirectory(string responseMessage) @@ -189,13 +192,14 @@ private string ParseWorkingDirectory(string responseMessage) { return parts[1]; } - + // Fallback for unquoted PWD responses like: /home/user is current directory var firstSpace = responseMessage.IndexOf(' '); if (firstSpace != -1) { return responseMessage.Substring(0, firstSpace); } + return responseMessage; } @@ -204,16 +208,16 @@ private string ParseWorkingDirectory(string responseMessage) /// /// /// - public async Task CreateDirectoryAsync( string directory ) + public async Task CreateDirectoryAsync(string directory) { - if ( directory.IsNullOrWhiteSpace() || directory.Equals( "." ) ) - throw new ArgumentOutOfRangeException( nameof( directory ), "Directory supplied was not valid" ); + if (directory.IsNullOrWhiteSpace() || directory.Equals(".")) + throw new ArgumentOutOfRangeException(nameof(directory), "Directory supplied was not valid"); - Logger?.LogDebug( $"[FtpClient] Creating directory {directory}" ); + Logger?.LogDebug($"[FtpClient] Creating directory {directory}"); EnsureLoggedIn(); - await CreateDirectoryStructureRecursively( directory.Split( '/' ), directory.StartsWith( "/" ) ); + await CreateDirectoryStructureRecursively(directory.Split('/'), directory.StartsWith("/")); } /// @@ -222,27 +226,28 @@ public async Task CreateDirectoryAsync( string directory ) /// /// /// - public async Task RenameAsync( string from, string to ) + public async Task RenameAsync(string from, string to) { EnsureLoggedIn(); - Logger?.LogDebug( $"[FtpClient] Renaming from {from}, to {to}" ); - var renameFromResponse = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + Logger?.LogDebug($"[FtpClient] Renaming from {from}, to {to}"); + var renameFromResponse = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.RNFR, Data = from - } ); + }); - if ( renameFromResponse.FtpStatusCode != FtpStatusCode.FileCommandPending ) - throw new FtpException( renameFromResponse.ResponseMessage ); + if (renameFromResponse.FtpStatusCode != FtpStatusCode.FileCommandPending) + throw new FtpException(renameFromResponse.ResponseMessage); - var renameToResponse = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + var renameToResponse = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.RNTO, Data = to - } ); + }); - if ( renameToResponse.FtpStatusCode != FtpStatusCode.FileActionOK && renameToResponse.FtpStatusCode != FtpStatusCode.ClosingData ) - throw new FtpException( renameFromResponse.ResponseMessage ); + if (renameToResponse.FtpStatusCode != FtpStatusCode.FileActionOK && + renameToResponse.FtpStatusCode != FtpStatusCode.ClosingData) + throw new FtpException(renameFromResponse.ResponseMessage); } /// @@ -250,36 +255,36 @@ public async Task RenameAsync( string from, string to ) /// /// /// - public async Task DeleteDirectoryAsync( string directory ) + public async Task DeleteDirectoryAsync(string directory) { - if ( directory.IsNullOrWhiteSpace() || directory.Equals( "." ) ) - throw new ArgumentOutOfRangeException( nameof( directory ), "Directory supplied was not valid" ); + if (directory.IsNullOrWhiteSpace() || directory.Equals(".")) + throw new ArgumentOutOfRangeException(nameof(directory), "Directory supplied was not valid"); - if ( directory == "/" ) + if (directory == "/") return; - Logger?.LogDebug( $"[FtpClient] Deleting directory {directory}" ); + Logger?.LogDebug($"[FtpClient] Deleting directory {directory}"); EnsureLoggedIn(); - var rmdResponse = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + var rmdResponse = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.RMD, Data = directory - } ); + }); - switch ( rmdResponse.FtpStatusCode ) + switch (rmdResponse.FtpStatusCode) { case FtpStatusCode.CommandOK: case FtpStatusCode.FileActionOK: return; case FtpStatusCode.ActionNotTakenFileUnavailable: - await DeleteNonEmptyDirectory( directory ); + await DeleteNonEmptyDirectory(directory); return; default: - throw new FtpException( rmdResponse.ResponseMessage ); + throw new FtpException(rmdResponse.ResponseMessage); } } @@ -288,24 +293,24 @@ public async Task DeleteDirectoryAsync( string directory ) /// /// /// - private async Task DeleteNonEmptyDirectory( string directory ) + private async Task DeleteNonEmptyDirectory(string directory) { - await ChangeWorkingDirectoryAsync( directory ); + await ChangeWorkingDirectoryAsync(directory); var allNodes = await ListAllAsync(); - foreach ( var file in allNodes.Where( x => x.NodeType == FtpNodeType.File ) ) + foreach (var file in allNodes.Where(x => x.NodeType == FtpNodeType.File)) { - await DeleteFileAsync( file.Name ); + await DeleteFileAsync(file.Name); } - foreach ( var dir in allNodes.Where( x => x.NodeType == FtpNodeType.Directory ) ) + foreach (var dir in allNodes.Where(x => x.NodeType == FtpNodeType.Directory)) { - await DeleteDirectoryAsync( dir.Name ); + await DeleteDirectoryAsync(dir.Name); } - await ChangeWorkingDirectoryAsync( ".." ); - await DeleteDirectoryAsync( directory ); + await ChangeWorkingDirectoryAsync(".."); + await DeleteDirectoryAsync(directory); } /// @@ -313,16 +318,16 @@ private async Task DeleteNonEmptyDirectory( string directory ) /// /// /// - public async Task SetClientName( string clientName ) + public async Task SetClientName(string clientName) { EnsureLoggedIn(); - Logger?.LogDebug( $"[FtpClient] Setting client name to {clientName}" ); + Logger?.LogDebug($"[FtpClient] Setting client name to {clientName}"); - return await ControlStream.SendCommandAsync( new FtpCommandEnvelope + return await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.CLNT, Data = clientName - } ); + }); } /// @@ -330,11 +335,11 @@ public async Task SetClientName( string clientName ) /// /// /// - public async Task OpenFileReadStreamAsync( string fileName ) + public async Task OpenFileReadStreamAsync(string fileName) { - Logger?.LogDebug( $"[FtpClient] Opening file read stream for {fileName}" ); + Logger?.LogDebug($"[FtpClient] Opening file read stream for {fileName}"); - return new FtpDataStream( await OpenFileStreamAsync( fileName, FtpCommand.RETR ), this, Logger ); + return new FtpDataStream(await OpenFileStreamAsync(fileName, FtpCommand.RETR), this, Logger); } /// @@ -342,15 +347,16 @@ public async Task OpenFileReadStreamAsync( string fileName ) /// /// /// - public async Task OpenFileWriteStreamAsync( string fileName ) + public async Task OpenFileWriteStreamAsync(string fileName) { - string filePath = WorkingDirectory.CombineAsUriWith( fileName ); - Logger?.LogDebug( $"[FtpClient] Opening file read stream for {filePath}" ); - var segments = filePath.Split( '/' ) - .Where( x => !x.IsNullOrWhiteSpace() ) - .ToList(); - await CreateDirectoryStructureRecursively( segments.Take( segments.Count - 1 ).ToArray(), filePath.StartsWith( "/" ) ); - return new FtpDataStream( await OpenFileStreamAsync( filePath, FtpCommand.STOR ), this, Logger ); + string filePath = WorkingDirectory.CombineAsUriWith(fileName); + Logger?.LogDebug($"[FtpClient] Opening file read stream for {filePath}"); + var segments = filePath.Split('/') + .Where(x => !x.IsNullOrWhiteSpace()) + .ToList(); + await CreateDirectoryStructureRecursively(segments.Take(segments.Count - 1).ToArray(), + filePath.StartsWith("/")); + return new FtpDataStream(await OpenFileStreamAsync(filePath, FtpCommand.STOR), this, Logger); } /// @@ -358,13 +364,13 @@ public async Task OpenFileWriteStreamAsync( string fileName ) /// /// /// - public async Task CloseFileDataStreamAsync( CancellationToken ctsToken = default( CancellationToken ) ) + public async Task CloseFileDataStreamAsync(CancellationToken ctsToken = default(CancellationToken)) { - Logger?.LogTrace( "[FtpClient] Closing write file stream" ); + Logger?.LogTrace("[FtpClient] Closing write file stream"); dataStream.Dispose(); - if ( ControlStream != null ) - await ControlStream.GetResponseAsync( ctsToken ); + if (ControlStream != null) + await ControlStream.GetResponseAsync(ctsToken); } /// @@ -376,7 +382,7 @@ public async Task> ListAllAsync() try { EnsureLoggedIn(); - Logger?.LogDebug( $"[FtpClient] Listing files in {WorkingDirectory}" ); + Logger?.LogDebug($"[FtpClient] Listing files in {WorkingDirectory}"); return await directoryProvider.ListAllAsync(); } finally @@ -394,7 +400,7 @@ public async Task> ListFilesAsync() try { EnsureLoggedIn(); - Logger?.LogDebug( $"[FtpClient] Listing files in {WorkingDirectory}" ); + Logger?.LogDebug($"[FtpClient] Listing files in {WorkingDirectory}"); return await directoryProvider.ListFilesAsync(); } finally @@ -412,7 +418,7 @@ public async Task> ListDirectoriesAsync() try { EnsureLoggedIn(); - Logger?.LogDebug( $"[FtpClient] Listing directories in {WorkingDirectory}" ); + Logger?.LogDebug($"[FtpClient] Listing directories in {WorkingDirectory}"); return await directoryProvider.ListDirectoriesAsync(); } finally @@ -427,18 +433,18 @@ public async Task> ListDirectoriesAsync() /// /// /// - public async Task DeleteFileAsync( string fileName ) + public async Task DeleteFileAsync(string fileName) { EnsureLoggedIn(); - Logger?.LogDebug( $"[FtpClient] Deleting file {fileName}" ); - var response = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + Logger?.LogDebug($"[FtpClient] Deleting file {fileName}"); + var response = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.DELE, Data = fileName - } ); + }); - if ( !response.IsSuccess ) - throw new FtpException( response.ResponseMessage ); + if (!response.IsSuccess) + throw new FtpException(response.ResponseMessage); } /// @@ -447,20 +453,20 @@ public async Task DeleteFileAsync( string fileName ) /// /// /// - public async Task SetTransferMode( FtpTransferMode transferMode, char secondType = '\0' ) + public async Task SetTransferMode(FtpTransferMode transferMode, char secondType = '\0') { EnsureLoggedIn(); - Logger?.LogTrace( $"[FtpClient] Setting transfer mode {transferMode}, {secondType}" ); - var response = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + Logger?.LogTrace($"[FtpClient] Setting transfer mode {transferMode}, {secondType}"); + var response = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.TYPE, Data = secondType != '\0' - ? $"{(char) transferMode} {secondType}" - : $"{(char) transferMode}" - } ); + ? $"{(char)transferMode} {secondType}" + : $"{(char)transferMode}" + }); - if ( !response.IsSuccess ) - throw new FtpException( response.ResponseMessage ); + if (!response.IsSuccess) + throw new FtpException(response.ResponseMessage); } /// @@ -468,20 +474,20 @@ public async Task SetTransferMode( FtpTransferMode transferMode, char secondType /// /// /// - public async Task GetFileSizeAsync( string fileName ) + public async Task GetFileSizeAsync(string fileName) { EnsureLoggedIn(); - Logger?.LogDebug( $"[FtpClient] Getting file size for {fileName}" ); - var sizeResponse = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + Logger?.LogDebug($"[FtpClient] Getting file size for {fileName}"); + var sizeResponse = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.SIZE, Data = fileName - } ); + }); - if ( sizeResponse.FtpStatusCode != FtpStatusCode.FileStatus ) - throw new FtpException( sizeResponse.ResponseMessage ); + if (sizeResponse.FtpStatusCode != FtpStatusCode.FileStatus) + throw new FtpException(sizeResponse.ResponseMessage); - long fileSize = long.Parse( sizeResponse.ResponseMessage ); + long fileSize = long.Parse(sizeResponse.ResponseMessage); return fileSize; } @@ -491,46 +497,49 @@ public async Task GetFileSizeAsync( string fileName ) /// private IDirectoryProvider DetermineDirectoryProvider() { - Logger?.LogTrace( "[FtpClient] Determining directory provider" ); + Logger?.LogTrace("[FtpClient] Determining directory provider"); - if ( this.UsesMlsd() ) - return new MlsdDirectoryProvider( this, Logger, Configuration ); + if (this.UsesMlsd()) + return new MlsdDirectoryProvider(this, Logger, Configuration); - if ( Configuration.ForceFileSystem.HasValue ) + if (Configuration.ForceFileSystem.HasValue) { - var forcedProvider = new ListDirectoryProvider( this, Logger, Configuration ); + var forcedProvider = new ListDirectoryProvider(this, Logger, Configuration); forcedProvider.ClearParsers(); - - switch ( Configuration.ForceFileSystem.Value ) + + switch (Configuration.ForceFileSystem.Value) { case FtpFileSystemType.Windows: - forcedProvider.AddParser( new Components.DirectoryListing.Parser.DosDirectoryParser( Logger ) ); + forcedProvider.AddParser(new Components.DirectoryListing.Parser.DosDirectoryParser(Logger)); break; case FtpFileSystemType.Unix: - forcedProvider.AddParser( new Components.DirectoryListing.Parser.UnixDirectoryParser( Logger ) ); + forcedProvider.AddParser(new Components.DirectoryListing.Parser.UnixDirectoryParser(Logger)); break; default: - throw new NotSupportedException( $"Unsupported file system type: {Configuration.ForceFileSystem.Value}" ); + throw new NotSupportedException( + $"Unsupported file system type: {Configuration.ForceFileSystem.Value}"); } - + return forcedProvider; } - return new ListDirectoryProvider( this, Logger, Configuration ); + return new ListDirectoryProvider(this, Logger, Configuration); } private async Task> DetermineFeaturesAsync() { EnsureLoggedIn(); - Logger?.LogTrace( "[FtpClient] Determining features" ); - var response = await ControlStream.SendCommandAsync( FtpCommand.FEAT ); + Logger?.LogTrace("[FtpClient] Determining features"); + var response = await ControlStream.SendCommandAsync(FtpCommand.FEAT); - if ( response.FtpStatusCode == FtpStatusCode.CommandSyntaxError || response.FtpStatusCode == FtpStatusCode.CommandNotImplemented ) + if (response.FtpStatusCode == FtpStatusCode.CommandSyntaxError || + response.FtpStatusCode == FtpStatusCode.CommandNotImplemented) return Enumerable.Empty(); - var features = response.Data.Where( x => !x.StartsWith( ( (int) FtpStatusCode.SystemHelpReply ).ToString() ) && !x.IsNullOrWhiteSpace() ) - .Select( x => x.Replace( Constants.CARRIAGE_RETURN, string.Empty ).Trim() ) - .ToList(); + var features = response.Data.Where(x => + !x.StartsWith(((int)FtpStatusCode.SystemHelpReply).ToString()) && !x.IsNullOrWhiteSpace()) + .Select(x => x.Replace(Constants.CARRIAGE_RETURN, string.Empty).Trim()) + .ToList(); return features; } @@ -541,56 +550,57 @@ private async Task> DetermineFeaturesAsync() /// /// /// - private async Task CreateDirectoryStructureRecursively( IReadOnlyCollection directories, bool isRootedPath ) + private async Task CreateDirectoryStructureRecursively(IReadOnlyCollection directories, + bool isRootedPath) { - Logger?.LogDebug( $"[FtpClient] Creating directory structure recursively {string.Join( "/", directories )}" ); + Logger?.LogDebug($"[FtpClient] Creating directory structure recursively {string.Join("/", directories)}"); string originalPath = WorkingDirectory; - if ( isRootedPath && directories.Any() ) - await ChangeWorkingDirectoryAsync( "/" ); + if (isRootedPath && directories.Any()) + await ChangeWorkingDirectoryAsync("/"); - if ( !directories.Any() ) + if (!directories.Any()) return; - if ( directories.Count == 1 ) + if (directories.Count == 1) { - await ControlStream.SendCommandAsync( new FtpCommandEnvelope + await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.MKD, Data = directories.First() - } ); + }); - await ChangeWorkingDirectoryAsync( originalPath ); + await ChangeWorkingDirectoryAsync(originalPath); return; } - foreach ( string directory in directories ) + foreach (string directory in directories) { - if ( directory.IsNullOrWhiteSpace() ) + if (directory.IsNullOrWhiteSpace()) continue; - var response = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + var response = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.CWD, Data = directory - } ); + }); - if ( response.FtpStatusCode != FtpStatusCode.ActionNotTakenFileUnavailable ) + if (response.FtpStatusCode != FtpStatusCode.ActionNotTakenFileUnavailable) continue; - await ControlStream.SendCommandAsync( new FtpCommandEnvelope + await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.MKD, Data = directory - } ); - await ControlStream.SendCommandAsync( new FtpCommandEnvelope + }); + await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.CWD, Data = directory - } ); + }); } - await ChangeWorkingDirectoryAsync( originalPath ); + await ChangeWorkingDirectoryAsync(originalPath); } @@ -600,22 +610,22 @@ private async Task CreateDirectoryStructureRecursively( IReadOnlyCollection /// /// - private async Task OpenFileStreamAsync( string fileName, FtpCommand command ) + private async Task OpenFileStreamAsync(string fileName, FtpCommand command) { EnsureLoggedIn(); - Logger?.LogDebug( $"[FtpClient] Opening filestream for {fileName}, {command}" ); + Logger?.LogDebug($"[FtpClient] Opening filestream for {fileName}, {command}"); dataStream = await ConnectDataStreamAsync(); - var retrResponse = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + var retrResponse = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = command, Data = fileName - } ); + }); - if ( ( retrResponse.FtpStatusCode != FtpStatusCode.DataAlreadyOpen ) && - ( retrResponse.FtpStatusCode != FtpStatusCode.OpeningData ) && - ( retrResponse.FtpStatusCode != FtpStatusCode.ClosingData ) ) - throw new FtpException( retrResponse.ResponseMessage ); + if ((retrResponse.FtpStatusCode != FtpStatusCode.DataAlreadyOpen) && + (retrResponse.FtpStatusCode != FtpStatusCode.OpeningData) && + (retrResponse.FtpStatusCode != FtpStatusCode.ClosingData)) + throw new FtpException(retrResponse.ResponseMessage); return dataStream; } @@ -625,8 +635,8 @@ private async Task OpenFileStreamAsync( string fileName, FtpCommand comm /// private void EnsureLoggedIn() { - if ( !IsConnected || !IsAuthenticated ) - throw new FtpException( "User must be logged in" ); + if (!IsConnected || !IsAuthenticated) + throw new FtpException("User must be logged in"); } /// @@ -635,7 +645,7 @@ private void EnsureLoggedIn() /// internal async Task ConnectDataStreamAsync() { - if ( Configuration.DataConnectionType == FtpDataConnectionType.Active ) + if (Configuration.DataConnectionType == FtpDataConnectionType.Active) return await ConnectActiveDataStreamAsync(); return await ConnectPassiveDataStreamAsync(); @@ -646,29 +656,30 @@ internal async Task ConnectDataStreamAsync() /// private async Task ConnectPassiveDataStreamAsync() { - Logger?.LogTrace( "[FtpClient] Connecting passive data socket" ); + Logger?.LogTrace("[FtpClient] Connecting passive data socket"); - var epsvResult = await ControlStream.SendCommandAsync( FtpCommand.EPSV ); + var epsvResult = await ControlStream.SendCommandAsync(FtpCommand.EPSV); int? passivePortNumber; - if ( epsvResult.FtpStatusCode == FtpStatusCode.EnteringExtendedPassive ) + if (epsvResult.FtpStatusCode == FtpStatusCode.EnteringExtendedPassive) { passivePortNumber = epsvResult.ResponseMessage.ExtractEpsvPortNumber(); } else { // EPSV failed - try regular PASV - var pasvResult = await ControlStream.SendCommandAsync( FtpCommand.PASV ); - if ( pasvResult.FtpStatusCode != FtpStatusCode.EnteringPassive ) - throw new FtpException( pasvResult.ResponseMessage ); + var pasvResult = await ControlStream.SendCommandAsync(FtpCommand.PASV); + if (pasvResult.FtpStatusCode != FtpStatusCode.EnteringPassive) + throw new FtpException(pasvResult.ResponseMessage); passivePortNumber = pasvResult.ResponseMessage.ExtractPasvPortNumber(); } - if ( !passivePortNumber.HasValue ) - throw new FtpException( "Could not determine EPSV/PASV data port" ); + if (!passivePortNumber.HasValue) + throw new FtpException("Could not determine EPSV/PASV data port"); - return await ControlStream.OpenDataStreamAsync( Configuration.Host, passivePortNumber.Value, CancellationToken.None ); + return await ControlStream.OpenDataStreamAsync(Configuration.Host, passivePortNumber.Value, + CancellationToken.None); } /// @@ -678,42 +689,48 @@ private async Task ConnectPassiveDataStreamAsync() /// private async Task ConnectActiveDataStreamAsync() { - Logger?.LogTrace( "[FtpClient] Connecting active data socket" ); + Logger?.LogTrace("[FtpClient] Connecting active data socket"); var localIp = Configuration.ActiveExternalIp ?? ControlStream.LocalIpAddress; - if ( string.IsNullOrEmpty( localIp ) ) - throw new FtpException( "Could not determine local IP address for Active mode. Set ActiveExternalIp in configuration." ); + if (string.IsNullOrEmpty(localIp)) + throw new FtpException( + "Could not determine local IP address for Active mode. Set ActiveExternalIp in configuration."); + + if (!System.Net.IPAddress.TryParse(localIp, out var ipAddress) || + ipAddress.AddressFamily != System.Net.Sockets.AddressFamily.InterNetwork) + throw new FtpException( + $"Active mode requires a valid IPv4 address. Configured address '{localIp}' is invalid or not IPv4."); - var listener = new System.Net.Sockets.TcpListener( System.Net.IPAddress.Any, 0 ); + var listener = new System.Net.Sockets.TcpListener(ipAddress, 0); listener.Start(); - int localPort = ( (System.Net.IPEndPoint) listener.LocalEndpoint ).Port; + int localPort = ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; // Send PORT command: PORT h1,h2,h3,h4,p1,p2 - string portCommand = FormatPortCommand( localIp, localPort ); - var portResult = await ControlStream.SendCommandAsync( new FtpCommandEnvelope + string portCommand = FormatPortCommand(localIp, localPort); + var portResult = await ControlStream.SendCommandAsync(new FtpCommandEnvelope { FtpCommand = FtpCommand.PORT, Data = portCommand - } ); + }); - if ( portResult.FtpStatusCode != FtpStatusCode.CommandOK ) + if (portResult.FtpStatusCode != FtpStatusCode.CommandOK) { listener.Stop(); - throw new FtpException( "PORT command failed: " + portResult.ResponseMessage ); + throw new FtpException("PORT command failed: " + portResult.ResponseMessage); } // Return a lazy-accept stream — the actual accept happens when the caller // first reads from the stream (i.e. after LIST/STOR has been sent) - return new Infrastructure.Stream.ActiveDataStream( listener, ControlStream, Logger ); + return new Infrastructure.Stream.ActiveDataStream(listener, ControlStream, Logger); } /// /// Formats an IP and port into PORT command arguments: h1,h2,h3,h4,p1,p2 /// - private static string FormatPortCommand( string ip, int port ) + private static string FormatPortCommand(string ip, int port) { - string ipPart = ip.Replace( '.', ',' ); + string ipPart = ip.Replace('.', ','); int highByte = port / 256; int lowByte = port % 256; return $"{ipPart},{highByte},{lowByte}"; @@ -725,15 +742,16 @@ private static string FormatPortCommand( string ip, int port ) /// /// /// - private async Task BailIfResponseNotAsync( FtpResponse response, params FtpStatusCode[] codes ) + private async Task BailIfResponseNotAsync(FtpResponse response, params FtpStatusCode[] codes) { - if ( codes.Any( x => x == response.FtpStatusCode ) ) + if (codes.Any(x => x == response.FtpStatusCode)) return; - Logger?.LogDebug( $"Bailing due to response codes being {response.FtpStatusCode}, which is not one of: [{string.Join( ",", codes )}]" ); + Logger?.LogDebug( + $"Bailing due to response codes being {response.FtpStatusCode}, which is not one of: [{string.Join(",", codes)}]"); await LogOutAsync(); - throw new FtpException( response.ResponseMessage ); + throw new FtpException(response.ResponseMessage); } /// @@ -742,28 +760,30 @@ private async Task BailIfResponseNotAsync( FtpResponse response, params FtpStatu /// private async Task EnableUTF8IfPossible() { - if ( Equals( ControlStream.Encoding, Encoding.ASCII ) && Features.Any( x => x == Constants.UTF8 ) ) + if (Equals(ControlStream.Encoding, Encoding.ASCII) && Features.Any(x => x == Constants.UTF8)) { ControlStream.Encoding = Encoding.UTF8; } - if ( Equals( ControlStream.Encoding, Encoding.UTF8 ) ) + if (Equals(ControlStream.Encoding, Encoding.UTF8)) { // If the server supports UTF8 it should already be enabled and this // command should not matter however there are conflicting drafts // about this so we'll just execute it to be safe. - await ControlStream.SendCommandAsync( "OPTS UTF8 ON" ); + await ControlStream.SendCommandAsync("OPTS UTF8 ON"); } } - public async Task SendCommandAsync( FtpCommandEnvelope envelope, CancellationToken token = default( CancellationToken ) ) + public async Task SendCommandAsync(FtpCommandEnvelope envelope, + CancellationToken token = default(CancellationToken)) { - return await ControlStream.SendCommandAsync( envelope, token ); + return await ControlStream.SendCommandAsync(envelope, token); } - public async Task SendCommandAsync( string command, CancellationToken token = default( CancellationToken ) ) + public async Task SendCommandAsync(string command, + CancellationToken token = default(CancellationToken)) { - return await ControlStream.SendCommandAsync( command, token ); + return await ControlStream.SendCommandAsync(command, token); } /// @@ -772,28 +792,28 @@ private async Task EnableUTF8IfPossible() /// private async Task IgnoreStaleData() { - if ( IsConnected && ControlStream.SocketDataAvailable() ) + if (IsConnected && ControlStream.SocketDataAvailable()) { var staleData = await ControlStream.GetResponseAsync(); - Logger?.LogWarning( $"Stale data detected: {staleData.ResponseMessage}" ); + Logger?.LogWarning($"Stale data detected: {staleData.ResponseMessage}"); } } public void Dispose() { - Logger?.LogDebug( "Disposing of FtpClient" ); - try + Logger?.LogDebug("Disposing of FtpClient"); + try { if (IsConnected) { LogOutAsync().GetAwaiter().GetResult(); } - } + } catch (Exception ex) { Logger?.LogWarning(0, ex, "Exception during logout on dispose."); } - + dataStream?.Dispose(); ControlStream?.Dispose(); dataSocketSemaphore?.Dispose(); diff --git a/src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs b/src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs index ff62d87..e0e19f5 100644 --- a/src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs +++ b/src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs @@ -23,31 +23,47 @@ internal class ActiveDataStream : Stream private bool accepted; private bool disposed; - public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger ) + public ActiveDataStream(TcpListener listener, FtpControlStream controlStream, ILogger logger) { - this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) ); - this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) ); + this.listener = listener ?? throw new ArgumentNullException(nameof(listener)); + this.controlStream = controlStream ?? throw new ArgumentNullException(nameof(controlStream)); this.logger = logger; } - private async Task EnsureAcceptedAsync() + private async Task EnsureAcceptedAsync(CancellationToken cancellationToken = default) { - if ( accepted ) + if (accepted) return; - logger?.LogDebug( "[ActiveDataStream] Accepting inbound data connection from server" ); - acceptedClient = await listener.AcceptTcpClientAsync(); - innerStream = await controlStream.WrapDataStreamAsync( acceptedClient ); - accepted = true; - logger?.LogDebug( "[ActiveDataStream] Data connection accepted" ); - } + logger?.LogDebug("[ActiveDataStream] Accepting inbound data connection from server"); - private void EnsureAccepted() - { - if ( accepted ) - return; + var timeoutTask = Task.Delay(controlStream.Configuration.TimeoutSeconds * 1000, cancellationToken); + var acceptTask = listener.AcceptTcpClientAsync(); + + var completedTask = await Task.WhenAny(acceptTask, timeoutTask); + if (completedTask == timeoutTask) + { + cancellationToken.ThrowIfCancellationRequested(); + throw new TimeoutException("Timeout waiting for Active mode data connection."); + } - EnsureAcceptedAsync().GetAwaiter().GetResult(); + acceptedClient = await acceptTask; + + // Validate source IP to prevent data connection hijacking + var remoteEndpoint = acceptedClient.Client.RemoteEndPoint as System.Net.IPEndPoint; + var controlEndpoint = controlStream.RemoteEndPoint; + + if (remoteEndpoint != null && controlEndpoint != null && + !remoteEndpoint.Address.Equals(controlEndpoint.Address)) + { + acceptedClient.Dispose(); + throw new FtpException( + $"Rejected active data connection from unexpected source IP: {remoteEndpoint.Address}"); + } + + innerStream = await controlStream.WrapDataStreamAsync(acceptedClient); + accepted = true; + logger?.LogDebug("[ActiveDataStream] Data connection accepted"); } public override bool CanRead => true; @@ -61,48 +77,77 @@ public override long Position set => throw new NotSupportedException(); } - public override int Read( byte[] buffer, int offset, int count ) + public override int Read(byte[] buffer, int offset, int count) + { + throw new NotSupportedException( + "Synchronous operations are not supported on this stream. Use ReadAsync instead."); + } + + public override async Task ReadAsync(byte[] buffer, int offset, int count, + CancellationToken cancellationToken) + { + await EnsureAcceptedAsync(cancellationToken); + return await innerStream.ReadAsync(buffer, offset, count, cancellationToken); + } + + public override void Write(byte[] buffer, int offset, int count) + { + throw new NotSupportedException( + "Synchronous operations are not supported on this stream. Use WriteAsync instead."); + } + + public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + await EnsureAcceptedAsync(cancellationToken); + await innerStream.WriteAsync(buffer, offset, count, cancellationToken); + } + +#if !NETSTANDARD2_0 && !NET462 + public override int Read(Span buffer) { - EnsureAccepted(); - return innerStream.Read( buffer, offset, count ); + throw new NotSupportedException( + "Synchronous operations are not supported on this stream. Use ReadAsync instead."); } - public override async Task ReadAsync( byte[] buffer, int offset, int count, CancellationToken cancellationToken ) + public override async ValueTask ReadAsync(Memory buffer, + CancellationToken cancellationToken = default) { - await EnsureAcceptedAsync(); - return await innerStream.ReadAsync( buffer, offset, count, cancellationToken ); + await EnsureAcceptedAsync(cancellationToken); + return await innerStream.ReadAsync(buffer, cancellationToken); } - public override void Write( byte[] buffer, int offset, int count ) + public override void Write(ReadOnlySpan buffer) { - EnsureAccepted(); - innerStream.Write( buffer, offset, count ); + throw new NotSupportedException( + "Synchronous operations are not supported on this stream. Use WriteAsync instead."); } - public override async Task WriteAsync( byte[] buffer, int offset, int count, CancellationToken cancellationToken ) + public override async ValueTask WriteAsync(ReadOnlyMemory buffer, + CancellationToken cancellationToken = default) { - await EnsureAcceptedAsync(); - await innerStream.WriteAsync( buffer, offset, count, cancellationToken ); + await EnsureAcceptedAsync(cancellationToken); + await innerStream.WriteAsync(buffer, cancellationToken); } +#endif public override void Flush() { innerStream?.Flush(); } - public override long Seek( long offset, SeekOrigin origin ) + public override long Seek(long offset, SeekOrigin origin) { throw new NotSupportedException(); } - public override void SetLength( long value ) + public override void SetLength(long value) { throw new NotSupportedException(); } - protected override void Dispose( bool disposing ) + protected override void Dispose(bool disposing) { - if ( !disposed && disposing ) + if (!disposed && disposing) { innerStream?.Dispose(); acceptedClient?.Dispose(); @@ -110,7 +155,7 @@ protected override void Dispose( bool disposing ) disposed = true; } - base.Dispose( disposing ); + base.Dispose(disposing); } } } diff --git a/src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs b/src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs index 85d527f..a6fbddd 100644 --- a/src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs +++ b/src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs @@ -18,7 +18,7 @@ public class FtpControlStream : Stream { - protected readonly FtpClientConfiguration Configuration; + internal readonly FtpClientConfiguration Configuration; public ILogger Logger; protected readonly IDnsResolver dnsResolver; protected Socket Socket; @@ -44,7 +44,7 @@ internal string LocalIpAddress { get { - if ( Socket?.LocalEndPoint is System.Net.IPEndPoint localEndPoint ) + if (Socket?.LocalEndPoint is System.Net.IPEndPoint localEndPoint) return localEndPoint.Address.ToString(); return null; } @@ -62,6 +62,8 @@ internal void ResetTimeouts() BaseStream.WriteTimeout = Configuration.TimeoutSeconds * 1000; } + internal System.Net.IPEndPoint RemoteEndPoint => Socket?.RemoteEndPoint as System.Net.IPEndPoint; + public FtpControlStream(FtpClientConfiguration configuration, IDnsResolver dnsResolver) { Logger?.LogDebug("Constructing new FtpSocketStream"); @@ -155,8 +157,6 @@ private int Read(Span buffer) } - - public override void Write(byte[] buffer, int offset, int count) { NetworkStream?.Write(buffer, offset, count); @@ -236,6 +236,7 @@ protected string ReadLine(Encoding encoding, CancellationToken token) throw new FtpException("Line length limit exceeded"); continue; } + line = encoding.GetString(data.ToArray()).Trim('\r', '\n'); break; } @@ -363,16 +364,18 @@ public async Task OpenDataStreamAsync(string host, int port, Cancellatio /// Wraps an accepted TcpClient (from Active mode) into a data connection stream. /// Applies TLS if the control connection is encrypted. /// - public async Task WrapDataStreamAsync( System.Net.Sockets.TcpClient acceptedClient ) + internal async Task WrapDataStreamAsync(System.Net.Sockets.TcpClient acceptedClient) { - Logger?.LogDebug( "[FtpSocketStream] Wrapping accepted data connection" ); - var socketStream = new FtpControlStream( Configuration, dnsResolver ) + Logger?.LogDebug("[FtpSocketStream] Wrapping accepted data connection"); + var socketStream = new FtpControlStream(Configuration, dnsResolver) { Logger = Logger, IsDataConnection = true }; socketStream.Socket = acceptedClient.Client; - socketStream.BaseStream = new NetworkStream( acceptedClient.Client ); + socketStream.Socket.ReceiveTimeout = Configuration.TimeoutSeconds * 1000; + socketStream.Socket.LingerState = new LingerOption(true, 0); + socketStream.BaseStream = new NetworkStream(acceptedClient.Client); - if ( IsEncrypted ) + if (IsEncrypted) { await socketStream.ActivateEncryptionAsync(); } diff --git a/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs b/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs index 7502e4e..98cc78d 100644 --- a/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs +++ b/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs @@ -16,130 +16,130 @@ namespace CoreFtp.Tests.Integration.FtpClientTests public class ActiveModeTests : IDisposable { - private readonly ILogger logger; - private readonly TcpListener controlListener; - private readonly int controlPort; + private readonly ILogger _logger; + private readonly TcpListener _controlListener; + private readonly int _controlPort; - public ActiveModeTests( ITestOutputHelper outputHelper ) + public ActiveModeTests(ITestOutputHelper outputHelper) { - var factory = LoggerFactory.Create( builder => - builder.AddProvider( new XunitLoggerProvider( outputHelper ) ) ); - logger = factory.CreateLogger(); + var factory = LoggerFactory.Create(builder => + builder.AddProvider(new XunitLoggerProvider(outputHelper))); + _logger = factory.CreateLogger(); - controlListener = new TcpListener( IPAddress.Loopback, 0 ); - controlListener.Start(); - controlPort = ( (IPEndPoint) controlListener.LocalEndpoint ).Port; + _controlListener = new TcpListener(IPAddress.Loopback, 0); + _controlListener.Start(); + _controlPort = ((IPEndPoint)_controlListener.LocalEndpoint).Port; } public void Dispose() { - controlListener.Stop(); + _controlListener.Stop(); } - private async Task RunFakeFtpServer( Func handler ) + private async Task RunFakeFtpServer(Func handler) { - using ( var client = await controlListener.AcceptTcpClientAsync() ) - using ( var stream = client.GetStream() ) - using ( var reader = new StreamReader( stream, Encoding.ASCII ) ) - using ( var writer = new StreamWriter( stream, Encoding.ASCII ) { AutoFlush = true } ) + using (var client = await _controlListener.AcceptTcpClientAsync()) + using (var stream = client.GetStream()) + using (var reader = new StreamReader(stream, Encoding.ASCII)) + using (var writer = new StreamWriter(stream, Encoding.ASCII) { AutoFlush = true }) { - await handler( reader, writer ); + await handler(reader, writer); } } - private static (string ip, int port) ParsePortCommand( string portArgs ) + private static (string ip, int port) ParsePortCommand(string portArgs) { - var parts = portArgs.Split( ',' ); - string ip = $"{parts[ 0 ]}.{parts[ 1 ]}.{parts[ 2 ]}.{parts[ 3 ]}"; - int port = int.Parse( parts[ 4 ] ) * 256 + int.Parse( parts[ 5 ] ); + var parts = portArgs.Split(','); + string ip = $"{parts[0]}.{parts[1]}.{parts[2]}.{parts[3]}"; + int port = int.Parse(parts[4]) * 256 + int.Parse(parts[5]); return (ip, port); } [Fact] public async Task ListAllAsync_ActiveMode_ReturnsDirectoryListing() { - var serverTask = RunFakeFtpServer( async ( reader, writer ) => + var serverTask = RunFakeFtpServer(async (reader, writer) => { // Greeting - await writer.WriteLineAsync( "220 Welcome" ); + await writer.WriteLineAsync("220 Welcome"); // USER await reader.ReadLineAsync(); - await writer.WriteLineAsync( "331 Password required" ); + await writer.WriteLineAsync("331 Password required"); // PASS await reader.ReadLineAsync(); - await writer.WriteLineAsync( "230 Logged in" ); + await writer.WriteLineAsync("230 Logged in"); // FEAT await reader.ReadLineAsync(); - await writer.WriteLineAsync( "211-Features:" ); - await writer.WriteLineAsync( " UTF8" ); - await writer.WriteLineAsync( "211 End" ); + await writer.WriteLineAsync("211-Features:"); + await writer.WriteLineAsync(" UTF8"); + await writer.WriteLineAsync("211 End"); // OPTS UTF8 ON await reader.ReadLineAsync(); - await writer.WriteLineAsync( "200 OK" ); + await writer.WriteLineAsync("200 OK"); // TYPE I await reader.ReadLineAsync(); - await writer.WriteLineAsync( "200 OK" ); + await writer.WriteLineAsync("200 OK"); // CWD / await reader.ReadLineAsync(); - await writer.WriteLineAsync( "250 CWD OK" ); + await writer.WriteLineAsync("250 CWD OK"); // PWD await reader.ReadLineAsync(); - await writer.WriteLineAsync( "257 \"/\"" ); + await writer.WriteLineAsync("257 \"/\""); // PORT command var portLine = await reader.ReadLineAsync(); - var portArgs = portLine.Substring( "PORT ".Length ); - var (ip, port) = ParsePortCommand( portArgs ); - await writer.WriteLineAsync( "200 PORT command successful" ); + var portArgs = portLine.Substring("PORT ".Length); + var (ip, port) = ParsePortCommand(portArgs); + await writer.WriteLineAsync("200 PORT command successful"); // LIST await reader.ReadLineAsync(); - await writer.WriteLineAsync( "150 Opening data connection" ); + await writer.WriteLineAsync("150 Opening data connection"); // Server connects back to client's data listener - using ( var dataClient = new TcpClient() ) + using (var dataClient = new TcpClient()) { - await dataClient.ConnectAsync( ip, port ); - using ( var dataStream = dataClient.GetStream() ) - using ( var dataWriter = new StreamWriter( dataStream, Encoding.ASCII ) { AutoFlush = true } ) + await dataClient.ConnectAsync(ip, port); + using (var dataStream = dataClient.GetStream()) + using (var dataWriter = new StreamWriter(dataStream, Encoding.ASCII) { AutoFlush = true }) { - await dataWriter.WriteLineAsync( "-rw-r--r-- 1 user group 1024 Jan 01 00:00 file1.txt" ); - await dataWriter.WriteLineAsync( "drwxr-xr-x 2 user group 4096 Jan 01 00:00 subdir" ); - await dataWriter.WriteLineAsync( "-rw-r--r-- 1 user group 2048 Jan 01 00:00 file2.log" ); + await dataWriter.WriteLineAsync("-rw-r--r-- 1 user group 1024 Jan 01 00:00 file1.txt"); + await dataWriter.WriteLineAsync("drwxr-xr-x 2 user group 4096 Jan 01 00:00 subdir"); + await dataWriter.WriteLineAsync("-rw-r--r-- 1 user group 2048 Jan 01 00:00 file2.log"); } } - await writer.WriteLineAsync( "226 Transfer complete" ); - } ); + await writer.WriteLineAsync("226 Transfer complete"); + }); var config = new FtpClientConfiguration { Host = "localhost", - Port = controlPort, + Port = _controlPort, Username = "test", Password = "pwd", DataConnectionType = FtpDataConnectionType.Active, ActiveExternalIp = "127.0.0.1" }; - using ( var client = new FtpClient( config ) { Logger = logger } ) + using (var client = new FtpClient(config) { Logger = _logger }) { await client.LoginAsync(); var nodes = await client.ListAllAsync(); - nodes.Should().HaveCount( 3 ); - nodes.Count( n => n.NodeType == FtpNodeType.File ).Should().Be( 2 ); - nodes.Count( n => n.NodeType == FtpNodeType.Directory ).Should().Be( 1 ); - nodes.Any( n => n.Name == "file1.txt" ).Should().BeTrue(); - nodes.Any( n => n.Name == "subdir" ).Should().BeTrue(); - nodes.Any( n => n.Name == "file2.log" ).Should().BeTrue(); + nodes.Should().HaveCount(3); + nodes.Count(n => n.NodeType == FtpNodeType.File).Should().Be(2); + nodes.Count(n => n.NodeType == FtpNodeType.Directory).Should().Be(1); + nodes.Any(n => n.Name == "file1.txt").Should().BeTrue(); + nodes.Any(n => n.Name == "subdir").Should().BeTrue(); + nodes.Any(n => n.Name == "file2.log").Should().BeTrue(); } await serverTask; @@ -148,69 +148,69 @@ public async Task ListAllAsync_ActiveMode_ReturnsDirectoryListing() [Fact] public async Task ListFilesAsync_ActiveMode_ReturnsOnlyFiles() { - var serverTask = RunFakeFtpServer( async ( reader, writer ) => + var serverTask = RunFakeFtpServer(async (reader, writer) => { - await writer.WriteLineAsync( "220 Welcome" ); + await writer.WriteLineAsync("220 Welcome"); await reader.ReadLineAsync(); - await writer.WriteLineAsync( "331 Password required" ); + await writer.WriteLineAsync("331 Password required"); await reader.ReadLineAsync(); - await writer.WriteLineAsync( "230 Logged in" ); + await writer.WriteLineAsync("230 Logged in"); await reader.ReadLineAsync(); - await writer.WriteLineAsync( "211-Features:" ); - await writer.WriteLineAsync( " UTF8" ); - await writer.WriteLineAsync( "211 End" ); + await writer.WriteLineAsync("211-Features:"); + await writer.WriteLineAsync(" UTF8"); + await writer.WriteLineAsync("211 End"); await reader.ReadLineAsync(); - await writer.WriteLineAsync( "200 OK" ); + await writer.WriteLineAsync("200 OK"); await reader.ReadLineAsync(); - await writer.WriteLineAsync( "200 OK" ); + await writer.WriteLineAsync("200 OK"); await reader.ReadLineAsync(); - await writer.WriteLineAsync( "250 CWD OK" ); + await writer.WriteLineAsync("250 CWD OK"); await reader.ReadLineAsync(); - await writer.WriteLineAsync( "257 \"/\"" ); + await writer.WriteLineAsync("257 \"/\""); // PORT command var portLine = await reader.ReadLineAsync(); - var portArgs = portLine.Substring( "PORT ".Length ); - var (ip, port) = ParsePortCommand( portArgs ); - await writer.WriteLineAsync( "200 PORT command successful" ); + var portArgs = portLine.Substring("PORT ".Length); + var (ip, port) = ParsePortCommand(portArgs); + await writer.WriteLineAsync("200 PORT command successful"); // LIST await reader.ReadLineAsync(); - await writer.WriteLineAsync( "150 Opening data connection" ); + await writer.WriteLineAsync("150 Opening data connection"); - using ( var dataClient = new TcpClient() ) + using (var dataClient = new TcpClient()) { - await dataClient.ConnectAsync( ip, port ); - using ( var dataStream = dataClient.GetStream() ) - using ( var dataWriter = new StreamWriter( dataStream, Encoding.ASCII ) { AutoFlush = true } ) + await dataClient.ConnectAsync(ip, port); + using (var dataStream = dataClient.GetStream()) + using (var dataWriter = new StreamWriter(dataStream, Encoding.ASCII) { AutoFlush = true }) { - await dataWriter.WriteLineAsync( "-rw-r--r-- 1 user group 1024 Jan 01 00:00 readme.md" ); - await dataWriter.WriteLineAsync( "drwxr-xr-x 2 user group 4096 Jan 01 00:00 docs" ); + await dataWriter.WriteLineAsync("-rw-r--r-- 1 user group 1024 Jan 01 00:00 readme.md"); + await dataWriter.WriteLineAsync("drwxr-xr-x 2 user group 4096 Jan 01 00:00 docs"); } } - await writer.WriteLineAsync( "226 Transfer complete" ); - } ); + await writer.WriteLineAsync("226 Transfer complete"); + }); var config = new FtpClientConfiguration { Host = "localhost", - Port = controlPort, + Port = _controlPort, Username = "test", Password = "pwd", DataConnectionType = FtpDataConnectionType.Active, ActiveExternalIp = "127.0.0.1" }; - using ( var client = new FtpClient( config ) { Logger = logger } ) + using (var client = new FtpClient(config) { Logger = _logger }) { await client.LoginAsync(); var files = await client.ListFilesAsync(); - files.Should().HaveCount( 1 ); - files[ 0 ].Name.Should().Be( "readme.md" ); - files[ 0 ].NodeType.Should().Be( FtpNodeType.File ); + files.Should().HaveCount(1); + files[0].Name.Should().Be("readme.md"); + files[0].NodeType.Should().Be(FtpNodeType.File); } await serverTask; From b6c7c5feaf7237cfc185bb9b85b20466aab4e966 Mon Sep 17 00:00:00 2001 From: Nick Briscoe Date: Sun, 1 Mar 2026 14:23:18 +0000 Subject: [PATCH 3/4] chore: remove accidental test logs and temp files --- .../spark_NICK-PC_2026-03-01_14_20_01.trx | 72 ------------------- issues_comments.json | 1 - pulls_comments.json | 1 - 3 files changed, 74 deletions(-) delete mode 100644 TestResults/spark_NICK-PC_2026-03-01_14_20_01.trx delete mode 100644 issues_comments.json delete mode 100644 pulls_comments.json diff --git a/TestResults/spark_NICK-PC_2026-03-01_14_20_01.trx b/TestResults/spark_NICK-PC_2026-03-01_14_20_01.trx deleted file mode 100644 index adf0ea1..0000000 --- a/TestResults/spark_NICK-PC_2026-03-01_14_20_01.trx +++ /dev/null @@ -1,72 +0,0 @@ - - - - - - - - - - Error sending QUIT command during logout - - System.NotSupportedException : Synchronous operations are not supported on this stream. Use ReadAsync instead. - at CoreFtp.Infrastructure.Stream.ActiveDataStream.Read(Byte[] buffer, Int32 offset, Int32 count) in D:\code\oss\CoreFTP\src\CoreFtp\Infrastructure\Stream\ActiveDataStream.cs:line 82 - at CoreFtp.Components.DirectoryListing.DirectoryProviderBase.ReadLine(Encoding encoding) in D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\DirectoryProviderBase.cs:line 38 - at CoreFtp.Components.DirectoryListing.DirectoryProviderBase.RetrieveDirectoryListing()+MoveNext() in D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\DirectoryProviderBase.cs:line 22 - at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection) - at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source) - at CoreFtp.Components.DirectoryListing.ListDirectoryProvider.ListNodesAsync(Nullable`1 ftpNodeType) in D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\ListDirectoryProvider.cs:line 108 - at CoreFtp.Components.DirectoryListing.ListDirectoryProvider.ListAllAsync() in D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\ListDirectoryProvider.cs:line 50 - at CoreFtp.FtpClient.ListAllAsync() in D:\code\oss\CoreFTP\src\CoreFtp\FtpClient.cs:line 386 - at CoreFtp.FtpClient.ListAllAsync() in D:\code\oss\CoreFTP\src\CoreFtp\FtpClient.cs:line 391 - at CoreFtp.Tests.Integration.FtpClientTests.ActiveModeTests.ListAllAsync_ActiveMode_ReturnsDirectoryListing() in D:\code\oss\CoreFTP\tests\CoreFtp.Tests.Integration\FtpClientTests\ActiveModeTests.cs:line 135 ---- End of stack trace from previous location --- - - - - - - - - - - - - - - - - - - - - - [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v3.0.0+e341b939fe (64-bit .NET 10.0.3) -[xUnit.net 00:00:00.04] Discovering: CoreFtp.Tests.Integration -[xUnit.net 00:00:00.06] Discovered: CoreFtp.Tests.Integration -[xUnit.net 00:00:00.07] Starting: CoreFtp.Tests.Integration -[xUnit.net 00:00:00.16] System.NotSupportedException : Synchronous operations are not supported on this stream. Use ReadAsync instead. -[xUnit.net 00:00:00.16] Stack Trace: -[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Infrastructure\Stream\ActiveDataStream.cs(82,0): at CoreFtp.Infrastructure.Stream.ActiveDataStream.Read(Byte[] buffer, Int32 offset, Int32 count) -[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\DirectoryProviderBase.cs(38,0): at CoreFtp.Components.DirectoryListing.DirectoryProviderBase.ReadLine(Encoding encoding) -[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\DirectoryProviderBase.cs(22,0): at CoreFtp.Components.DirectoryListing.DirectoryProviderBase.RetrieveDirectoryListing()+MoveNext() -[xUnit.net 00:00:00.16] at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection) -[xUnit.net 00:00:00.16] at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source) -[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\ListDirectoryProvider.cs(108,0): at CoreFtp.Components.DirectoryListing.ListDirectoryProvider.ListNodesAsync(Nullable`1 ftpNodeType) -[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\Components\DirectoryListing\ListDirectoryProvider.cs(50,0): at CoreFtp.Components.DirectoryListing.ListDirectoryProvider.ListAllAsync() -[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\FtpClient.cs(386,0): at CoreFtp.FtpClient.ListAllAsync() -[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\src\CoreFtp\FtpClient.cs(391,0): at CoreFtp.FtpClient.ListAllAsync() -[xUnit.net 00:00:00.16] D:\code\oss\CoreFTP\tests\CoreFtp.Tests.Integration\FtpClientTests\ActiveModeTests.cs(135,0): at CoreFtp.Tests.Integration.FtpClientTests.ActiveModeTests.ListAllAsync_ActiveMode_ReturnsDirectoryListing() -[xUnit.net 00:00:00.16] --- End of stack trace from previous location --- -[xUnit.net 00:00:00.16] Output: -[xUnit.net 00:00:00.16] Error sending QUIT command during logout -[xUnit.net 00:00:00.16] Finished: CoreFtp.Tests.Integration - - - - - [xUnit.net 00:00:00.16] CoreFtp.Tests.Integration.FtpClientTests.ActiveModeTests.ListAllAsync_ActiveMode_ReturnsDirectoryListing [FAIL] - - - - \ No newline at end of file diff --git a/issues_comments.json b/issues_comments.json deleted file mode 100644 index ea71526..0000000 --- a/issues_comments.json +++ /dev/null @@ -1 +0,0 @@ -[{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/issues/comments/3978804597","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#issuecomment-3978804597","issue_url":"https://api.github.com/repos/sparkeh9/CoreFTP/issues/55","id":3978804597,"node_id":"IC_kwDOBBPGOs7tJ711","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"created_at":"2026-03-01T01:29:28Z","updated_at":"2026-03-01T01:29:28Z","body":"## Summary of Changes\n\nHello, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!\n\nThis pull request significantly enhances the `CoreFtp` library by integrating full support for Active FTP mode. This addresses a critical limitation, allowing the client to connect to FTP servers that exclusively operate in active mode, thereby broadening the library's compatibility. The implementation includes new configuration options and a clever lazy-accept mechanism to manage the asynchronous nature of active data connections, all while maintaining existing passive mode functionality and ensuring backward compatibility.\n\n### Highlights\n\n* **Active FTP Mode Support**: Added comprehensive support for Active FTP mode (PORT/EPRT), enabling the library to connect to servers that do not support passive mode.\n* **New Configuration Options**: Introduced a new `FtpDataConnectionType` enum and `ActiveExternalIp` property in `FtpClientConfiguration` to allow explicit selection and configuration of active mode.\n* **Lazy Data Stream Acceptance**: Implemented `ActiveDataStream`, a wrapper stream that defers the acceptance of the server's inbound data connection until the first read operation, effectively solving the PORT/LIST sequencing deadlock.\n* **Refactored Data Connection Logic**: Refactored `FtpClient` to separate and manage passive and active data stream connection logic, including a new helper for formatting PORT commands.\n* **Control Stream Enhancements**: Enhanced `FtpControlStream` with a `LocalIpAddress` property and a `WrapDataStreamAsync` method to facilitate the wrapping of accepted data streams, including TLS support.\n* **Integration Tests**: Included new integration tests (`ActiveModeTests.cs`) to thoroughly verify the functionality and correctness of the Active FTP mode implementation.\n* **Backward Compatibility**: Ensured full backward compatibility, with `AutoPassive` remaining the default data connection type and no breaking changes introduced to existing functionality.\n\n
\nChangelog\n\n* **src/CoreFtp/Enum/FtpCommand.cs**\n * Added `PORT` and `EPRT` commands to the `FtpCommand` enumeration.\n* **src/CoreFtp/Enum/FtpDataConnectionType.cs**\n * Added a new enumeration `FtpDataConnectionType` with `AutoPassive` and `Active` options.\n* **src/CoreFtp/FtpClient.cs**\n * Refactored `ConnectDataStreamAsync` to delegate to `ConnectActiveDataStreamAsync` or `ConnectPassiveDataStreamAsync` based on configuration.\n * Implemented `ConnectActiveDataStreamAsync` to handle the setup for active mode data connections, including `TcpListener` and `PORT` command sending.\n * Added a private static helper method `FormatPortCommand` to construct the `PORT` command argument string.\n* **src/CoreFtp/FtpClientConfiguration.cs**\n * Added `DataConnectionType` property to specify the desired FTP data connection mode.\n * Added `ActiveExternalIp` property to allow explicit setting of the external IP for active mode, crucial for NAT scenarios.\n* **src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs**\n * Added a new `ActiveDataStream` class, which is a `Stream` wrapper that lazily accepts an inbound `TcpClient` connection on the first read/write operation.\n* **src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs**\n * Added a `LocalIpAddress` property to retrieve the local IP of the control connection.\n * Added a `WrapDataStreamAsync` method to encapsulate an accepted `TcpClient` into a data stream, applying TLS if the control connection is encrypted.\n* **tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs**\n * Added a new test class `ActiveModeTests` containing integration tests for `ListAllAsync` and `ListFilesAsync` in active mode, using a fake FTP server.\n
\n\n
\nActivity\n\n* No human activity (comments, reviews, etc.) has been recorded for this pull request yet.\n
\n\n
\nUsing Gemini Code Assist\n
\n\nThe full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips.\n\nInvoking Gemini\n\nYou can request assistance from Gemini at any point by creating a comment using either `/gemini ` or `@gemini-code-assist `. Below is a summary of the supported commands on the current page.\n\nFeature | Command | Description\n--- | --- | ---\nCode Review | `/gemini review` | Performs a code review for the current pull request in its current state.\nPull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state.\nComment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments.\nHelp | `/gemini help` | Displays a list of available commands.\n\nCustomization\n\nTo customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github).\n\nLimitations & Feedback\n\nGemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG).\n\nYou can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the [Gemini Code Assist IDE Extension](https://cloud.google.com/products/gemini/code-assist).\n
\n\n\n[^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).\n","author_association":"NONE","pin":null,"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/issues/comments/3978804597/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"performed_via_github_app":{"id":956858,"client_id":"Iv23ctcJt9oWoM6OPLsw","slug":"gemini-code-assist","node_id":"A_kwHOABR6NM4ADpm6","owner":{"login":"google","id":1342004,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEzNDIwMDQ=","avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=4","gravatar_id":"","url":"https://api.github.com/users/google","html_url":"https://github.com/google","followers_url":"https://api.github.com/users/google/followers","following_url":"https://api.github.com/users/google/following{/other_user}","gists_url":"https://api.github.com/users/google/gists{/gist_id}","starred_url":"https://api.github.com/users/google/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/google/subscriptions","organizations_url":"https://api.github.com/users/google/orgs","repos_url":"https://api.github.com/users/google/repos","events_url":"https://api.github.com/users/google/events{/privacy}","received_events_url":"https://api.github.com/users/google/received_events","type":"Organization","user_view_type":"public","site_admin":false},"name":"Gemini Code Assist","description":"**Accelerate code reviews and improve code quality**\r\n\r\nBring the power of [Gemini](https://gemini.google.com/) directly to GitHub, providing AI-powered code reviews on your pull requests, with automatic pull request summaries, ready-to-commit code suggestions and the ability to invoke Gemini for assistance at any point on the PR.\r\n","external_url":"https://developers.google.com/gemini-code-assist/docs/review-github-code","html_url":"https://github.com/apps/gemini-code-assist","created_at":"2024-07-29T20:55:46Z","updated_at":"2025-09-23T20:29:15Z","permissions":{"contents":"read","issues":"write","members":"read","metadata":"read","pull_requests":"write"},"events":["commit_comment","issue_comment","pull_request","pull_request_review","pull_request_review_comment","pull_request_review_thread","push"]}}] diff --git a/pulls_comments.json b/pulls_comments.json deleted file mode 100644 index 024bdec..0000000 --- a/pulls_comments.json +++ /dev/null @@ -1 +0,0 @@ -[{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151692","pull_request_review_id":3871046696,"id":2868151692,"node_id":"PRRC_kwDOBBPGOs6q9IWM","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![security-high](https://www.gstatic.com/codereviewagent/security-high-priority.svg) ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)\n\nIn Active FTP mode, the client opens a local port and waits for the server to connect. The current implementation of `ActiveDataStream.EnsureAcceptedAsync` accepts the first TCP connection that arrives on the listening port without verifying the source IP address. An attacker who can connect to the client's listening port (e.g., by scanning or if the client is on a shared network) can intercept the data stream before the legitimate FTP server connects. This allows the attacker to either steal sensitive data being uploaded or inject malicious data into a download (e.g., a fake directory listing or a malicious file).","created_at":"2026-03-01T01:31:26Z","updated_at":"2026-03-01T01:31:27Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151692","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151692"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151692"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151692/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":null,"original_start_line":null,"start_side":null,"line":39,"original_line":39,"side":"RIGHT","author_association":"NONE","original_position":39,"position":39,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151694","pull_request_review_id":3871046696,"id":2868151694,"node_id":"PRRC_kwDOBBPGOs6q9IWO","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;\n+ logger?.LogDebug( \"[ActiveDataStream] Data connection accepted\" );\n+ }\n+\n+ private void EnsureAccepted()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ EnsureAcceptedAsync().GetAwaiter().GetResult();\n+ }\n+\n+ public override bool CanRead => true;\n+ public override bool CanSeek => false;\n+ public override bool CanWrite => true;\n+ public override long Length => innerStream?.Length ?? 0;\n+\n+ public override long Position\n+ {\n+ get => innerStream?.Position ?? 0;\n+ set => throw new NotSupportedException();\n+ }\n+\n+ public override int Read( byte[] buffer, int offset, int count )\n+ {\n+ EnsureAccepted();\n+ return innerStream.Read( buffer, offset, count );\n+ }","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![high](https://www.gstatic.com/codereviewagent/high-priority.svg)\n\nThis method uses `EnsureAccepted()`, which in turn calls `EnsureAcceptedAsync().GetAwaiter().GetResult()`. This sync-over-async pattern is dangerous and can lead to deadlocks in certain environments (like UI or ASP.NET applications). To make the stream safer to use, it's better to prevent synchronous operations if a fully synchronous implementation isn't feasible. I recommend throwing `NotSupportedException` to enforce async-only usage.\n\n```c#\n public override int Read( byte[] buffer, int offset, int count )\n {\n throw new NotSupportedException(\"Synchronous operations are not supported on this stream. Use ReadAsync instead.\");\n }\n```","created_at":"2026-03-01T01:31:26Z","updated_at":"2026-03-01T01:31:27Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151694","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151694"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151694"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151694/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":64,"original_start_line":64,"start_side":"RIGHT","line":68,"original_line":68,"side":"RIGHT","author_association":"NONE","original_position":68,"position":68,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151695","pull_request_review_id":3871046696,"id":2868151695,"node_id":"PRRC_kwDOBBPGOs6q9IWP","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;\n+ logger?.LogDebug( \"[ActiveDataStream] Data connection accepted\" );\n+ }\n+\n+ private void EnsureAccepted()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ EnsureAcceptedAsync().GetAwaiter().GetResult();\n+ }\n+\n+ public override bool CanRead => true;\n+ public override bool CanSeek => false;\n+ public override bool CanWrite => true;\n+ public override long Length => innerStream?.Length ?? 0;\n+\n+ public override long Position\n+ {\n+ get => innerStream?.Position ?? 0;\n+ set => throw new NotSupportedException();\n+ }\n+\n+ public override int Read( byte[] buffer, int offset, int count )\n+ {\n+ EnsureAccepted();\n+ return innerStream.Read( buffer, offset, count );\n+ }\n+\n+ public override async Task ReadAsync( byte[] buffer, int offset, int count, CancellationToken cancellationToken )\n+ {\n+ await EnsureAcceptedAsync();\n+ return await innerStream.ReadAsync( buffer, offset, count, cancellationToken );\n+ }\n+\n+ public override void Write( byte[] buffer, int offset, int count )\n+ {\n+ EnsureAccepted();\n+ innerStream.Write( buffer, offset, count );\n+ }","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![high](https://www.gstatic.com/codereviewagent/high-priority.svg)\n\nSimilar to the `Read` method, this synchronous `Write` method relies on a sync-over-async call via `EnsureAccepted()`, which can cause deadlocks. To ensure the safety and reliability of the library, this synchronous method should be disallowed. Throwing `NotSupportedException` will guide users to the correct asynchronous API.\n\n```c#\n public override void Write( byte[] buffer, int offset, int count )\n {\n throw new NotSupportedException(\"Synchronous operations are not supported on this stream. Use WriteAsync instead.\");\n }\n```","created_at":"2026-03-01T01:31:27Z","updated_at":"2026-03-01T01:31:27Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151695","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151695"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151695"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151695/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":76,"original_start_line":76,"start_side":"RIGHT","line":80,"original_line":80,"side":"RIGHT","author_association":"NONE","original_position":80,"position":80,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151697","pull_request_review_id":3871046696,"id":2868151697,"node_id":"PRRC_kwDOBBPGOs6q9IWR","diff_hunk":"@@ -660,6 +671,54 @@ internal async Task ConnectDataStreamAsync()\n return await ControlStream.OpenDataStreamAsync( Configuration.Host, passivePortNumber.Value, CancellationToken.None );\n }\n \n+ /// \n+ /// Produces a data socket using Active (PORT/EPRT) mode.\n+ /// Binds a local TcpListener, sends PORT, and returns a lazy-accept stream that\n+ /// accepts the server's inbound connection on the first read (after LIST/STOR is sent).\n+ /// \n+ private async Task ConnectActiveDataStreamAsync()\n+ {\n+ Logger?.LogTrace( \"[FtpClient] Connecting active data socket\" );\n+\n+ var localIp = Configuration.ActiveExternalIp ?? ControlStream.LocalIpAddress;\n+ if ( string.IsNullOrEmpty( localIp ) )\n+ throw new FtpException( \"Could not determine local IP address for Active mode. Set ActiveExternalIp in configuration.\" );\n+\n+ var listener = new System.Net.Sockets.TcpListener( System.Net.IPAddress.Any, 0 );","path":"src/CoreFtp/FtpClient.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![security-medium](https://www.gstatic.com/codereviewagent/security-medium-priority.svg) ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)\n\nThe `TcpListener` is currently bound to `IPAddress.Any`, exposing the listening port on all network interfaces and increasing the attack surface. It is recommended to bind it specifically to the local IP address used for the control connection to mitigate this security risk. Furthermore, while addressing this, consider enhancing the active mode implementation within this method to support `EPRT` for IPv6 compatibility, as the current `PORT` command is limited to IPv4.","created_at":"2026-03-01T01:31:27Z","updated_at":"2026-03-01T01:31:27Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151697","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151697"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868151697"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868151697/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":null,"original_start_line":null,"start_side":null,"line":687,"original_line":687,"side":"RIGHT","author_association":"NONE","original_position":43,"position":43,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154896","pull_request_review_id":3871050263,"id":2868154896,"node_id":"PRRC_kwDOBBPGOs6q9JIQ","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"chatgpt-codex-connector[bot]","id":199175422,"node_id":"BOT_kgDOC98s_g","avatar_url":"https://avatars.githubusercontent.com/in/1144995?v=4","gravatar_id":"","url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D","html_url":"https://github.com/apps/chatgpt-codex-connector","followers_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/followers","following_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/repos","events_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"**![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat) Bound active accept wait to transfer timeout**\n\n`EnsureAcceptedAsync` waits on `AcceptTcpClientAsync()` with no cancellation or timeout, so when Active mode is misconfigured (e.g., wrong `ActiveExternalIp`, NAT/firewall blocks callback, or server never opens data connection) any LIST/RETR/STOR read/write can hang indefinitely and never surface the FTP error response from the control channel. This effectively ignores `TimeoutSeconds` for the most failure-prone part of Active mode and can stall callers permanently.\n\nUseful? React with ­ƒæì┬á/ ­ƒæÄ.","created_at":"2026-03-01T01:32:37Z","updated_at":"2026-03-01T01:32:37Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868154896","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154896"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868154896"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154896/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":null,"original_start_line":null,"start_side":null,"line":39,"original_line":39,"side":"RIGHT","author_association":"NONE","original_position":39,"position":39,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154898","pull_request_review_id":3871050263,"id":2868154898,"node_id":"PRRC_kwDOBBPGOs6q9JIS","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"chatgpt-codex-connector[bot]","id":199175422,"node_id":"BOT_kgDOC98s_g","avatar_url":"https://avatars.githubusercontent.com/in/1144995?v=4","gravatar_id":"","url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D","html_url":"https://github.com/apps/chatgpt-codex-connector","followers_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/followers","following_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/repos","events_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/chatgpt-codex-connector%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Validate active data peer before wrapping socket**\n\nThe first inbound connection is accepted and used as the transfer stream without checking that it comes from the same server as the control connection. In environments where another host can reach the client listener (especially plain FTP where `PORT` is visible on-path), a race connection can be accepted and treated as trusted data, allowing transfer corruption or data exfiltration.\n\nUseful? React with ­ƒæì┬á/ ­ƒæÄ.","created_at":"2026-03-01T01:32:37Z","updated_at":"2026-03-01T01:32:37Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868154898","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154898"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868154898"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868154898/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":39,"original_start_line":39,"start_side":"RIGHT","line":40,"original_line":40,"side":"RIGHT","author_association":"NONE","original_position":40,"position":40,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157022","pull_request_review_id":3871052843,"id":2868157022,"node_id":"PRRC_kwDOBBPGOs6q9Jpe","diff_hunk":"@@ -345,6 +359,27 @@ public async Task OpenDataStreamAsync(string host, int port, Cancellatio\n return socketStream;\n }\n \n+ /// \n+ /// Wraps an accepted TcpClient (from Active mode) into a data connection stream.\n+ /// Applies TLS if the control connection is encrypted.\n+ /// \n+ public async Task WrapDataStreamAsync( System.Net.Sockets.TcpClient acceptedClient )\n+ {\n+ Logger?.LogDebug( \"[FtpSocketStream] Wrapping accepted data connection\" );\n+ var socketStream = new FtpControlStream( Configuration, dnsResolver )\n+ { Logger = Logger, IsDataConnection = true };\n+\n+ socketStream.Socket = acceptedClient.Client;\n+ socketStream.BaseStream = new NetworkStream( acceptedClient.Client );\n+\n+ if ( IsEncrypted )\n+ {\n+ await socketStream.ActivateEncryptionAsync();\n+ }\n+\n+ return socketStream;","path":"src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"WrapDataStreamAsync() builds a data-connection stream from an accepted socket but doesnÔÇÖt apply the same socket configuration as OpenDataStreamAsync()/ConnectSocketAsync (e.g., ReceiveTimeout / linger / any other options), which can change timeout behavior for Active mode transfers and potentially hang indefinitely. Consider mirroring the data-socket setup performed for passive connections before returning the wrapped stream.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157022","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157022"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157022"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157022/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":366,"original_start_line":366,"start_side":"RIGHT","line":380,"original_line":380,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":43,"position":43,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157030","pull_request_review_id":3871052843,"id":2868157030,"node_id":"PRRC_kwDOBBPGOs6q9Jpm","diff_hunk":"@@ -660,6 +671,54 @@ internal async Task ConnectDataStreamAsync()\n return await ControlStream.OpenDataStreamAsync( Configuration.Host, passivePortNumber.Value, CancellationToken.None );\n }\n \n+ /// \n+ /// Produces a data socket using Active (PORT/EPRT) mode.\n+ /// Binds a local TcpListener, sends PORT, and returns a lazy-accept stream that\n+ /// accepts the server's inbound connection on the first read (after LIST/STOR is sent).\n+ /// \n+ private async Task ConnectActiveDataStreamAsync()\n+ {\n+ Logger?.LogTrace( \"[FtpClient] Connecting active data socket\" );\n+\n+ var localIp = Configuration.ActiveExternalIp ?? ControlStream.LocalIpAddress;\n+ if ( string.IsNullOrEmpty( localIp ) )\n+ throw new FtpException( \"Could not determine local IP address for Active mode. Set ActiveExternalIp in configuration.\" );\n+\n+ var listener = new System.Net.Sockets.TcpListener( System.Net.IPAddress.Any, 0 );\n+ listener.Start();\n+\n+ int localPort = ( (System.Net.IPEndPoint) listener.LocalEndpoint ).Port;\n+\n+ // Send PORT command: PORT h1,h2,h3,h4,p1,p2\n+ string portCommand = FormatPortCommand( localIp, localPort );\n+ var portResult = await ControlStream.SendCommandAsync( new FtpCommandEnvelope","path":"src/CoreFtp/FtpClient.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"ConnectActiveDataStreamAsync always uses PORT and FormatPortCommand assumes an IPv4 dotted-quad address. If ActiveExternalIp/LocalIpAddress is IPv6 (contains ':'), the generated command will be invalid and the listener is also IPv4-only (IPAddress.Any). Consider either implementing EPRT for IPv6 (and/or preferring EPRT with fallback), or explicitly validating and throwing a clear exception when a non-IPv4 address is supplied.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157030","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157030"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157030"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157030/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":683,"original_start_line":683,"start_side":"RIGHT","line":694,"original_line":694,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":50,"position":50,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157033","pull_request_review_id":3871052843,"id":2868157033,"node_id":"PRRC_kwDOBBPGOs6q9Jpp","diff_hunk":"@@ -660,6 +671,54 @@ internal async Task ConnectDataStreamAsync()\n return await ControlStream.OpenDataStreamAsync( Configuration.Host, passivePortNumber.Value, CancellationToken.None );\n }\n \n+ /// \n+ /// Produces a data socket using Active (PORT/EPRT) mode.\n+ /// Binds a local TcpListener, sends PORT, and returns a lazy-accept stream that\n+ /// accepts the server's inbound connection on the first read (after LIST/STOR is sent).\n+ /// \n+ private async Task ConnectActiveDataStreamAsync()\n+ {\n+ Logger?.LogTrace( \"[FtpClient] Connecting active data socket\" );\n+\n+ var localIp = Configuration.ActiveExternalIp ?? ControlStream.LocalIpAddress;\n+ if ( string.IsNullOrEmpty( localIp ) )\n+ throw new FtpException( \"Could not determine local IP address for Active mode. Set ActiveExternalIp in configuration.\" );\n+\n+ var listener = new System.Net.Sockets.TcpListener( System.Net.IPAddress.Any, 0 );\n+ listener.Start();\n+\n+ int localPort = ( (System.Net.IPEndPoint) listener.LocalEndpoint ).Port;\n+\n+ // Send PORT command: PORT h1,h2,h3,h4,p1,p2\n+ string portCommand = FormatPortCommand( localIp, localPort );\n+ var portResult = await ControlStream.SendCommandAsync( new FtpCommandEnvelope\n+ {\n+ FtpCommand = FtpCommand.PORT,\n+ Data = portCommand\n+ } );\n+\n+ if ( portResult.FtpStatusCode != FtpStatusCode.CommandOK )\n+ {\n+ listener.Stop();\n+ throw new FtpException( \"PORT command failed: \" + portResult.ResponseMessage );\n+ }\n+\n+ // Return a lazy-accept stream ÔÇö the actual accept happens when the caller\n+ // first reads from the stream (i.e. after LIST/STOR has been sent)\n+ return new Infrastructure.Stream.ActiveDataStream( listener, ControlStream, Logger );\n+ }\n+\n+ /// \n+ /// Formats an IP and port into PORT command arguments: h1,h2,h3,h4,p1,p2\n+ /// \n+ private static string FormatPortCommand( string ip, int port )\n+ {\n+ string ipPart = ip.Replace( '.', ',' );\n+ int highByte = port / 256;\n+ int lowByte = port % 256;\n+ return $\"{ipPart},{highByte},{lowByte}\";\n+ }","path":"src/CoreFtp/FtpClient.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"FormatPortCommand accepts a free-form string and does a simple '.'ÔåÆ',' replace; this will silently produce malformed PORT arguments if ActiveExternalIp is a hostname, contains whitespace, or includes an IPv6 literal. Consider parsing/validating the input with IPAddress.TryParse (and rejecting non-IPv4 for PORT), so configuration mistakes fail fast with a clear message.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157033","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157033"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157033"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157033/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":714,"original_start_line":714,"start_side":"RIGHT","line":720,"original_line":720,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":76,"position":76,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157041","pull_request_review_id":3871052843,"id":2868157041,"node_id":"PRRC_kwDOBBPGOs6q9Jpx","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;\n+ logger?.LogDebug( \"[ActiveDataStream] Data connection accepted\" );\n+ }\n+\n+ private void EnsureAccepted()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ EnsureAcceptedAsync().GetAwaiter().GetResult();\n+ }","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"EnsureAccepted() blocks on EnsureAcceptedAsync().GetAwaiter().GetResult(), which is sync-over-async and can deadlock under a SynchronizationContext. For the synchronous Read/Write paths, prefer TcpListener.AcceptTcpClient() (fully sync) or start the accept in advance and wait on it, rather than blocking on an async method.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157041","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157041"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157041"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157041/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":45,"original_start_line":45,"start_side":"RIGHT","line":51,"original_line":51,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":51,"position":51,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157042","pull_request_review_id":3871052843,"id":2868157042,"node_id":"PRRC_kwDOBBPGOs6q9Jpy","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;\n+ logger?.LogDebug( \"[ActiveDataStream] Data connection accepted\" );\n+ }","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"EnsureAcceptedAsync() uses AcceptTcpClientAsync() without any cancellation support, so ReadAsync/WriteAsync cancellation tokens wonÔÇÖt be able to abort a hung accept if the server never connects. Consider wiring cancellation through (e.g., AcceptTcpClientAsync(token) on newer TFMs, or Task.WhenAny with token + listener.Stop()), and ensure listener shutdown correctly unblocks the accept.","created_at":"2026-03-01T01:33:38Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157042","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157042"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157042"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157042/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":33,"original_start_line":33,"start_side":"RIGHT","line":43,"original_line":43,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":43,"position":43,"subject_type":"line"},{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157045","pull_request_review_id":3871052843,"id":2868157045,"node_id":"PRRC_kwDOBBPGOs6q9Jp1","diff_hunk":"@@ -0,0 +1,116 @@\n+namespace CoreFtp.Infrastructure.Stream\n+{\n+ using System;\n+ using System.IO;\n+ using System.Net.Sockets;\n+ using System.Threading;\n+ using System.Threading.Tasks;\n+ using Microsoft.Extensions.Logging;\n+\n+ /// \n+ /// A wrapper stream for Active FTP mode that lazily accepts the server's \n+ /// inbound data connection on the first read operation. This defers the \n+ /// TcpListener.AcceptTcpClient call until after the data command (e.g. LIST)\n+ /// has been sent on the control channel.\n+ /// \n+ internal class ActiveDataStream : Stream\n+ {\n+ private readonly TcpListener listener;\n+ private readonly FtpControlStream controlStream;\n+ private readonly ILogger logger;\n+ private Stream innerStream;\n+ private TcpClient acceptedClient;\n+ private bool accepted;\n+ private bool disposed;\n+\n+ public ActiveDataStream( TcpListener listener, FtpControlStream controlStream, ILogger logger )\n+ {\n+ this.listener = listener ?? throw new ArgumentNullException( nameof( listener ) );\n+ this.controlStream = controlStream ?? throw new ArgumentNullException( nameof( controlStream ) );\n+ this.logger = logger;\n+ }\n+\n+ private async Task EnsureAcceptedAsync()\n+ {\n+ if ( accepted )\n+ return;\n+\n+ logger?.LogDebug( \"[ActiveDataStream] Accepting inbound data connection from server\" );\n+ acceptedClient = await listener.AcceptTcpClientAsync();\n+ innerStream = await controlStream.WrapDataStreamAsync( acceptedClient );\n+ accepted = true;","path":"src/CoreFtp/Infrastructure/Stream/ActiveDataStream.cs","commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","original_commit_id":"3cc0c6376df8c7678bae0e2b9a97f785e8b224bc","user":{"login":"Copilot","id":175728472,"node_id":"BOT_kgDOCnlnWA","avatar_url":"https://avatars.githubusercontent.com/in/946600?v=4","gravatar_id":"","url":"https://api.github.com/users/Copilot","html_url":"https://github.com/apps/copilot-pull-request-reviewer","followers_url":"https://api.github.com/users/Copilot/followers","following_url":"https://api.github.com/users/Copilot/following{/other_user}","gists_url":"https://api.github.com/users/Copilot/gists{/gist_id}","starred_url":"https://api.github.com/users/Copilot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Copilot/subscriptions","organizations_url":"https://api.github.com/users/Copilot/orgs","repos_url":"https://api.github.com/users/Copilot/repos","events_url":"https://api.github.com/users/Copilot/events{/privacy}","received_events_url":"https://api.github.com/users/Copilot/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"Active mode currently accepts the first inbound connection to the listener without verifying itÔÇÖs from the expected FTP server. This allows a different host to connect first and inject/consume data. Consider validating acceptedClient.RemoteEndPoint against the control connectionÔÇÖs remote address (or resolved server address) and rejecting unexpected peers before wrapping the stream.","created_at":"2026-03-01T01:33:39Z","updated_at":"2026-03-01T01:33:39Z","html_url":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157045","pull_request_url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55","_links":{"self":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157045"},"html":{"href":"https://github.com/sparkeh9/CoreFTP/pull/55#discussion_r2868157045"},"pull_request":{"href":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/55"}},"reactions":{"url":"https://api.github.com/repos/sparkeh9/CoreFTP/pulls/comments/2868157045/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":38,"original_start_line":38,"start_side":"RIGHT","line":41,"original_line":41,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":41,"position":41,"subject_type":"line"}] From 27867cd6924f7dfaa6b6fb72473807675c2a7b6f Mon Sep 17 00:00:00 2001 From: Nick Briscoe Date: Sun, 1 Mar 2026 14:25:46 +0000 Subject: [PATCH 4/4] style: revert underscore fields in ActiveModeTests.cs per repo conventions --- .../FtpClientTests/ActiveModeTests.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs b/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs index 98cc78d..3c7659f 100644 --- a/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs +++ b/tests/CoreFtp.Tests.Integration/FtpClientTests/ActiveModeTests.cs @@ -16,29 +16,29 @@ namespace CoreFtp.Tests.Integration.FtpClientTests public class ActiveModeTests : IDisposable { - private readonly ILogger _logger; - private readonly TcpListener _controlListener; - private readonly int _controlPort; + private readonly ILogger logger; + private readonly TcpListener controlListener; + private readonly int controlPort; public ActiveModeTests(ITestOutputHelper outputHelper) { var factory = LoggerFactory.Create(builder => builder.AddProvider(new XunitLoggerProvider(outputHelper))); - _logger = factory.CreateLogger(); + logger = factory.CreateLogger(); - _controlListener = new TcpListener(IPAddress.Loopback, 0); - _controlListener.Start(); - _controlPort = ((IPEndPoint)_controlListener.LocalEndpoint).Port; + controlListener = new TcpListener(IPAddress.Loopback, 0); + controlListener.Start(); + controlPort = ((IPEndPoint)controlListener.LocalEndpoint).Port; } public void Dispose() { - _controlListener.Stop(); + controlListener.Stop(); } private async Task RunFakeFtpServer(Func handler) { - using (var client = await _controlListener.AcceptTcpClientAsync()) + using (var client = await controlListener.AcceptTcpClientAsync()) using (var stream = client.GetStream()) using (var reader = new StreamReader(stream, Encoding.ASCII)) using (var writer = new StreamWriter(stream, Encoding.ASCII) { AutoFlush = true }) @@ -121,14 +121,14 @@ public async Task ListAllAsync_ActiveMode_ReturnsDirectoryListing() var config = new FtpClientConfiguration { Host = "localhost", - Port = _controlPort, + Port = controlPort, Username = "test", Password = "pwd", DataConnectionType = FtpDataConnectionType.Active, ActiveExternalIp = "127.0.0.1" }; - using (var client = new FtpClient(config) { Logger = _logger }) + using (var client = new FtpClient(config) { Logger = logger }) { await client.LoginAsync(); @@ -195,14 +195,14 @@ public async Task ListFilesAsync_ActiveMode_ReturnsOnlyFiles() var config = new FtpClientConfiguration { Host = "localhost", - Port = _controlPort, + Port = controlPort, Username = "test", Password = "pwd", DataConnectionType = FtpDataConnectionType.Active, ActiveExternalIp = "127.0.0.1" }; - using (var client = new FtpClient(config) { Logger = _logger }) + using (var client = new FtpClient(config) { Logger = logger }) { await client.LoginAsync();