From 3ad2451ab07ddbc1fa7ae4912e3f936a94505e90 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Carle <29762210+jscarle@users.noreply.github.com> Date: Tue, 13 Feb 2024 16:41:31 -0500 Subject: [PATCH 1/8] Added guard clauses to various timeouts to ensure they don't exceed an Int32 in milliseconds. --- .../Abstractions/SocketAbstraction.cs | 4 +- src/Renci.SshNet/BaseClient.cs | 2 + src/Renci.SshNet/Common/TimeSpanExtensions.cs | 54 +++++++++++++++++++ src/Renci.SshNet/ConnectionInfo.cs | 30 ++++++++++- src/Renci.SshNet/ForwardedPort.cs | 2 + src/Renci.SshNet/ForwardedPortDynamic.cs | 2 + src/Renci.SshNet/ForwardedPortLocal.cs | 2 + src/Renci.SshNet/ForwardedPortRemote.cs | 2 + src/Renci.SshNet/NetConfClient.cs | 8 +-- src/Renci.SshNet/ScpClient.cs | 15 +++++- src/Renci.SshNet/Sftp/SftpFileStream.cs | 15 +++++- src/Renci.SshNet/SftpClient.cs | 8 +-- src/Renci.SshNet/SshCommand.cs | 15 +++++- .../Classes/NetConfClientTest.cs | 4 ++ .../Classes/SftpClientTest.cs | 4 ++ 15 files changed, 146 insertions(+), 21 deletions(-) create mode 100644 src/Renci.SshNet/Common/TimeSpanExtensions.cs diff --git a/src/Renci.SshNet/Abstractions/SocketAbstraction.cs b/src/Renci.SshNet/Abstractions/SocketAbstraction.cs index 24dceb3d0..503116af2 100644 --- a/src/Renci.SshNet/Abstractions/SocketAbstraction.cs +++ b/src/Renci.SshNet/Abstractions/SocketAbstraction.cs @@ -129,7 +129,7 @@ public static void ClearReadBuffer(Socket socket) public static int ReadPartial(Socket socket, byte[] buffer, int offset, int size, TimeSpan timeout) { - socket.ReceiveTimeout = (int) timeout.TotalMilliseconds; + socket.ReceiveTimeout = timeout.AsTimeout(); try { @@ -274,7 +274,7 @@ public static int Read(Socket socket, byte[] buffer, int offset, int size, TimeS var totalBytesRead = 0; var totalBytesToRead = size; - socket.ReceiveTimeout = (int) readTimeout.TotalMilliseconds; + socket.ReceiveTimeout = readTimeout.AsTimeout(); do { diff --git a/src/Renci.SshNet/BaseClient.cs b/src/Renci.SshNet/BaseClient.cs index 56d2145cc..836995f90 100644 --- a/src/Renci.SshNet/BaseClient.cs +++ b/src/Renci.SshNet/BaseClient.cs @@ -101,6 +101,8 @@ public TimeSpan KeepAliveInterval { CheckDisposed(); + value.EnsureValidTimeout(); + if (value == _keepAliveInterval) { return; diff --git a/src/Renci.SshNet/Common/TimeSpanExtensions.cs b/src/Renci.SshNet/Common/TimeSpanExtensions.cs new file mode 100644 index 000000000..3420badb7 --- /dev/null +++ b/src/Renci.SshNet/Common/TimeSpanExtensions.cs @@ -0,0 +1,54 @@ +using System; +#if NET21_OR_GREATER || NET6_0_OR_GREATER +using System.Runtime.CompilerServices; +#endif + +namespace Renci.SshNet.Common +{ + /// + /// Provides extension methods for . + /// + internal static class TimeSpanExtensions + { + /// + /// Returns the specified as a valid timeout in milliseconds. + /// + /// The to ensure validity. + /// The name of the calling member. + /// + /// Thrown when does not represent a value between -1 and , inclusive. + /// + public static int AsTimeout(this TimeSpan timeSpan, +#if NET21_OR_GREATER || NET6_0_OR_GREATER + [CallerArgumentExpression(nameof(timeSpan))] +#endif + string callerMemberName = "") + { + var timeoutInMilliseconds = timeSpan.TotalMilliseconds; + return timeoutInMilliseconds is < -1d or > int.MaxValue + ? throw new ArgumentOutOfRangeException(callerMemberName, "The timeout must represent a value between -1 and Int32.MaxValue, inclusive.") + : (int) timeoutInMilliseconds; + } + + /// + /// Ensures that the specified represents a valid timeout in milliseconds. + /// + /// The to ensure validity. + /// The name of the calling member. + /// + /// Thrown when does not represent a value between -1 and , inclusive. + /// + public static void EnsureValidTimeout(this TimeSpan timeSpan, +#if NET21_OR_GREATER || NET6_0_OR_GREATER + [CallerArgumentExpression(nameof(timeSpan))] +#endif + string callerMemberName = "") + { + var timeoutInMilliseconds = timeSpan.TotalMilliseconds; + if (timeoutInMilliseconds is < -1d or > int.MaxValue) + { + throw new ArgumentOutOfRangeException(callerMemberName, "The timeout must represent a value between -1 and Int32.MaxValue, inclusive."); + } + } + } +} diff --git a/src/Renci.SshNet/ConnectionInfo.cs b/src/Renci.SshNet/ConnectionInfo.cs index b4cb0fc85..d6c9ea070 100644 --- a/src/Renci.SshNet/ConnectionInfo.cs +++ b/src/Renci.SshNet/ConnectionInfo.cs @@ -43,6 +43,8 @@ public class ConnectionInfo : IConnectionInfoInternal /// 1 second. /// private static readonly TimeSpan DefaultChannelCloseTimeout = TimeSpan.FromSeconds(1); + private TimeSpan _timeout; + private TimeSpan _channelCloseTimeout; /// /// Gets supported key exchange algorithms for this connection. @@ -145,7 +147,19 @@ public class ConnectionInfo : IConnectionInfoInternal /// /// The connection timeout. The default value is 30 seconds. /// - public TimeSpan Timeout { get; set; } + public TimeSpan Timeout + { + get + { + return _timeout; + } + set + { + value.EnsureValidTimeout(); + + _timeout = value; + } + } /// /// Gets or sets the timeout to use when waiting for a server to acknowledge closing a channel. @@ -157,7 +171,19 @@ public class ConnectionInfo : IConnectionInfoInternal /// If a server does not send a SSH_MSG_CHANNEL_CLOSE message before the specified timeout /// elapses, the channel will be closed immediately. /// - public TimeSpan ChannelCloseTimeout { get; set; } + public TimeSpan ChannelCloseTimeout + { + get + { + return _channelCloseTimeout; + } + set + { + value.EnsureValidTimeout(); + + _channelCloseTimeout = value; + } + } /// /// Gets or sets the character encoding. diff --git a/src/Renci.SshNet/ForwardedPort.cs b/src/Renci.SshNet/ForwardedPort.cs index 50157e1c1..426e49ebb 100644 --- a/src/Renci.SshNet/ForwardedPort.cs +++ b/src/Renci.SshNet/ForwardedPort.cs @@ -102,6 +102,8 @@ public void Dispose() /// The maximum amount of time to wait for pending requests to finish processing. protected virtual void StopPort(TimeSpan timeout) { + timeout.EnsureValidTimeout(); + RaiseClosing(); var session = Session; diff --git a/src/Renci.SshNet/ForwardedPortDynamic.cs b/src/Renci.SshNet/ForwardedPortDynamic.cs index 0c6e417a8..eaa9e5013 100644 --- a/src/Renci.SshNet/ForwardedPortDynamic.cs +++ b/src/Renci.SshNet/ForwardedPortDynamic.cs @@ -101,6 +101,8 @@ protected override void StartPort() /// The maximum amount of time to wait for pending requests to finish processing. protected override void StopPort(TimeSpan timeout) { + timeout.EnsureValidTimeout(); + if (!ForwardedPortStatus.ToStopping(ref _status)) { return; diff --git a/src/Renci.SshNet/ForwardedPortLocal.cs b/src/Renci.SshNet/ForwardedPortLocal.cs index fce8f7fd7..c98e9ed0c 100644 --- a/src/Renci.SshNet/ForwardedPortLocal.cs +++ b/src/Renci.SshNet/ForwardedPortLocal.cs @@ -138,6 +138,8 @@ protected override void StartPort() /// The maximum amount of time to wait for pending requests to finish processing. protected override void StopPort(TimeSpan timeout) { + timeout.EnsureValidTimeout(); + if (!ForwardedPortStatus.ToStopping(ref _status)) { return; diff --git a/src/Renci.SshNet/ForwardedPortRemote.cs b/src/Renci.SshNet/ForwardedPortRemote.cs index 10430abc3..1ee2823a0 100644 --- a/src/Renci.SshNet/ForwardedPortRemote.cs +++ b/src/Renci.SshNet/ForwardedPortRemote.cs @@ -188,6 +188,8 @@ protected override void StartPort() /// The maximum amount of time to wait for the port to stop. protected override void StopPort(TimeSpan timeout) { + timeout.EnsureValidTimeout(); + if (!ForwardedPortStatus.ToStopping(ref _status)) { return; diff --git a/src/Renci.SshNet/NetConfClient.cs b/src/Renci.SshNet/NetConfClient.cs index 09cff7609..10eaa122b 100644 --- a/src/Renci.SshNet/NetConfClient.cs +++ b/src/Renci.SshNet/NetConfClient.cs @@ -36,13 +36,7 @@ public TimeSpan OperationTimeout } set { - var timeoutInMilliseconds = value.TotalMilliseconds; - if (timeoutInMilliseconds is < -1d or > int.MaxValue) - { - throw new ArgumentOutOfRangeException(nameof(value), "The timeout must represent a value between -1 and Int32.MaxValue, inclusive."); - } - - _operationTimeout = (int) timeoutInMilliseconds; + _operationTimeout = value.AsTimeout(); } } diff --git a/src/Renci.SshNet/ScpClient.cs b/src/Renci.SshNet/ScpClient.cs index 8121c9702..367287a45 100644 --- a/src/Renci.SshNet/ScpClient.cs +++ b/src/Renci.SshNet/ScpClient.cs @@ -38,6 +38,7 @@ public partial class ScpClient : BaseClient private static readonly Regex TimestampRe = new Regex(@"T(?\d+) 0 (?\d+) 0", RegexOptions.Compiled); private IRemotePathTransformation _remotePathTransformation; + private TimeSpan _operationTimeout; /// /// Gets or sets the operation timeout. @@ -46,7 +47,19 @@ public partial class ScpClient : BaseClient /// The timeout to wait until an operation completes. The default value is negative /// one (-1) milliseconds, which indicates an infinite time-out period. /// - public TimeSpan OperationTimeout { get; set; } + public TimeSpan OperationTimeout + { + get + { + return _operationTimeout; + } + set + { + value.EnsureValidTimeout(); + + _operationTimeout = value; + } + } /// /// Gets or sets the size of the buffer. diff --git a/src/Renci.SshNet/Sftp/SftpFileStream.cs b/src/Renci.SshNet/Sftp/SftpFileStream.cs index 7880ffbbd..808e6b983 100644 --- a/src/Renci.SshNet/Sftp/SftpFileStream.cs +++ b/src/Renci.SshNet/Sftp/SftpFileStream.cs @@ -35,6 +35,7 @@ public class SftpFileStream : Stream private bool _canRead; private bool _canSeek; private bool _canWrite; + private TimeSpan _timeout; /// /// Gets a value indicating whether the current stream supports reading. @@ -176,7 +177,19 @@ public virtual byte[] Handle /// /// The timeout. /// - public TimeSpan Timeout { get; set; } + public TimeSpan Timeout + { + get + { + return _timeout; + } + set + { + _timeout.EnsureValidTimeout(); + + _timeout = value; + } + } private SftpFileStream(ISftpSession session, string path, FileAccess access, int bufferSize, byte[] handle, long position) { diff --git a/src/Renci.SshNet/SftpClient.cs b/src/Renci.SshNet/SftpClient.cs index 01472dbff..9422aa173 100644 --- a/src/Renci.SshNet/SftpClient.cs +++ b/src/Renci.SshNet/SftpClient.cs @@ -59,13 +59,7 @@ public TimeSpan OperationTimeout { CheckDisposed(); - var timeoutInMilliseconds = value.TotalMilliseconds; - if (timeoutInMilliseconds is < -1d or > int.MaxValue) - { - throw new ArgumentOutOfRangeException(nameof(value), "The timeout must represent a value between -1 and Int32.MaxValue, inclusive."); - } - - _operationTimeout = (int) timeoutInMilliseconds; + _operationTimeout = value.AsTimeout(); } } diff --git a/src/Renci.SshNet/SshCommand.cs b/src/Renci.SshNet/SshCommand.cs index 7aa013ad8..249c06509 100644 --- a/src/Renci.SshNet/SshCommand.cs +++ b/src/Renci.SshNet/SshCommand.cs @@ -32,6 +32,7 @@ public class SshCommand : IDisposable private bool _hasError; private bool _isDisposed; private ChannelInputStream _inputStream; + private TimeSpan _commandTimeout; /// /// Gets the command text. @@ -44,7 +45,19 @@ public class SshCommand : IDisposable /// /// The command timeout. /// - public TimeSpan CommandTimeout { get; set; } + public TimeSpan CommandTimeout + { + get + { + return _commandTimeout; + } + set + { + _commandTimeout.EnsureValidTimeout(); + + _commandTimeout = value; + } + } /// /// Gets the command exit status. diff --git a/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs b/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs index d86ca2e37..500f442f1 100644 --- a/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs +++ b/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs @@ -86,7 +86,9 @@ public void OperationTimeout_LessThanLowerLimit() { Assert.IsNull(ex.InnerException); ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue, inclusive.", ex); +#if NET21_OR_GREATER || NET6_0_OR_GREATER Assert.AreEqual("value", ex.ParamName); +#endif } } @@ -105,7 +107,9 @@ public void OperationTimeout_GreaterThanLowerLimit() { Assert.IsNull(ex.InnerException); ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue, inclusive.", ex); +#if NET21_OR_GREATER || NET6_0_OR_GREATER Assert.AreEqual("value", ex.ParamName); +#endif } } } diff --git a/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs b/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs index 25624dcc7..dbf7b41bf 100644 --- a/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs +++ b/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs @@ -89,7 +89,9 @@ public void OperationTimeout_LessThanLowerLimit() { Assert.IsNull(ex.InnerException); ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue, inclusive.", ex); +#if NET21_OR_GREATER || NET6_0_OR_GREATER Assert.AreEqual("value", ex.ParamName); +#endif } } @@ -108,7 +110,9 @@ public void OperationTimeout_GreaterThanLowerLimit() { Assert.IsNull(ex.InnerException); ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue, inclusive.", ex); +#if NET21_OR_GREATER || NET6_0_OR_GREATER Assert.AreEqual("value", ex.ParamName); +#endif } } From 132e3ecf440c0b8081585cc2fd10819ab1a2bd42 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Carle <29762210+jscarle@users.noreply.github.com> Date: Fri, 16 Feb 2024 19:18:32 -0500 Subject: [PATCH 2/8] Fixed guard clauses. --- src/Renci.SshNet/Sftp/SftpFileStream.cs | 2 +- src/Renci.SshNet/SshCommand.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Renci.SshNet/Sftp/SftpFileStream.cs b/src/Renci.SshNet/Sftp/SftpFileStream.cs index 808e6b983..a06e2a6c7 100644 --- a/src/Renci.SshNet/Sftp/SftpFileStream.cs +++ b/src/Renci.SshNet/Sftp/SftpFileStream.cs @@ -185,7 +185,7 @@ public TimeSpan Timeout } set { - _timeout.EnsureValidTimeout(); + value.EnsureValidTimeout(); _timeout = value; } diff --git a/src/Renci.SshNet/SshCommand.cs b/src/Renci.SshNet/SshCommand.cs index 249c06509..6fcee8d11 100644 --- a/src/Renci.SshNet/SshCommand.cs +++ b/src/Renci.SshNet/SshCommand.cs @@ -53,7 +53,7 @@ public TimeSpan CommandTimeout } set { - _commandTimeout.EnsureValidTimeout(); + value.EnsureValidTimeout(); _commandTimeout = value; } From e7afbd49c40b074343de7939a80ed43382131717 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Carle <29762210+jscarle@users.noreply.github.com> Date: Fri, 16 Feb 2024 19:18:56 -0500 Subject: [PATCH 3/8] Updated build tags. --- src/Renci.SshNet/Common/TimeSpanExtensions.cs | 6 +++--- test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs | 4 ++-- test/Renci.SshNet.Tests/Classes/SftpClientTest.cs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Renci.SshNet/Common/TimeSpanExtensions.cs b/src/Renci.SshNet/Common/TimeSpanExtensions.cs index 3420badb7..3c4bab92b 100644 --- a/src/Renci.SshNet/Common/TimeSpanExtensions.cs +++ b/src/Renci.SshNet/Common/TimeSpanExtensions.cs @@ -1,5 +1,5 @@ using System; -#if NET21_OR_GREATER || NET6_0_OR_GREATER +#if NETCOREAPP3_0_OR_GREATER using System.Runtime.CompilerServices; #endif @@ -19,7 +19,7 @@ internal static class TimeSpanExtensions /// Thrown when does not represent a value between -1 and , inclusive. /// public static int AsTimeout(this TimeSpan timeSpan, -#if NET21_OR_GREATER || NET6_0_OR_GREATER +#if NETCOREAPP3_0_OR_GREATER [CallerArgumentExpression(nameof(timeSpan))] #endif string callerMemberName = "") @@ -39,7 +39,7 @@ public static int AsTimeout(this TimeSpan timeSpan, /// Thrown when does not represent a value between -1 and , inclusive. /// public static void EnsureValidTimeout(this TimeSpan timeSpan, -#if NET21_OR_GREATER || NET6_0_OR_GREATER +#if NETCOREAPP3_0_OR_GREATER [CallerArgumentExpression(nameof(timeSpan))] #endif string callerMemberName = "") diff --git a/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs b/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs index 500f442f1..58fc930ff 100644 --- a/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs +++ b/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs @@ -86,7 +86,7 @@ public void OperationTimeout_LessThanLowerLimit() { Assert.IsNull(ex.InnerException); ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue, inclusive.", ex); -#if NET21_OR_GREATER || NET6_0_OR_GREATER +#if NETCOREAPP3_0_OR_GREATER Assert.AreEqual("value", ex.ParamName); #endif } @@ -107,7 +107,7 @@ public void OperationTimeout_GreaterThanLowerLimit() { Assert.IsNull(ex.InnerException); ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue, inclusive.", ex); -#if NET21_OR_GREATER || NET6_0_OR_GREATER +#if NETCOREAPP3_0_OR_GREATER Assert.AreEqual("value", ex.ParamName); #endif } diff --git a/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs b/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs index dbf7b41bf..ea99d827f 100644 --- a/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs +++ b/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs @@ -89,7 +89,7 @@ public void OperationTimeout_LessThanLowerLimit() { Assert.IsNull(ex.InnerException); ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue, inclusive.", ex); -#if NET21_OR_GREATER || NET6_0_OR_GREATER +#if NETCOREAPP3_0_OR_GREATER Assert.AreEqual("value", ex.ParamName); #endif } @@ -110,7 +110,7 @@ public void OperationTimeout_GreaterThanLowerLimit() { Assert.IsNull(ex.InnerException); ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue, inclusive.", ex); -#if NET21_OR_GREATER || NET6_0_OR_GREATER +#if NETCOREAPP3_0_OR_GREATER Assert.AreEqual("value", ex.ParamName); #endif } From 5b1a11169900434a00f208ab1af82af58aa4d927 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Carle <29762210+jscarle@users.noreply.github.com> Date: Fri, 16 Feb 2024 19:21:48 -0500 Subject: [PATCH 4/8] Added guard clauses to various timeouts to ensure they don't exceed an Int32 in milliseconds. --- Directory.Build.props | 1 + .../Abstractions/SocketAbstraction.cs | 4 +- src/Renci.SshNet/BaseClient.cs | 2 + src/Renci.SshNet/Common/TimeSpanExtensions.cs | 46 +++++++++++++++++++ src/Renci.SshNet/ConnectionInfo.cs | 30 +++++++++++- src/Renci.SshNet/ForwardedPort.cs | 2 + src/Renci.SshNet/ForwardedPortDynamic.cs | 2 + src/Renci.SshNet/ForwardedPortLocal.cs | 2 + src/Renci.SshNet/ForwardedPortRemote.cs | 2 + src/Renci.SshNet/NetConfClient.cs | 8 +--- src/Renci.SshNet/ScpClient.cs | 15 +++++- src/Renci.SshNet/Sftp/SftpFileStream.cs | 15 +++++- src/Renci.SshNet/SftpClient.cs | 8 +--- src/Renci.SshNet/SshCommand.cs | 15 +++++- .../Classes/NetConfClientTest.cs | 4 ++ .../Classes/SftpClientTest.cs | 4 ++ 16 files changed, 139 insertions(+), 21 deletions(-) create mode 100644 src/Renci.SshNet/Common/TimeSpanExtensions.cs diff --git a/Directory.Build.props b/Directory.Build.props index 65a489ddc..1589eab99 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -11,6 +11,7 @@ latest 9999 true + IDE0005