Skip to content

Commit c5c6f28

Browse files
nikolamilekicRob-Hague
nikolamilekic
and
Rob-Hague
authored
Avoid rounding issues when checking Timeout values (#1700) (#1712)
* 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 * fix tests --------- Co-authored-by: Robert Hague <[email protected]>
1 parent ebdcb3e commit c5c6f28

File tree

5 files changed

+33
-140
lines changed

5 files changed

+33
-140
lines changed

‎src/Renci.SshNet/Common/TimeSpanExtensions.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ internal static class TimeSpanExtensions
1919
/// </exception>
2020
public static int AsTimeout(this TimeSpan timeSpan, [CallerArgumentExpression(nameof(timeSpan))] string? paramName = null)
2121
{
22-
var timeoutInMilliseconds = timeSpan.TotalMilliseconds;
23-
return timeoutInMilliseconds is < -1d or > int.MaxValue
24-
? throw new ArgumentOutOfRangeException(paramName, "The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.")
22+
var timeoutInMilliseconds = (long)timeSpan.TotalMilliseconds;
23+
return timeoutInMilliseconds is < -1 or > int.MaxValue
24+
? throw new ArgumentOutOfRangeException(paramName, timeSpan, "The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.")
2525
: (int)timeoutInMilliseconds;
2626
}
2727

‎test/Renci.SshNet.Tests/Classes/Common/TimeSpanExtensionsTest.cs‎

Lines changed: 14 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
using Microsoft.VisualStudio.TestTools.UnitTesting;
44

55
using Renci.SshNet.Common;
6-
using Renci.SshNet.Tests.Common;
76

87
namespace Renci.SshNet.Tests.Classes.Common
98
{
@@ -21,34 +20,17 @@ public void AsTimeout_ValidTimeSpan_ReturnsExpectedMilliseconds()
2120
}
2221

2322
[TestMethod]
24-
public void AsTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException()
23+
[DataRow(-2)]
24+
[DataRow((double)int.MaxValue + 1)]
25+
public void AsTimeout_InvalidTimeSpan_ThrowsArgumentOutOfRangeException(double milliseconds)
2526
{
26-
var timeSpan = TimeSpan.FromSeconds(-1);
27-
Assert.ThrowsExactly<ArgumentOutOfRangeException>(() => timeSpan.AsTimeout());
28-
}
27+
var timeSpan = TimeSpan.FromMilliseconds(milliseconds);
2928

30-
[TestMethod]
31-
public void AsTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException()
32-
{
33-
var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
34-
Assert.ThrowsExactly<ArgumentOutOfRangeException>(() => timeSpan.AsTimeout());
35-
}
29+
var ex = Assert.ThrowsExactly<ArgumentOutOfRangeException>(() => timeSpan.AsTimeout());
3630

37-
[TestMethod]
38-
public void AsTimeout_ArgumentOutOfRangeException_HasCorrectInformation()
39-
{
40-
var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
41-
try
42-
{
31+
Assert.Contains("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex.Message, StringComparison.Ordinal);
4332

44-
timeSpan.AsTimeout();
45-
}
46-
catch (ArgumentOutOfRangeException ex)
47-
{
48-
Assert.IsNull(ex.InnerException);
49-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
50-
Assert.AreEqual(nameof(timeSpan), ex.ParamName);
51-
}
33+
Assert.AreEqual(nameof(timeSpan), ex.ParamName);
5234
}
5335

5436
[TestMethod]
@@ -60,34 +42,17 @@ public void EnsureValidTimeout_ValidTimeSpan_DoesNotThrow()
6042
}
6143

6244
[TestMethod]
63-
public void EnsureValidTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException()
45+
[DataRow(-2)]
46+
[DataRow((double)int.MaxValue + 1)]
47+
public void EnsureValidTimeout_InvalidTimeSpan_ThrowsArgumentOutOfRangeException(double milliseconds)
6448
{
65-
var timeSpan = TimeSpan.FromSeconds(-1);
66-
Assert.ThrowsExactly<ArgumentOutOfRangeException>(() => timeSpan.EnsureValidTimeout());
67-
}
49+
var timeSpan = TimeSpan.FromMilliseconds(milliseconds);
6850

69-
[TestMethod]
70-
public void EnsureValidTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException()
71-
{
72-
var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
73-
Assert.ThrowsExactly<ArgumentOutOfRangeException>(() => timeSpan.EnsureValidTimeout());
74-
}
51+
var ex = Assert.ThrowsExactly<ArgumentOutOfRangeException>(() => timeSpan.EnsureValidTimeout());
7552

76-
[TestMethod]
77-
public void EnsureValidTimeout_ArgumentOutOfRangeException_HasCorrectInformation()
78-
{
79-
var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
80-
try
81-
{
53+
Assert.Contains("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex.Message, StringComparison.Ordinal);
8254

83-
timeSpan.EnsureValidTimeout();
84-
}
85-
catch (ArgumentOutOfRangeException ex)
86-
{
87-
Assert.IsNull(ex.InnerException);
88-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
89-
Assert.AreEqual(nameof(timeSpan), ex.ParamName);
90-
}
55+
Assert.AreEqual(nameof(timeSpan), ex.ParamName);
9156
}
9257
}
9358
}

‎test/Renci.SshNet.Tests/Classes/ConnectionInfoTest.cs‎

Lines changed: 8 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -286,35 +286,17 @@ public void Test_ConnectionInfo_Timeout_Valid()
286286
Resources.HOST, int.Parse(Resources.PORT), Resources.USERNAME,
287287
Resources.PASSWORD, new KeyboardInteractiveAuthenticationMethod(Resources.USERNAME));
288288

289-
try
290-
{
291-
connectionInfo.Timeout = TimeSpan.FromMilliseconds(-2);
292-
}
293-
catch (ArgumentOutOfRangeException ex)
294-
{
295-
Assert.IsNull(ex.InnerException);
296-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
289+
var ex = Assert.Throws<ArgumentOutOfRangeException>(() => connectionInfo.Timeout = TimeSpan.FromMilliseconds(-2));
290+
Assert.AreEqual("Timeout", ex.ParamName);
297291

298-
Assert.AreEqual("Timeout", ex.ParamName);
299-
}
292+
ex = Assert.Throws<ArgumentOutOfRangeException>(() => connectionInfo.Timeout = TimeSpan.FromMilliseconds((double)int.MaxValue + 1));
293+
Assert.AreEqual("Timeout", ex.ParamName);
300294

301295
connectionInfo.Timeout = TimeSpan.FromMilliseconds(-1);
302296
Assert.AreEqual(connectionInfo.Timeout, TimeSpan.FromMilliseconds(-1));
303297

304298
connectionInfo.Timeout = TimeSpan.FromMilliseconds(int.MaxValue);
305299
Assert.AreEqual(connectionInfo.Timeout, TimeSpan.FromMilliseconds(int.MaxValue));
306-
307-
try
308-
{
309-
connectionInfo.Timeout = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
310-
}
311-
catch (ArgumentOutOfRangeException ex)
312-
{
313-
Assert.IsNull(ex.InnerException);
314-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
315-
316-
Assert.AreEqual("Timeout", ex.ParamName);
317-
}
318300
}
319301

320302
[TestMethod]
@@ -325,35 +307,17 @@ public void Test_ConnectionInfo_ChannelCloseTimeout_Valid()
325307
Resources.HOST, int.Parse(Resources.PORT), Resources.USERNAME,
326308
Resources.PASSWORD, new KeyboardInteractiveAuthenticationMethod(Resources.USERNAME));
327309

328-
try
329-
{
330-
connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds(-2);
331-
}
332-
catch (ArgumentOutOfRangeException ex)
333-
{
334-
Assert.IsNull(ex.InnerException);
335-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
310+
var ex = Assert.Throws<ArgumentOutOfRangeException>(() => connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds(-2));
311+
Assert.AreEqual("ChannelCloseTimeout", ex.ParamName);
336312

337-
Assert.AreEqual("ChannelCloseTimeout", ex.ParamName);
338-
}
313+
ex = Assert.Throws<ArgumentOutOfRangeException>(() => connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds((double)int.MaxValue + 1));
314+
Assert.AreEqual("ChannelCloseTimeout", ex.ParamName);
339315

340316
connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds(-1);
341317
Assert.AreEqual(connectionInfo.ChannelCloseTimeout, TimeSpan.FromMilliseconds(-1));
342318

343319
connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds(int.MaxValue);
344320
Assert.AreEqual(connectionInfo.ChannelCloseTimeout, TimeSpan.FromMilliseconds(int.MaxValue));
345-
346-
try
347-
{
348-
connectionInfo.ChannelCloseTimeout = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
349-
}
350-
catch (ArgumentOutOfRangeException ex)
351-
{
352-
Assert.IsNull(ex.InnerException);
353-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
354-
355-
Assert.AreEqual("ChannelCloseTimeout", ex.ParamName);
356-
}
357321
}
358322

359323
[TestMethod]

‎test/Renci.SshNet.Tests/Classes/NetConfClientTest.cs‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,8 @@ public void OperationTimeout_LessThanLowerLimit()
8080
var connectionInfo = new PasswordConnectionInfo("host", 22, "admin", "pwd");
8181
var target = new NetConfClient(connectionInfo);
8282

83-
try
84-
{
85-
target.OperationTimeout = operationTimeout;
86-
}
87-
catch (ArgumentOutOfRangeException ex)
88-
{
89-
Assert.IsNull(ex.InnerException);
90-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
91-
92-
Assert.AreEqual("OperationTimeout", ex.ParamName);
93-
}
83+
var ex = Assert.Throws<ArgumentOutOfRangeException>(() => target.OperationTimeout = operationTimeout);
84+
Assert.AreEqual("OperationTimeout", ex.ParamName);
9485
}
9586

9687
[TestMethod]
@@ -100,17 +91,8 @@ public void OperationTimeout_GreaterThanLowerLimit()
10091
var connectionInfo = new PasswordConnectionInfo("host", 22, "admin", "pwd");
10192
var target = new NetConfClient(connectionInfo);
10293

103-
try
104-
{
105-
target.OperationTimeout = operationTimeout;
106-
}
107-
catch (ArgumentOutOfRangeException ex)
108-
{
109-
Assert.IsNull(ex.InnerException);
110-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
111-
112-
Assert.AreEqual("OperationTimeout", ex.ParamName);
113-
}
94+
var ex = Assert.Throws<ArgumentOutOfRangeException>(() => target.OperationTimeout = operationTimeout);
95+
Assert.AreEqual("OperationTimeout", ex.ParamName);
11496
}
11597
}
11698
}

‎test/Renci.SshNet.Tests/Classes/SftpClientTest.cs‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -83,17 +83,8 @@ public void OperationTimeout_LessThanLowerLimit()
8383
var connectionInfo = new PasswordConnectionInfo("host", 22, "admin", "pwd");
8484
var target = new SftpClient(connectionInfo);
8585

86-
try
87-
{
88-
target.OperationTimeout = operationTimeout;
89-
}
90-
catch (ArgumentOutOfRangeException ex)
91-
{
92-
Assert.IsNull(ex.InnerException);
93-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
94-
95-
Assert.AreEqual("OperationTimeout", ex.ParamName);
96-
}
86+
var ex = Assert.Throws<ArgumentOutOfRangeException>(() => target.OperationTimeout = operationTimeout);
87+
Assert.AreEqual("OperationTimeout", ex.ParamName);
9788
}
9889

9990
[TestMethod]
@@ -103,17 +94,8 @@ public void OperationTimeout_GreaterThanLowerLimit()
10394
var connectionInfo = new PasswordConnectionInfo("host", 22, "admin", "pwd");
10495
var target = new SftpClient(connectionInfo);
10596

106-
try
107-
{
108-
target.OperationTimeout = operationTimeout;
109-
}
110-
catch (ArgumentOutOfRangeException ex)
111-
{
112-
Assert.IsNull(ex.InnerException);
113-
ArgumentExceptionAssert.MessageEquals("The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive.", ex);
114-
115-
Assert.AreEqual("OperationTimeout", ex.ParamName);
116-
}
97+
var ex = Assert.Throws<ArgumentOutOfRangeException>(() => target.OperationTimeout = operationTimeout);
98+
Assert.AreEqual("OperationTimeout", ex.ParamName);
11799
}
118100
}
119101
}

0 commit comments

Comments
 (0)