Cleanup SocketAbstraction

* Use "using" and ManualResetEventSlim in Connect
* Delete unused and unnecessary methods

Author: Rob-Hague

src/Renci.SshNet/Abstractions/SocketAbstraction.cs

@@ -3,148 +3,43 @@
 using System.Net;
 using System.Net.Sockets;
 using System.Threading;
+#if NET6_0_OR_GREATER == false
 using System.Threading.Tasks;
+#endif
 
 using Renci.SshNet.Common;
 
 namespace Renci.SshNet.Abstractions
 {
     internal static partial class SocketAbstraction
     {
-        public static bool CanRead(Socket socket)
-        {
-            if (socket.Connected)
-            {
-                return socket.Poll(-1, SelectMode.SelectRead) && socket.Available > 0;
-            }
-
-            return false;
-        }
-
-        /// <summary>
-        /// Returns a value indicating whether the specified <see cref="Socket"/> can be used
-        /// to send data.
-        /// </summary>
-        /// <param name="socket">The <see cref="Socket"/> to check.</param>
-        /// <returns>
-        /// <see langword="true"/> if <paramref name="socket"/> can be written to; otherwise, <see langword="false"/>.
-        /// </returns>
-        public static bool CanWrite(Socket socket)
-        {
-            if (socket != null && socket.Connected)
-            {
-                return socket.Poll(-1, SelectMode.SelectWrite);
-            }
-
-            return false;
-        }
-
-        public static Socket Connect(IPEndPoint remoteEndpoint, TimeSpan connectTimeout)
-        {
-            var socket = new Socket(remoteEndpoint.AddressFamily, SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
-            ConnectCore(socket, remoteEndpoint, connectTimeout, ownsSocket: true);
-            return socket;
-        }
-
         public static void Connect(Socket socket, IPEndPoint remoteEndpoint, TimeSpan connectTimeout)
         {
-            ConnectCore(socket, remoteEndpoint, connectTimeout, ownsSocket: false);
-        }
-
-        public static async Task ConnectAsync(Socket socket, IPEndPoint remoteEndpoint, CancellationToken cancellationToken)
-        {
-            await socket.ConnectAsync(remoteEndpoint, cancellationToken).ConfigureAwait(false);
-        }
-
-        private static void ConnectCore(Socket socket, IPEndPoint remoteEndpoint, TimeSpan connectTimeout, bool ownsSocket)
-        {
-            var connectCompleted = new ManualResetEvent(initialState: false);
-            var args = new SocketAsyncEventArgs
-                {
-                    UserToken = connectCompleted,
-                    RemoteEndPoint = remoteEndpoint
-                };
-            args.Completed += ConnectCompleted;
+            using var connectCompleted = new ManualResetEventSlim(initialState: false);
+            using var args = new SocketAsyncEventArgs
+            {
+                RemoteEndPoint = remoteEndpoint
+            };
+            args.Completed += (_, _) => connectCompleted.Set();
 
             if (socket.ConnectAsync(args))
             {
-                if (!connectCompleted.WaitOne(connectTimeout))
+                if (!connectCompleted.Wait(connectTimeout))
                 {
-                    // avoid ObjectDisposedException in ConnectCompleted
-                    args.Completed -= ConnectCompleted;
-                    if (ownsSocket)
-                    {
-                        // dispose Socket
-                        socket.Dispose();
-                    }
-
-                    // dispose ManualResetEvent
-                    connectCompleted.Dispose();
-
-                    // dispose SocketAsyncEventArgs
-                    args.Dispose();
+                    socket.Dispose();
 
                     throw new SshOperationTimeoutException(string.Format(CultureInfo.InvariantCulture,
                                                                          "Connection failed to establish within {0:F0} milliseconds.",
                                                                          connectTimeout.TotalMilliseconds));
                 }
             }
 
-            // dispose ManualResetEvent
-            connectCompleted.Dispose();
-
             if (args.SocketError != SocketError.Success)
             {
                 var socketError = (int) args.SocketError;
 
-                if (ownsSocket)
-                {
-                    // dispose Socket
-                    socket.Dispose();
-                }
-
-                // dispose SocketAsyncEventArgs
-                args.Dispose();
-
                 throw new SocketException(socketError);
             }
-
-            // dispose SocketAsyncEventArgs
-            args.Dispose();
-        }
-
-        public static void ClearReadBuffer(Socket socket)
-        {
-            var timeout = TimeSpan.FromMilliseconds(500);
-            var buffer = new byte[256];
-            int bytesReceived;
-
-            do
-            {
-                bytesReceived = ReadPartial(socket, buffer, 0, buffer.Length, timeout);
-            }
-            while (bytesReceived > 0);
-        }
-
-        public static int ReadPartial(Socket socket, byte[] buffer, int offset, int size, TimeSpan timeout)
-        {
-            socket.ReceiveTimeout = timeout.AsTimeout(nameof(timeout));
-
-            try
-            {
-                return socket.Receive(buffer, offset, size, SocketFlags.None);
-            }
-            catch (SocketException ex)
-            {
-                if (ex.SocketErrorCode == SocketError.TimedOut)
-                {
-                    throw new SshOperationTimeoutException(string.Format(CultureInfo.InvariantCulture,
-                                                                         "Socket read operation has timed out after {0:F0} milliseconds.",
-                                                                         timeout.TotalMilliseconds));
-                }
-
-                throw;
-            }
         }
 
         public static void ReadContinuous(Socket socket, byte[] buffer, int offset, int size, Action<byte[], int, int> processReceivedBytesAction)
@@ -206,41 +101,6 @@ public static int ReadByte(Socket socket, TimeSpan timeout)
             return buffer[0];
         }
 
-        /// <summary>
-        /// Sends a byte using the specified <see cref="Socket"/>.
-        /// </summary>
-        /// <param name="socket">The <see cref="Socket"/> to write to.</param>
-        /// <param name="value">The value to send.</param>
-        /// <exception cref="SocketException">The write failed.</exception>
-        public static void SendByte(Socket socket, byte value)
-        {
-            var buffer = new[] { value };
-            _ = socket.Send(buffer);
-        }
-
-        /// <summary>
-        /// Receives data from a bound <see cref="Socket"/>.
-        /// </summary>
-        /// <param name="socket">The <see cref="Socket"/> to read from.</param>
-        /// <param name="size">The number of bytes to receive.</param>
-        /// <param name="timeout">Specifies the amount of time after which the call will time out.</param>
-        /// <returns>
-        /// The bytes received.
-        /// </returns>
-        /// <remarks>
-        /// If no data is available for reading, the <see cref="Read(Socket, int, TimeSpan)"/> method will
-        /// block until data is available or the time-out value is exceeded. If the time-out value is exceeded, the
-        /// <see cref="Read(Socket, int, TimeSpan)"/> call will throw a <see cref="SshOperationTimeoutException"/>.
-        ///  If you are in non-blocking mode, and there is no data available in the in the protocol stack buffer, the
-        /// <see cref="Read(Socket, int, TimeSpan)"/> method will complete immediately and throw a <see cref="SocketException"/>.
-        /// </remarks>
-        public static byte[] Read(Socket socket, int size, TimeSpan timeout)
-        {
-            var buffer = new byte[size];
-            _ = Read(socket, buffer, 0, size, timeout);
-            return buffer;
-        }
-
         /// <summary>
         /// Receives data from a bound <see cref="Socket"/> into a receive buffer.
         /// </summary>
@@ -258,10 +118,6 @@ public static byte[] Read(Socket socket, int size, TimeSpan timeout)
         /// block until data is available or the time-out value is exceeded. If the time-out value is exceeded, the
         /// <see cref="Read(Socket, byte[], int, int, TimeSpan)"/> call will throw a <see cref="SshOperationTimeoutException"/>.
         /// </para>
-        /// <para>
-        /// If you are in non-blocking mode, and there is no data available in the in the protocol stack buffer, the
-        /// <see cref="Read(Socket, byte[], int, int, TimeSpan)"/> method will complete immediately and throw a <see cref="SocketException"/>.
-        /// </para>
         /// </remarks>
         public static int Read(Socket socket, byte[] buffer, int offset, int size, TimeSpan readTimeout)
         {
@@ -301,10 +157,5 @@ public static ValueTask<int> ReadAsync(Socket socket, byte[] buffer, Cancellatio
             return socket.ReceiveAsync(new ArraySegment<byte>(buffer, 0, buffer.Length), SocketFlags.None, cancellationToken);
         }
 #endif
-        private static void ConnectCompleted(object sender, SocketAsyncEventArgs e)
-        {
-            var eventWaitHandle = (ManualResetEvent) e.UserToken;
-            _ = eventWaitHandle?.Set();
-        }
     }
 }

src/Renci.SshNet/Channels/ChannelForwardedTcpip.cs

@@ -3,6 +3,7 @@
 using System.Net.Sockets;
 using Renci.SshNet.Abstractions;
 using Renci.SshNet.Common;
+using Renci.SshNet.Connection;
 using Renci.SshNet.Messages.Connection;
 
 namespace Renci.SshNet.Channels
@@ -13,20 +14,23 @@ namespace Renci.SshNet.Channels
     internal sealed class ChannelForwardedTcpip : ServerChannel, IChannelForwardedTcpip
     {
         private readonly object _socketShutdownAndCloseLock = new object();
+        private readonly ISocketFactory _socketFactory;
         private Socket _socket;
         private IForwardedPort _forwardedPort;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="ChannelForwardedTcpip"/> class.
         /// </summary>
         /// <param name="session">The session.</param>
+        /// <param name="socketFactory">The socket factory.</param>
         /// <param name="localChannelNumber">The local channel number.</param>
         /// <param name="localWindowSize">Size of the window.</param>
         /// <param name="localPacketSize">Size of the packet.</param>
         /// <param name="remoteChannelNumber">The remote channel number.</param>
         /// <param name="remoteWindowSize">The window size of the remote party.</param>
         /// <param name="remotePacketSize">The maximum size of a data packet that we can send to the remote party.</param>
         internal ChannelForwardedTcpip(ISession session,
+                                       ISocketFactory socketFactory,
                                        uint localChannelNumber,
                                        uint localWindowSize,
                                        uint localPacketSize,
@@ -41,6 +45,7 @@ internal ChannelForwardedTcpip(ISession session,
                    remoteWindowSize,
                    remotePacketSize)
         {
+            _socketFactory = socketFactory;
         }
 
         /// <summary>
@@ -72,7 +77,9 @@ public void Bind(IPEndPoint remoteEndpoint, IForwardedPort forwardedPort)
             // Try to connect to the socket
             try
             {
-                _socket = SocketAbstraction.Connect(remoteEndpoint, ConnectionInfo.Timeout);
+                _socket = _socketFactory.Create(remoteEndpoint.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
+
+                SocketAbstraction.Connect(_socket, remoteEndpoint, ConnectionInfo.Timeout);
 
                 // Send channel open confirmation message
                 SendMessage(new ChannelOpenConfirmationMessage(RemoteChannelNumber, LocalWindowSize, LocalPacketSize, LocalChannelNumber));

src/Renci.SshNet/Common/Extensions.cs

@@ -5,7 +5,7 @@
 using System.Net;
 using System.Net.Sockets;
 using System.Text;
-using Renci.SshNet.Abstractions;
+
 using Renci.SshNet.Messages;
 
 namespace Renci.SshNet.Common
@@ -336,22 +336,17 @@ public static byte[] Concat(this byte[] first, byte[] second)
 
         internal static bool CanRead(this Socket socket)
         {
-            return SocketAbstraction.CanRead(socket);
+            return socket.Connected && socket.Poll(-1, SelectMode.SelectRead) && socket.Available > 0;
         }
 
         internal static bool CanWrite(this Socket socket)
         {
-            return SocketAbstraction.CanWrite(socket);
+            return socket is not null && socket.Connected && socket.Poll(-1, SelectMode.SelectWrite);
         }
 
         internal static bool IsConnected(this Socket socket)
         {
-            if (socket is null)
-            {
-                return false;
-            }
-
-            return socket.Connected;
+            return socket is not null && socket.Connected;
         }
     }
 }

src/Renci.SshNet/Connection/ConnectorBase.cs

@@ -86,7 +86,7 @@ protected async Task<Socket> SocketConnectAsync(string host, int port, Cancellat
             var socket = SocketFactory.Create(ep.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
             try
             {
-                await SocketAbstraction.ConnectAsync(socket, ep, cancellationToken).ConfigureAwait(false);
+                await socket.ConnectAsync(ep, cancellationToken).ConfigureAwait(false);
 
                 const int socketBufferSize = 2 * Session.MaximumSshPacketSize;
                 socket.SendBufferSize = socketBufferSize;

src/Renci.SshNet/Connection/Socks5Connector.cs

@@ -63,7 +63,8 @@ protected override void HandleProxyConnect(IConnectionInfo connectionInfo, Socke
                     _ = socket.Send(authenticationRequest);
 
                     // Read authentication result
-                    var authenticationResult = SocketAbstraction.Read(socket, 2, connectionInfo.Timeout);
+                    var authenticationResult = new byte[2];
+                    _ = SocketAbstraction.Read(socket, authenticationResult, 0, authenticationResult.Length, connectionInfo.Timeout);
 
                     if (authenticationResult[0] != 0x01)
                     {

src/Renci.SshNet/ForwardedPortDynamic.cs

@@ -503,18 +503,18 @@ private bool HandleSocks4(Socket socket, IChannelDirectTcpip channel, TimeSpan t
 
             channel.Open(host, port, this, socket);
 
-            SocketAbstraction.SendByte(socket, 0x00);
+            _ = socket.Send([0x00]);
 
             if (channel.IsOpen)
             {
-                SocketAbstraction.SendByte(socket, 0x5a);
+                _ = socket.Send([0x5a]);
                 _ = socket.Send(portBuffer);
                 _ = socket.Send(ipBuffer);
                 return true;
             }
 
             // signal that request was rejected or failed
-            SocketAbstraction.SendByte(socket, 0x5b);
+            _ = socket.Send([0x5b]);
             return false;
         }
 

src/Renci.SshNet/Session.cs

@@ -2232,6 +2232,7 @@ IChannelForwardedTcpip ISession.CreateChannelForwardedTcpip(uint remoteChannelNu
                                                                     uint remoteChannelDataPacketSize)
         {
             return new ChannelForwardedTcpip(this,
+                                             _socketFactory,
                                              NextChannelNumber,
                                              InitialLocalWindowSize,
                                              LocalChannelDataPacketSize,