Skip to content

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
merged 13 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/Renci.SshNet/.editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,7 @@ dotnet_diagnostic.IDE0048.severity = none
# IDE0305: Collection initialization can be simplified
# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0305
dotnet_diagnostic.IDE0305.severity = none

# IDE0005: Remove unnecessary using directives
# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0005
dotnet_diagnostic.IDE0005.severity = suggestion
Comment on lines +162 to +165
Copy link
Collaborator

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.

Copy link
Collaborator

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 :-(

Copy link
Collaborator

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.

4 changes: 2 additions & 2 deletions src/Renci.SshNet/Abstractions/SocketAbstraction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(nameof(timeout));

try
{
Expand Down Expand Up @@ -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(nameof(readTimeout));

do
{
Expand Down
2 changes: 2 additions & 0 deletions src/Renci.SshNet/BaseClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ public TimeSpan KeepAliveInterval
{
CheckDisposed();

value.EnsureValidTimeout(nameof(KeepAliveInterval));

if (value == _keepAliveInterval)
{
return;
Expand Down
46 changes: 46 additions & 0 deletions src/Renci.SshNet/Common/TimeSpanExtensions.cs
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
? 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);
}
}
}
}
31 changes: 29 additions & 2 deletions src/Renci.SshNet/ConnectionInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ public class ConnectionInfo : IConnectionInfoInternal
/// </value>
private static readonly TimeSpan DefaultChannelCloseTimeout = TimeSpan.FromSeconds(1);

private TimeSpan _timeout;
private TimeSpan _channelCloseTimeout;

/// <summary>
/// Gets supported key exchange algorithms for this connection.
/// </summary>
Expand Down Expand Up @@ -145,7 +148,19 @@ public class ConnectionInfo : IConnectionInfoInternal
/// <value>
/// The connection timeout. The default value is 30 seconds.
/// </value>
public TimeSpan Timeout { get; set; }
public TimeSpan Timeout
{
get
{
return _timeout;
}
set
{
value.EnsureValidTimeout(nameof(Timeout));

_timeout = value;
}
}

/// <summary>
/// Gets or sets the timeout to use when waiting for a server to acknowledge closing a channel.
Expand All @@ -157,7 +172,19 @@ public class ConnectionInfo : IConnectionInfoInternal
/// If a server does not send a <c>SSH_MSG_CHANNEL_CLOSE</c> message before the specified timeout
/// elapses, the channel will be closed immediately.
/// </remarks>
public TimeSpan ChannelCloseTimeout { get; set; }
public TimeSpan ChannelCloseTimeout
{
get
{
return _channelCloseTimeout;
}
set
{
value.EnsureValidTimeout(nameof(ChannelCloseTimeout));

_channelCloseTimeout = value;
}
}

/// <summary>
/// Gets or sets the character encoding.
Expand Down
2 changes: 2 additions & 0 deletions src/Renci.SshNet/ForwardedPort.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ public void Dispose()
/// <param name="timeout">The maximum amount of time to wait for pending requests to finish processing.</param>
protected virtual void StopPort(TimeSpan timeout)
{
timeout.EnsureValidTimeout(nameof(timeout));

RaiseClosing();

var session = Session;
Expand Down
2 changes: 2 additions & 0 deletions src/Renci.SshNet/ForwardedPortDynamic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ protected override void StartPort()
/// <param name="timeout">The maximum amount of time to wait for pending requests to finish processing.</param>
protected override void StopPort(TimeSpan timeout)
{
timeout.EnsureValidTimeout(nameof(timeout));

if (!ForwardedPortStatus.ToStopping(ref _status))
{
return;
Expand Down
2 changes: 2 additions & 0 deletions src/Renci.SshNet/ForwardedPortLocal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ protected override void StartPort()
/// <param name="timeout">The maximum amount of time to wait for pending requests to finish processing.</param>
protected override void StopPort(TimeSpan timeout)
{
timeout.EnsureValidTimeout(nameof(timeout));

if (!ForwardedPortStatus.ToStopping(ref _status))
{
return;
Expand Down
2 changes: 2 additions & 0 deletions src/Renci.SshNet/ForwardedPortRemote.cs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ protected override void StartPort()
/// <param name="timeout">The maximum amount of time to wait for the port to stop.</param>
protected override void StopPort(TimeSpan timeout)
{
timeout.EnsureValidTimeout(nameof(timeout));

if (!ForwardedPortStatus.ToStopping(ref _status))
{
return;
Expand Down
8 changes: 1 addition & 7 deletions src/Renci.SshNet/NetConfClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(nameof(OperationTimeout));
}
}

Expand Down
15 changes: 14 additions & 1 deletion src/Renci.SshNet/ScpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public partial class ScpClient : BaseClient
private static readonly Regex TimestampRe = new Regex(@"T(?<mtime>\d+) 0 (?<atime>\d+) 0", RegexOptions.Compiled);

private IRemotePathTransformation _remotePathTransformation;
private TimeSpan _operationTimeout;

/// <summary>
/// Gets or sets the operation timeout.
Expand All @@ -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.
/// </value>
public TimeSpan OperationTimeout { get; set; }
public TimeSpan OperationTimeout
{
get
{
return _operationTimeout;
}
set
{
value.EnsureValidTimeout(nameof(OperationTimeout));

_operationTimeout = value;
}
}

/// <summary>
/// Gets or sets the size of the buffer.
Expand Down
15 changes: 14 additions & 1 deletion src/Renci.SshNet/Sftp/SftpFileStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class SftpFileStream : Stream
private bool _canRead;
private bool _canSeek;
private bool _canWrite;
private TimeSpan _timeout;

/// <summary>
/// Gets a value indicating whether the current stream supports reading.
Expand Down Expand Up @@ -176,7 +177,19 @@ public virtual byte[] Handle
/// <value>
/// The timeout.
/// </value>
public TimeSpan Timeout { get; set; }
public TimeSpan Timeout
{
get
{
return _timeout;
}
set
{
value.EnsureValidTimeout(nameof(Timeout));

_timeout = value;
}
}

private SftpFileStream(ISftpSession session, string path, FileAccess access, int bufferSize, byte[] handle, long position)
{
Expand Down
8 changes: 1 addition & 7 deletions src/Renci.SshNet/SftpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(nameof(OperationTimeout));
}
}

Expand Down
15 changes: 14 additions & 1 deletion src/Renci.SshNet/SshCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class SshCommand : IDisposable
private bool _hasError;
private bool _isDisposed;
private ChannelInputStream _inputStream;
private TimeSpan _commandTimeout;

/// <summary>
/// Gets the command text.
Expand All @@ -44,7 +45,19 @@ public class SshCommand : IDisposable
/// <value>
/// The command timeout.
/// </value>
public TimeSpan CommandTimeout { get; set; }
public TimeSpan CommandTimeout
{
get
{
return _commandTimeout;
}
set
{
value.EnsureValidTimeout(nameof(CommandTimeout));

_commandTimeout = value;
}
}

/// <summary>
/// Gets the command exit status.
Expand Down
103 changes: 103 additions & 0 deletions test/Renci.SshNet.Tests/Classes/Common/TimeSpanExtensionsTest.cs
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);
}
}
}
}
Loading