-
-
Notifications
You must be signed in to change notification settings - Fork 952
Limit TimeSpan timeouts to Int32 MaxValue #1321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
WojciechNagorski
merged 13 commits into
sshnet:develop
from
jscarle:feature/ConnectionInfoTimeout
Feb 20, 2024
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
3ad2451
Added guard clauses to various timeouts to ensure they don't exceed a…
jscarle 76f2878
Merge branch 'develop' into feature/ConnectionInfoTimeout
jscarle 1ba6a63
Merge branch 'develop' into feature/ConnectionInfoTimeout
jscarle 132e3ec
Fixed guard clauses.
jscarle e7afbd4
Updated build tags.
jscarle 5b1a111
Added guard clauses to various timeouts to ensure they don't exceed a…
jscarle 1564121
Merge branch 'feature/ConnectionInfoTimeout_v2' into feature/Connecti…
jscarle 7bdd6f1
Fixed tests.
jscarle c3273b0
Added additional tests.
jscarle b4127ac
Replaced NoWarn with .editorconfig setting
jscarle 25feb4c
Merge branch 'develop' into feature/ConnectionInfoTimeout
jscarle e13d3f5
Merge branch 'develop' into feature/ConnectionInfoTimeout
jscarle f229e42
Fixed references to parameter names.
jscarle File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
using System; | ||
|
||
namespace Renci.SshNet.Common | ||
{ | ||
/// <summary> | ||
/// Provides extension methods for <see cref="TimeSpan"/>. | ||
/// </summary> | ||
internal static class TimeSpanExtensions | ||
{ | ||
private const string OutOfRangeTimeoutMessage = | ||
$"The timeout must represent a value between -1 and Int32.MaxValue milliseconds, inclusive."; | ||
|
||
/// <summary> | ||
/// Returns the specified <paramref name="timeSpan"/> as a valid timeout in milliseconds. | ||
/// </summary> | ||
/// <param name="timeSpan">The <see cref="TimeSpan"/> to ensure validity.</param> | ||
/// <param name="callerMemberName">The name of the calling member.</param> | ||
/// <exception cref="ArgumentOutOfRangeException"> | ||
/// Thrown when <paramref name="timeSpan"/> does not represent a value between -1 and <see cref="int.MaxValue"/>, inclusive. | ||
/// </exception> | ||
public static int AsTimeout(this TimeSpan timeSpan, string callerMemberName) | ||
{ | ||
var timeoutInMilliseconds = timeSpan.TotalMilliseconds; | ||
return timeoutInMilliseconds is < -1d or > int.MaxValue | ||
jscarle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
? throw new ArgumentOutOfRangeException(callerMemberName, OutOfRangeTimeoutMessage) | ||
: (int) timeoutInMilliseconds; | ||
} | ||
|
||
/// <summary> | ||
/// Ensures that the specified <paramref name="timeSpan"/> represents a valid timeout in milliseconds. | ||
/// </summary> | ||
/// <param name="timeSpan">The <see cref="TimeSpan"/> to ensure validity.</param> | ||
/// <param name="callerMemberName">The name of the calling member.</param> | ||
/// <exception cref="ArgumentOutOfRangeException"> | ||
/// Thrown when <paramref name="timeSpan"/> does not represent a value between -1 and <see cref="int.MaxValue"/>, inclusive. | ||
/// </exception> | ||
public static void EnsureValidTimeout(this TimeSpan timeSpan, string callerMemberName) | ||
{ | ||
var timeoutInMilliseconds = timeSpan.TotalMilliseconds; | ||
if (timeoutInMilliseconds is < -1d or > int.MaxValue) | ||
{ | ||
throw new ArgumentOutOfRangeException(callerMemberName, OutOfRangeTimeoutMessage); | ||
} | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
103 changes: 103 additions & 0 deletions
103
test/Renci.SshNet.Tests/Classes/Common/TimeSpanExtensionsTest.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
using System; | ||
|
||
using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
|
||
using Renci.SshNet.Common; | ||
using Renci.SshNet.Tests.Common; | ||
|
||
namespace Renci.SshNet.Tests.Classes.Common | ||
{ | ||
[TestClass] | ||
public class TimeSpanExtensionsTest | ||
{ | ||
[TestMethod] | ||
public void AsTimeout_ValidTimeSpan_ReturnsExpectedMilliseconds() | ||
{ | ||
var timeSpan = TimeSpan.FromSeconds(10); | ||
|
||
var timeout = timeSpan.AsTimeout("TestMethodName"); | ||
|
||
Assert.AreEqual(10000, timeout); | ||
} | ||
|
||
[TestMethod] | ||
[ExpectedException(typeof(ArgumentOutOfRangeException))] | ||
public void AsTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException() | ||
{ | ||
var timeSpan = TimeSpan.FromSeconds(-1); | ||
|
||
timeSpan.AsTimeout("TestMethodName"); | ||
} | ||
|
||
[TestMethod] | ||
[ExpectedException(typeof(ArgumentOutOfRangeException))] | ||
public void AsTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException() | ||
{ | ||
var timeSpan = TimeSpan.FromMilliseconds((double) int.MaxValue + 1); | ||
|
||
timeSpan.AsTimeout("TestMethodName"); | ||
} | ||
|
||
[TestMethod] | ||
public void AsTimeout_ArgumentOutOfRangeException_HasCorrectInformation() | ||
{ | ||
|
||
try | ||
{ | ||
var timeSpan = TimeSpan.FromMilliseconds((double) int.MaxValue + 1); | ||
|
||
timeSpan.AsTimeout("TestMethodName"); | ||
} | ||
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("TestMethodName", ex.ParamName); | ||
} | ||
} | ||
|
||
[TestMethod] | ||
public void EnsureValidTimeout_ValidTimeSpan_DoesNotThrow() | ||
{ | ||
var timeSpan = TimeSpan.FromSeconds(5); | ||
|
||
timeSpan.EnsureValidTimeout("TestMethodName"); | ||
} | ||
|
||
[TestMethod] | ||
[ExpectedException(typeof(ArgumentOutOfRangeException))] | ||
public void EnsureValidTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException() | ||
{ | ||
var timeSpan = TimeSpan.FromSeconds(-1); | ||
|
||
timeSpan.EnsureValidTimeout("TestMethodName"); | ||
} | ||
|
||
[TestMethod] | ||
[ExpectedException(typeof(ArgumentOutOfRangeException))] | ||
public void EnsureValidTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException() | ||
{ | ||
var timeSpan = TimeSpan.FromMilliseconds((double) int.MaxValue + 1); | ||
|
||
timeSpan.EnsureValidTimeout("TestMethodName"); | ||
} | ||
|
||
[TestMethod] | ||
public void EnsureValidTimeout_ArgumentOutOfRangeException_HasCorrectInformation() | ||
{ | ||
|
||
try | ||
{ | ||
var timeSpan = TimeSpan.FromMilliseconds((double) int.MaxValue + 1); | ||
|
||
timeSpan.EnsureValidTimeout("TestMethodName"); | ||
} | ||
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("TestMethodName", ex.ParamName); | ||
} | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you revert this? I think after restarting VS (or restarting your computer) everything should work.
This is the last message and then I will be able to merge this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am seeing this locally as well, the errors look real. I think it could be related to upgrading the SDK but I have no idea why we haven't seen it before.
I am trying to fix it with
dotnet format
but not having much luck :-(There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wrong, I don't know what's going on.