Skip to content

Miscellaneous cleanup/throw helpers #1491

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.

Sign up for GitHub

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 5 commits into from
Sep 19, 2024
Merged

Miscellaneous cleanup/throw helpers #1491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc106ee
PrivateKeyFile stuff
Rob-Hague Sep 10, 2024
60ab3ba
Extract from net7 branch
Rob-Hague Aug 8, 2024
c8f81bb
more ThrowHelper stuff
Rob-Hague Sep 10, 2024
36c69b4
Merge branch 'develop' into misc
Rob-Hague Sep 11, 2024
39b852b
Merge branch 'develop' into misc
Rob-Hague Sep 14, 2024
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
2 changes: 1 addition & 1 deletion Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
Disable nullable warnings on old frameworks because of missing annotations.
-->
<PropertyGroup Condition=" !$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0')) ">
<NoWarn>$(NoWarn);CS8602</NoWarn>
<NoWarn>$(NoWarn);CS8602;CS8604;CS8777</NoWarn>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not possible to fix these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are coming from string.IsNullOrEmpty and friends which are not annotated for nullable analysis on lower targets

</PropertyGroup>

<!--
Expand Down
1 change: 1 addition & 0 deletions Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<PackageVersion Include="MSTest.TestFramework" Version="3.6.0" />
<PackageVersion Include="Moq" Version="4.20.72" />
<PackageVersion Include="Nerdbank.GitVersioning" Version="3.7.70-alpha" />
<PackageVersion Include="PolySharp" Version="1.14.1" />
<PackageVersion Include="SonarAnalyzer.CSharp" Version="9.19.0.84025" />
<PackageVersion Include="StyleCop.Analyzers" Version="1.2.0-beta.556" />
<PackageVersion Include="System.Formats.Asn1" Version="8.0.1" />
Expand Down
3 changes: 3 additions & 0 deletions src/Renci.SshNet/.editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,3 +188,6 @@ dotnet_diagnostic.MA0040.severity = none
# MA0042: Do not use blocking calls in an async method
# duplicate of CA1849
dotnet_diagnostic.MA0042.severity = none

# S3236: Caller information arguments should not be provided explicitly
dotnet_diagnostic.S3236.severity = none
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this one reported? Don't have time to check myself now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It comes when one wants to provide a value for a parameter marked with [CallerExpressionAttribute], e.g.

public TimeSpan Timeout
{
    get
    {
        return _timeout;
    }
    set
    {
        // EnsureValidTimeout(this TimeSpan timeSpan, [CallerArgumentExpression(nameof(timeSpan))] string? paramName = null)

        value.EnsureValidTimeout(nameof(Timeout));

        _timeout = value;
    }
}

I would probably suppress this one at the call sites, but #1480 will also bring more places where we don't want it (from Debug.Assert)

4 changes: 2 additions & 2 deletions src/Renci.SshNet/Abstractions/SocketAbstraction.cs
View file
Open in desktop
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 = timeout.AsTimeout(nameof(timeout));
socket.ReceiveTimeout = timeout.AsTimeout();

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 = readTimeout.AsTimeout(nameof(readTimeout));
socket.ReceiveTimeout = readTimeout.AsTimeout();

do
{
Expand Down
12 changes: 4 additions & 8 deletions src/Renci.SshNet/Abstractions/ThreadAbstraction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System.Threading;
using System.Threading.Tasks;

using Renci.SshNet.Common;

namespace Renci.SshNet.Abstractions
{
internal static class ThreadAbstraction
Expand All @@ -16,10 +18,7 @@ internal static class ThreadAbstraction
/// </returns>
public static Task ExecuteThreadLongRunning(Action action)
{
if (action is null)
{
throw new ArgumentNullException(nameof(action));
}
ThrowHelper.ThrowIfNull(action);

return Task.Factory.StartNew(action,
CancellationToken.None,
Expand All @@ -33,10 +32,7 @@ public static Task ExecuteThreadLongRunning(Action action)
/// <param name="action">The action to execute.</param>
public static void ExecuteThread(Action action)
{
if (action is null)
{
throw new ArgumentNullException(nameof(action));
}
ThrowHelper.ThrowIfNull(action);

_ = ThreadPool.QueueUserWorkItem(o => action());
}
Expand Down
7 changes: 3 additions & 4 deletions src/Renci.SshNet/AuthenticationMethod.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System;

using Renci.SshNet.Common;

namespace Renci.SshNet
{
/// <summary>
Expand Down Expand Up @@ -34,10 +36,7 @@ public abstract class AuthenticationMethod : IAuthenticationMethod
/// <exception cref="ArgumentException"><paramref name="username"/> is whitespace or <see langword="null"/>.</exception>
protected AuthenticationMethod(string username)
{
if (string.IsNullOrWhiteSpace(username))
{
throw new ArgumentException("username");
}
ThrowHelper.ThrowIfNullOrWhiteSpace(username);

Username = username;
}
Expand Down
22 changes: 4 additions & 18 deletions src/Renci.SshNet/BaseClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,8 @@ protected BaseClient(ConnectionInfo connectionInfo, bool ownsConnectionInfo)
/// </remarks>
private protected BaseClient(ConnectionInfo connectionInfo, bool ownsConnectionInfo, IServiceFactory serviceFactory)
{
if (connectionInfo is null)
{
throw new ArgumentNullException(nameof(connectionInfo));
}

if (serviceFactory is null)
{
throw new ArgumentNullException(nameof(serviceFactory));
}
ThrowHelper.ThrowIfNull(connectionInfo);
ThrowHelper.ThrowIfNull(serviceFactory);

_connectionInfo = connectionInfo;
_ownsConnectionInfo = ownsConnectionInfo;
Expand Down Expand Up @@ -458,17 +451,10 @@ protected virtual void Dispose(bool disposing)
/// <summary>
/// Check if the current instance is disposed.
/// </summary>
/// <exception cref="ObjectDisposedException">THe current instance is disposed.</exception>
/// <exception cref="ObjectDisposedException">The current instance is disposed.</exception>
protected void CheckDisposed()
{
#if NET7_0_OR_GREATER
ObjectDisposedException.ThrowIf(_isDisposed, this);
#else
if (_isDisposed)
{
throw new ObjectDisposedException(GetType().FullName);
}
#endif // NET7_0_OR_GREATER
ThrowHelper.ThrowObjectDisposedIf(_isDisposed, this);
}

/// <summary>
Expand Down
11 changes: 2 additions & 9 deletions src/Renci.SshNet/ClientAuthentication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,8 @@ internal int PartialSuccessLimit
/// <exception cref="SshAuthenticationException">Failed to authenticate the client.</exception>
public void Authenticate(IConnectionInfoInternal connectionInfo, ISession session)
{
if (connectionInfo is null)
{
throw new ArgumentNullException(nameof(connectionInfo));
}

if (session is null)
{
throw new ArgumentNullException(nameof(session));
}
ThrowHelper.ThrowIfNull(connectionInfo);
ThrowHelper.ThrowIfNull(session);

session.RegisterMessage("SSH_MSG_USERAUTH_FAILURE");
session.RegisterMessage("SSH_MSG_USERAUTH_SUCCESS");
Expand Down
5 changes: 1 addition & 4 deletions src/Renci.SshNet/Common/ChannelDataEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ internal class ChannelDataEventArgs : ChannelEventArgs
public ChannelDataEventArgs(uint channelNumber, byte[] data)
: base(channelNumber)
{
if (data is null)
{
throw new ArgumentNullException(nameof(data));
}
ThrowHelper.ThrowIfNull(data);

Data = data;
}
Expand Down
15 changes: 2 additions & 13 deletions src/Renci.SshNet/Common/ChannelInputStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,7 @@ public override int Read(byte[] buffer, int offset, int count)
/// <exception cref="ArgumentOutOfRangeException">offset or count is negative.</exception>
public override void Write(byte[] buffer, int offset, int count)
{
if (buffer == null)
{
throw new ArgumentNullException(nameof(buffer));
}
ThrowHelper.ThrowIfNull(buffer);

if (offset + count > buffer.Length)
{
Expand All @@ -116,10 +113,7 @@ public override void Write(byte[] buffer, int offset, int count)
throw new ArgumentOutOfRangeException(nameof(offset), "offset or count is negative.");
}

if (_isDisposed)
{
throw CreateObjectDisposedException();
}
ThrowHelper.ThrowObjectDisposedIf(_isDisposed, this);

if (count == 0)
{
Expand Down Expand Up @@ -208,10 +202,5 @@ public override long Position
get { return _totalPosition; }
set { throw new NotSupportedException(); }
}

private ObjectDisposedException CreateObjectDisposedException()
{
return new ObjectDisposedException(GetType().FullName);
}
}
}
5 changes: 1 addition & 4 deletions src/Renci.SshNet/Common/ChannelRequestEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ internal sealed class ChannelRequestEventArgs : EventArgs
/// <exception cref="ArgumentNullException"><paramref name="info"/> is <see langword="null"/>.</exception>
public ChannelRequestEventArgs(RequestInfo info)
{
if (info is null)
{
throw new ArgumentNullException(nameof(info));
}
ThrowHelper.ThrowIfNull(info);

Info = info;
}
Expand Down
31 changes: 8 additions & 23 deletions src/Renci.SshNet/Common/Extensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Net;
using System.Net.Sockets;
using System.Numerics;
using System.Runtime.CompilerServices;
using System.Text;

using Renci.SshNet.Abstractions;
Expand Down Expand Up @@ -148,7 +149,7 @@ internal static void DebugPrint(this IEnumerable<byte> bytes)
Debug.WriteLine(sb.ToString());
}

internal static void ValidatePort(this uint value, string argument)
internal static void ValidatePort(this uint value, [CallerArgumentExpression(nameof(value))] string argument = null)
{
if (value > IPEndPoint.MaxPort)
{
Expand All @@ -157,7 +158,7 @@ internal static void ValidatePort(this uint value, string argument)
}
}

internal static void ValidatePort(this int value, string argument)
internal static void ValidatePort(this int value, [CallerArgumentExpression(nameof(value))] string argument = null)
{
if (value < IPEndPoint.MinPort)
{
Expand Down Expand Up @@ -187,10 +188,7 @@ internal static void ValidatePort(this int value, string argument)
/// </remarks>
public static byte[] Take(this byte[] value, int offset, int count)
{
if (value is null)
{
throw new ArgumentNullException(nameof(value));
}
ThrowHelper.ThrowIfNull(value);

if (count == 0)
{
Expand Down Expand Up @@ -222,10 +220,7 @@ public static byte[] Take(this byte[] value, int offset, int count)
/// </remarks>
public static byte[] Take(this byte[] value, int count)
{
if (value is null)
{
throw new ArgumentNullException(nameof(value));
}
ThrowHelper.ThrowIfNull(value);

if (count == 0)
{
Expand All @@ -244,15 +239,8 @@ public static byte[] Take(this byte[] value, int count)

public static bool IsEqualTo(this byte[] left, byte[] right)
{
if (left is null)
{
throw new ArgumentNullException(nameof(left));
}

if (right is null)
{
throw new ArgumentNullException(nameof(right));
}
ThrowHelper.ThrowIfNull(left);
ThrowHelper.ThrowIfNull(right);

return left.AsSpan().SequenceEqual(right);
}
Expand All @@ -266,10 +254,7 @@ public static bool IsEqualTo(this byte[] left, byte[] right)
/// </returns>
public static byte[] TrimLeadingZeros(this byte[] value)
{
if (value is null)
{
throw new ArgumentNullException(nameof(value));
}
ThrowHelper.ThrowIfNull(value);

for (var i = 0; i < value.Length; i++)
{
Expand Down
15 changes: 2 additions & 13 deletions src/Renci.SshNet/Common/HostKeyEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,7 @@ public string FingerPrintMD5
/// <exception cref="ArgumentNullException"><paramref name="host"/> is <see langword="null"/>.</exception>
public HostKeyEventArgs(KeyHostAlgorithm host)
{
if (host is null)
{
throw new ArgumentNullException(nameof(host));
}
ThrowHelper.ThrowIfNull(host);

CanTrust = true;
HostKey = host.Data;
Expand All @@ -102,15 +99,7 @@ public HostKeyEventArgs(KeyHostAlgorithm host)

_lazyFingerPrint = new Lazy<byte[]>(() => CryptoAbstraction.HashMD5(HostKey));

_lazyFingerPrintSHA256 = new Lazy<string>(() =>
{
return Convert.ToBase64String(CryptoAbstraction.HashSHA256(HostKey))
#if NET || NETSTANDARD2_1_OR_GREATER
.Replace("=", string.Empty, StringComparison.Ordinal);
#else
.Replace("=", string.Empty);
#endif // NET || NETSTANDARD2_1_OR_GREATER
});
_lazyFingerPrintSHA256 = new Lazy<string>(() => Convert.ToBase64String(CryptoAbstraction.HashSHA256(HostKey)).TrimEnd('='));

_lazyFingerPrintMD5 = new Lazy<string>(() =>
{
Expand Down
5 changes: 1 addition & 4 deletions src/Renci.SshNet/Common/PacketDump.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ public static string Create(List<byte> data, int indentLevel)

public static string Create(byte[] data, int indentLevel)
{
if (data is null)
{
throw new ArgumentNullException(nameof(data));
}
ThrowHelper.ThrowIfNull(data);

if (indentLevel < 0)
{
Expand Down
14 changes: 1 addition & 13 deletions src/Renci.SshNet/Common/PipeStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public override void Write(byte[] buffer, int offset, int count)
{
lock (_sync)
{
ThrowIfDisposed();
ThrowHelper.ThrowObjectDisposedIf(_disposed, this);

AssertValid();

Expand Down Expand Up @@ -232,17 +232,5 @@ public override long Position
get { return 0; }
set { throw new NotSupportedException(); }
}

private void ThrowIfDisposed()
{
#if NET7_0_OR_GREATER
ObjectDisposedException.ThrowIf(_disposed, this);
#else
if (_disposed)
{
throw new ObjectDisposedException(GetType().FullName);
}
#endif // NET7_0_OR_GREATER
}
}
}
8 changes: 2 additions & 6 deletions src/Renci.SshNet/Common/PortForwardEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,8 @@ public class PortForwardEventArgs : EventArgs
/// <exception cref="ArgumentOutOfRangeException"><paramref name="port" /> is not within <see cref="IPEndPoint.MinPort" /> and <see cref="IPEndPoint.MaxPort" />.</exception>
internal PortForwardEventArgs(string host, uint port)
{
if (host is null)
{
throw new ArgumentNullException(nameof(host));
}

port.ValidatePort("port");
ThrowHelper.ThrowIfNull(host);
port.ValidatePort();

OriginatorHost = host;
OriginatorPort = port;
Expand Down
Loading