From d9b9fba78edcbdb5cfd612d76a8a51044d4d701c Mon Sep 17 00:00:00 2001 From: Nikola Milekic Date: Tue, 7 Oct 2025 13:50:06 +0200 Subject: [PATCH 1/2] Avoid rounding issues when checking Timeout values (#1700) AsTimeout is called from the SshCommand constructor with Timeout.InfiniteTimeSpan. In this scenario the range check should never fail, but unfortunately it does in certain scenarios, due to a runtime or compiler bug (as soon as optimizations are turned off the issue miraculously disappears). Closes #1700 --- src/Renci.SshNet/Common/TimeSpanExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Renci.SshNet/Common/TimeSpanExtensions.cs b/src/Renci.SshNet/Common/TimeSpanExtensions.cs index 8f81cdd8c..ef9865c90 100644 --- a/src/Renci.SshNet/Common/TimeSpanExtensions.cs +++ b/src/Renci.SshNet/Common/TimeSpanExtensions.cs @@ -19,7 +19,7 @@ internal static class TimeSpanExtensions /// public static int AsTimeout(this TimeSpan timeSpan, [CallerArgumentExpression(nameof(timeSpan))] string? paramName = null) { - var timeoutInMilliseconds = timeSpan.TotalMilliseconds; + var timeoutInMilliseconds = Math.Round(timeSpan.TotalMilliseconds); return timeoutInMilliseconds is < -1d or > int.MaxValue ? throw new ArgumentOutOfRangeException(paramName, "The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.") : (int)timeoutInMilliseconds; From 9584bb7b33eaf1837a2bed24dce0ae6998e32491 Mon Sep 17 00:00:00 2001 From: Robert Hague Date: Wed, 8 Oct 2025 10:12:02 +0200 Subject: [PATCH 2/2] fix tests --- src/Renci.SshNet/Common/TimeSpanExtensions.cs | 6 +- .../Classes/Common/TimeSpanExtensionsTest.cs | 63 +++++-------------- .../Classes/ConnectionInfoTest.cs | 52 +++------------ .../Classes/NetConfClientTest.cs | 26 ++------ .../Classes/SftpClientTest.cs | 26 ++------ 5 files changed, 33 insertions(+), 140 deletions(-) diff --git a/src/Renci.SshNet/Common/TimeSpanExtensions.cs b/src/Renci.SshNet/Common/TimeSpanExtensions.cs index ef9865c90..781d1f4dd 100644 --- a/src/Renci.SshNet/Common/TimeSpanExtensions.cs +++ b/src/Renci.SshNet/Common/TimeSpanExtensions.cs @@ -19,9 +19,9 @@ internal static class TimeSpanExtensions /// public static int AsTimeout(this TimeSpan timeSpan, [CallerArgumentExpression(nameof(timeSpan))] string? paramName = null) { - var timeoutInMilliseconds = Math.Round(timeSpan.TotalMilliseconds); - return timeoutInMilliseconds is < -1d or > int.MaxValue - ? throw new ArgumentOutOfRangeException(paramName, "The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.") + var timeoutInMilliseconds = (long)timeSpan.TotalMilliseconds; + return timeoutInMilliseconds is < -1 or > int.MaxValue + ? throw new ArgumentOutOfRangeException(paramName, timeSpan, "The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.") : (int)timeoutInMilliseconds; } diff --git a/test/Renci.SshNet.Tests/Classes/Common/TimeSpanExtensionsTest.cs b/test/Renci.SshNet.Tests/Classes/Common/TimeSpanExtensionsTest.cs index 27cd774ac..e5d5d1249 100644 --- a/test/Renci.SshNet.Tests/Classes/Common/TimeSpanExtensionsTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Common/TimeSpanExtensionsTest.cs @@ -3,7 +3,6 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Renci.SshNet.Common; -using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Common { @@ -21,34 +20,17 @@ public void AsTimeout_ValidTimeSpan_ReturnsExpectedMilliseconds() } [TestMethod] - public void AsTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException() + [DataRow(-2)] + [DataRow((double)int.MaxValue + 1)] + public void AsTimeout_InvalidTimeSpan_ThrowsArgumentOutOfRangeException(double milliseconds) { - var timeSpan = TimeSpan.FromSeconds(-1); - Assert.ThrowsExactly(() => timeSpan.AsTimeout()); - } + var timeSpan = TimeSpan.FromMilliseconds(milliseconds); - [TestMethod] - public void AsTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException() - { - var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1); - Assert.ThrowsExactly(() => timeSpan.AsTimeout()); - } + var ex = Assert.ThrowsExactly(() => timeSpan.AsTimeout()); - [TestMethod] - public void AsTimeout_ArgumentOutOfRangeException_HasCorrectInformation() - { - var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1); - try - { + Assert.Contains("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex.Message, StringComparison.Ordinal); - timeSpan.AsTimeout(); - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); - Assert.AreEqual(nameof(timeSpan), ex.ParamName); - } + Assert.AreEqual(nameof(timeSpan), ex.ParamName); } [TestMethod] @@ -60,34 +42,17 @@ public void EnsureValidTimeout_ValidTimeSpan_DoesNotThrow() } [TestMethod] - public void EnsureValidTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException() + [DataRow(-2)] + [DataRow((double)int.MaxValue + 1)] + public void EnsureValidTimeout_InvalidTimeSpan_ThrowsArgumentOutOfRangeException(double milliseconds) { - var timeSpan = TimeSpan.FromSeconds(-1); - Assert.ThrowsExactly(() => timeSpan.EnsureValidTimeout()); - } + var timeSpan = TimeSpan.FromMilliseconds(milliseconds); - [TestMethod] - public void EnsureValidTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException() - { - var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1); - Assert.ThrowsExactly(() => timeSpan.EnsureValidTimeout()); - } + var ex = Assert.ThrowsExactly(() => timeSpan.EnsureValidTimeout()); - [TestMethod] - public void EnsureValidTimeout_ArgumentOutOfRangeException_HasCorrectInformation() - { - var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1); - try - { + Assert.Contains("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex.Message, StringComparison.Ordinal); - timeSpan.EnsureValidTimeout(); - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); - Assert.AreEqual(nameof(timeSpan), ex.ParamName); - } + Assert.AreEqual(nameof(timeSpan), ex.ParamName); } } } diff --git a/test/Renci.SshNet.Tests/Classes/ConnectionInfoTest.cs b/test/Renci.SshNet.Tests/Classes/ConnectionInfoTest.cs index 407eb5a3e..c44f1ed92 100644 --- a/test/Renci.SshNet.Tests/Classes/ConnectionInfoTest.cs +++ b/test/Renci.SshNet.Tests/Classes/ConnectionInfoTest.cs @@ -286,35 +286,17 @@ public void Test_ConnectionInfo_Timeout_Valid() Resources.HOST, int.Parse(Resources.PORT), Resources.USERNAME, Resources.PASSWORD, new KeyboardInteractiveAuthenticationMethod(Resources.USERNAME)); - try - { - connectionInfo.Timeout = TimeSpan.FromMilliseconds(-2); - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); + var ex = Assert.Throws(() => connectionInfo.Timeout = TimeSpan.FromMilliseconds(-2)); + Assert.AreEqual("Timeout", ex.ParamName); - Assert.AreEqual("Timeout", ex.ParamName); - } + ex = Assert.Throws(() => connectionInfo.Timeout = TimeSpan.FromMilliseconds((double)int.MaxValue + 1)); + Assert.AreEqual("Timeout", ex.ParamName); connectionInfo.Timeout = TimeSpan.FromMilliseconds(-1); Assert.AreEqual(connectionInfo.Timeout, TimeSpan.FromMilliseconds(-1)); connectionInfo.Timeout = TimeSpan.FromMilliseconds(int.MaxValue); Assert.AreEqual(connectionInfo.Timeout, TimeSpan.FromMilliseconds(int.MaxValue)); - - try - { - connectionInfo.Timeout = TimeSpan.FromMilliseconds((double)int.MaxValue + 1); - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); - - Assert.AreEqual("Timeout", ex.ParamName); - } } [TestMethod] @@ -325,35 +307,17 @@ public void Test_ConnectionInfo_ChannelCloseTimeout_Valid() Resources.HOST, int.Parse(Resources.PORT), Resources.USERNAME, Resources.PASSWORD, new KeyboardInteractiveAuthenticationMethod(Resources.USERNAME)); - try - { - connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds(-2); - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); + var ex = Assert.Throws(() => connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds(-2)); + Assert.AreEqual("ChannelCloseTimeout", ex.ParamName); - Assert.AreEqual("ChannelCloseTimeout", ex.ParamName); - } + ex = Assert.Throws(() => connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds((double)int.MaxValue + 1)); + Assert.AreEqual("ChannelCloseTimeout", ex.ParamName); connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds(-1); Assert.AreEqual(connectionInfo.ChannelCloseTimeout, TimeSpan.FromMilliseconds(-1)); connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds(int.MaxValue); Assert.AreEqual(connectionInfo.ChannelCloseTimeout, TimeSpan.FromMilliseconds(int.MaxValue)); - - try - { - connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds((double)int.MaxValue + 1); - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); - - Assert.AreEqual("ChannelCloseTimeout", ex.ParamName); - } } [TestMethod] diff --git a/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs b/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs index 4917442a6..69d492a13 100644 --- a/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs +++ b/test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs @@ -80,17 +80,8 @@ public void OperationTimeout_LessThanLowerLimit() var connectionInfo = new PasswordConnectionInfo("host", 22, "admin", "pwd"); var target = new NetConfClient(connectionInfo); - try - { - target.OperationTimeout = operationTimeout; - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); - - Assert.AreEqual("OperationTimeout", ex.ParamName); - } + var ex = Assert.Throws(() => target.OperationTimeout = operationTimeout); + Assert.AreEqual("OperationTimeout", ex.ParamName); } [TestMethod] @@ -100,17 +91,8 @@ public void OperationTimeout_GreaterThanLowerLimit() var connectionInfo = new PasswordConnectionInfo("host", 22, "admin", "pwd"); var target = new NetConfClient(connectionInfo); - try - { - target.OperationTimeout = operationTimeout; - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); - - Assert.AreEqual("OperationTimeout", ex.ParamName); - } + var ex = Assert.Throws(() => target.OperationTimeout = operationTimeout); + Assert.AreEqual("OperationTimeout", ex.ParamName); } } } diff --git a/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs b/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs index 21c2f2758..2b632d8f8 100644 --- a/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs +++ b/test/Renci.SshNet.Tests/Classes/SftpClientTest.cs @@ -83,17 +83,8 @@ public void OperationTimeout_LessThanLowerLimit() var connectionInfo = new PasswordConnectionInfo("host", 22, "admin", "pwd"); var target = new SftpClient(connectionInfo); - try - { - target.OperationTimeout = operationTimeout; - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); - - Assert.AreEqual("OperationTimeout", ex.ParamName); - } + var ex = Assert.Throws(() => target.OperationTimeout = operationTimeout); + Assert.AreEqual("OperationTimeout", ex.ParamName); } [TestMethod] @@ -103,17 +94,8 @@ public void OperationTimeout_GreaterThanLowerLimit() var connectionInfo = new PasswordConnectionInfo("host", 22, "admin", "pwd"); var target = new SftpClient(connectionInfo); - try - { - target.OperationTimeout = operationTimeout; - } - catch (ArgumentOutOfRangeException ex) - { - Assert.IsNull(ex.InnerException); - ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex); - - Assert.AreEqual("OperationTimeout", ex.ParamName); - } + var ex = Assert.Throws(() => target.OperationTimeout = operationTimeout); + Assert.AreEqual("OperationTimeout", ex.ParamName); } } }