Skip to content

Commit

Permalink
Limit TimeSpan timeouts to Int32 MaxValue (#1321)
Browse files Browse the repository at this point in the history
* Added guard clauses to various timeouts to ensure they don't exceed an Int32 in milliseconds.

* Fixed guard clauses.

* Updated build tags.

* Added guard clauses to various timeouts to ensure they don't exceed an Int32 in milliseconds.

* Fixed tests.

* Added additional tests.

* Replaced NoWarn with .editorconfig setting

* Fixed references to parameter names.
  • Loading branch information
jscarle authored Feb 20, 2024
1 parent 664595d commit 4b9c3bf
Show file tree
Hide file tree
Showing 18 changed files with 328 additions and 29 deletions.
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
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

0 comments on commit 4b9c3bf

Please sign in to comment.