Browse Source

Update SonarAnalyzer.CSharp (#1494)

* Update SonarAnalyzer.CSharp

* fix S3993: Custom attributes should be marked with "System.AttributeUsageAttribute"

https://rules.sonarsource.com/csharp/RSPEC-3993/

* fix S6966: Awaitable method should be used

Introduced abstractions for CancellationTokenSource.CancelAsync()
and Stream.DisposeAsync() to avoid #ifdef.

Supressed pipeStream.WriteAsync because it deadlocks the test.
I assume because PipeStream doesn't override WriteAsync.

https://rules.sonarsource.com/csharp/RSPEC-6966/

temp

* fix S3431: "[ExpectedException]" should not be used

Removed the Connect() from Multifactor_PublicKeyWithEmptyPassPhrase
because the Exception is already thrown in the factory.

https://rules.sonarsource.com/csharp/RSPEC-3431/

* fix S2325: Methods and properties that don't access instance data should be static

This one is pretty redundant with CA1822 (which is also disabled in the
tests).

It caught a few more cases in the library itself, most of which can't
be changed because they are public API.

https://rules.sonarsource.com/csharp/RSPEC-2325/

* fix S127: "for" loop stop conditions should be invariant

not sure if this one is worth having. The only cases it found
are imho legitimate or not worth fixing, so I supressed them.

https://rules.sonarsource.com/csharp/RSPEC-127/

* fix S1964: An abstract class should have both abstract and concrete methods

Suppressed for public APIs, changed ExtendedReplyInfo to interface.

https://rules.sonarsource.com/csharp/RSPEC-1694/

* Remove redundant test

this is already covered by Test_PrivateKey_SSH2_Encrypted_ShouldThrowSshPassPhraseNullOrEmptyExceptionWhenPassphraseIsNull

* Revert "fix S2325: Methods and properties that don't access instance data should be static"

suppress it instead

This reverts commit 2020604958dfc3ebc7d7605fdc3ba2d4f6c1dab5.

* Revert "fix S127: "for" loop stop conditions should be invariant"

suppress it instead

This reverts commit 1d8b4ac3359d973244d0077bd8fd68ee84741006.
mus65 1 year ago
parent
commit
3b55ba3a0a

+ 12 - 0
.editorconfig

@@ -29,6 +29,12 @@ dotnet_diagnostic.S101.severity = none
 # This is a duplicate of CA2201 and MA0012.
 # This is a duplicate of CA2201 and MA0012.
 dotnet_diagnostic.S112.severity = none
 dotnet_diagnostic.S112.severity = none
 
 
+# S127: "for" loop stop conditions should be invariant
+# https://rules.sonarsource.com/csharp/RSPEC-127
+#
+# Limited use.
+dotnet_diagnostic.S127.severity = none
+
 # S907: Remove use of 'goto'
 # S907: Remove use of 'goto'
 # https://rules.sonarsource.com/csharp/RSPEC-907
 # https://rules.sonarsource.com/csharp/RSPEC-907
 #
 #
@@ -128,6 +134,12 @@ dotnet_diagnostic.S2259.severity = none
 # This is a duplicate of IDE0032.
 # This is a duplicate of IDE0032.
 dotnet_diagnostic.S2292.severity = none
 dotnet_diagnostic.S2292.severity = none
 
 
+# S2325: Methods and properties that don't access instance data should be static
+# https://rules.sonarsource.com/csharp/RSPEC-2325
+#
+# This is a duplicate of CA1822
+dotnet_diagnostic.S2325.severity = none
+
 # S2445: Blocks should be synchronized on read-only fields
 # S2445: Blocks should be synchronized on read-only fields
 # https://rules.sonarsource.com/csharp/RSPEC-2445
 # https://rules.sonarsource.com/csharp/RSPEC-2445
 #
 #

+ 1 - 1
Directory.Packages.props

@@ -19,7 +19,7 @@
     <PackageVersion Include="Moq" Version="4.20.72" />
     <PackageVersion Include="Moq" Version="4.20.72" />
     <PackageVersion Include="Nerdbank.GitVersioning" Version="3.7.70-alpha" />
     <PackageVersion Include="Nerdbank.GitVersioning" Version="3.7.70-alpha" />
     <PackageVersion Include="PolySharp" Version="1.14.1" />
     <PackageVersion Include="PolySharp" Version="1.14.1" />
-    <PackageVersion Include="SonarAnalyzer.CSharp" Version="9.19.0.84025" />
+    <PackageVersion Include="SonarAnalyzer.CSharp" Version="9.32.0.97167" />
     <PackageVersion Include="StyleCop.Analyzers" Version="1.2.0-beta.556" />
     <PackageVersion Include="StyleCop.Analyzers" Version="1.2.0-beta.556" />
     <PackageVersion Include="System.Formats.Asn1" Version="8.0.1" />
     <PackageVersion Include="System.Formats.Asn1" Version="8.0.1" />
     <PackageVersion Include="Testcontainers" Version="3.10.0" />
     <PackageVersion Include="Testcontainers" Version="3.10.0" />

+ 16 - 0
src/Renci.SshNet/Abstractions/CancellationTokenSourceExtensions.cs

@@ -0,0 +1,16 @@
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Renci.SshNet.Abstractions
+{
+    internal static class CancellationTokenSourceExtensions
+    {
+#if !NET8_OR_GREATER
+        public static Task CancelAsync(this CancellationTokenSource cancellationTokenSource)
+        {
+            cancellationTokenSource.Cancel();
+            return Task.CompletedTask;
+        }
+#endif
+    }
+}

+ 18 - 0
src/Renci.SshNet/Abstractions/StreamExtensions.cs

@@ -0,0 +1,18 @@
+#if !NET && !NETSTANDARD2_1_OR_GREATER
+using System.IO;
+using System.Threading.Tasks;
+#endif
+
+namespace Renci.SshNet.Abstractions
+{
+    internal static class StreamExtensions
+    {
+#if !NET && !NETSTANDARD2_1_OR_GREATER
+        public static ValueTask DisposeAsync(this Stream stream)
+        {
+            stream.Dispose();
+            return default;
+        }
+#endif
+    }
+}

+ 2 - 0
src/Renci.SshNet/Security/Algorithm.cs

@@ -3,7 +3,9 @@
     /// <summary>
     /// <summary>
     /// Represents the abstract base class from which all implementations of algorithms must inherit.
     /// Represents the abstract base class from which all implementations of algorithms must inherit.
     /// </summary>
     /// </summary>
+#pragma warning disable S1694 // An abstract class should have both abstract and concrete methods
     public abstract class Algorithm
     public abstract class Algorithm
+#pragma warning restore S1694 // An abstract class should have both abstract and concrete methods
     {
     {
         /// <summary>
         /// <summary>
         /// Gets the algorithm name.
         /// Gets the algorithm name.

+ 2 - 0
src/Renci.SshNet/Security/Cryptography/DigitalSignature.cs

@@ -3,7 +3,9 @@
     /// <summary>
     /// <summary>
     /// Base class for signature implementations.
     /// Base class for signature implementations.
     /// </summary>
     /// </summary>
+#pragma warning disable S1694 // An abstract class should have both abstract and concrete methods
     public abstract class DigitalSignature
     public abstract class DigitalSignature
+#pragma warning restore S1694 // An abstract class should have both abstract and concrete methods
     {
     {
         /// <summary>
         /// <summary>
         /// Verifies the signature.
         /// Verifies the signature.

+ 0 - 9
src/Renci.SshNet/Sftp/Responses/ExtendedReplies/ExtendedReplyInfo.cs

@@ -1,9 +0,0 @@
-using Renci.SshNet.Common;
-
-namespace Renci.SshNet.Sftp.Responses
-{
-    internal abstract class ExtendedReplyInfo
-    {
-        public abstract void LoadData(SshDataStream stream);
-    }
-}

+ 16 - 0
src/Renci.SshNet/Sftp/Responses/ExtendedReplies/IExtendedReplyInfo.cs

@@ -0,0 +1,16 @@
+using Renci.SshNet.Common;
+
+namespace Renci.SshNet.Sftp.Responses
+{
+    /// <summary>
+    /// Extended Reply Info.
+    /// </summary>
+    internal interface IExtendedReplyInfo
+    {
+        /// <summary>
+        /// Loads the data from the stream into the instance.
+        /// </summary>
+        /// <param name="stream">The stream.</param>
+        void LoadData(SshDataStream stream);
+    }
+}

+ 2 - 2
src/Renci.SshNet/Sftp/Responses/ExtendedReplies/StatVfsReplyInfo.cs

@@ -2,11 +2,11 @@
 
 
 namespace Renci.SshNet.Sftp.Responses
 namespace Renci.SshNet.Sftp.Responses
 {
 {
-    internal sealed class StatVfsReplyInfo : ExtendedReplyInfo
+    internal sealed class StatVfsReplyInfo : IExtendedReplyInfo
     {
     {
         public SftpFileSystemInformation Information { get; private set; }
         public SftpFileSystemInformation Information { get; private set; }
 
 
-        public override void LoadData(SshDataStream stream)
+        public void LoadData(SshDataStream stream)
         {
         {
             Information = new SftpFileSystemInformation(stream.ReadUInt64(), // FileSystemBlockSize
             Information = new SftpFileSystemInformation(stream.ReadUInt64(), // FileSystemBlockSize
                                                        stream.ReadUInt64(), // BlockSize
                                                        stream.ReadUInt64(), // BlockSize

+ 1 - 1
src/Renci.SshNet/Sftp/Responses/SftpExtendedReplyResponse.cs

@@ -13,7 +13,7 @@
         }
         }
 
 
         public T GetReply<T>()
         public T GetReply<T>()
-            where T : ExtendedReplyInfo, new()
+            where T : IExtendedReplyInfo, new()
         {
         {
             var result = new T();
             var result = new T();
             result.LoadData(DataStream);
             result.LoadData(DataStream);

+ 0 - 6
test/Renci.SshNet.IntegrationTests/AuthenticationMethodFactory.cs

@@ -27,12 +27,6 @@
             return new PrivateKeyAuthenticationMethod(Users.Regular.UserName, privateKeyFile);
             return new PrivateKeyAuthenticationMethod(Users.Regular.UserName, privateKeyFile);
         }
         }
 
 
-        public PrivateKeyAuthenticationMethod CreateRegularUserPrivateKeyWithEmptyPassPhraseAuthenticationMethod()
-        {
-            var privateKeyFile = GetPrivateKey("Data.Key.RSA.Encrypted.Aes.256.CBC.12345.txt", null);
-            return new PrivateKeyAuthenticationMethod(Users.Regular.UserName, privateKeyFile);
-        }
-
         public PrivateKeyAuthenticationMethod CreateRegularUserPrivateKeyAuthenticationMethodWithBadKey()
         public PrivateKeyAuthenticationMethod CreateRegularUserPrivateKeyAuthenticationMethodWithBadKey()
         {
         {
             string unauthorizedKey = """
             string unauthorizedKey = """

+ 0 - 15
test/Renci.SshNet.IntegrationTests/AuthenticationTests.cs

@@ -89,21 +89,6 @@ namespace Renci.SshNet.IntegrationTests
             }
             }
         }
         }
 
 
-        [TestMethod]
-        [ExpectedException(typeof(SshPassPhraseNullOrEmptyException))]
-        public void Multifactor_PublicKeyWithEmptyPassPhrase()
-        {
-            _remoteSshdConfig.WithAuthenticationMethods(Users.Regular.UserName, "publickey")
-                             .Update()
-                             .Restart();
-
-            var connectionInfo = _connectionInfoFactory.Create(_authenticationMethodFactory.CreateRegularUserPrivateKeyWithEmptyPassPhraseAuthenticationMethod());
-            using (var client = new SftpClient(connectionInfo))
-            {
-                client.Connect();
-            }
-        }
-
         [TestMethod]
         [TestMethod]
         public void Multifactor_PublicKey_MultiplePrivateKey()
         public void Multifactor_PublicKey_MultiplePrivateKey()
         {
         {

+ 4 - 1
test/Renci.SshNet.IntegrationTests/OldIntegrationTests/SshCommandTest.cs

@@ -1,5 +1,8 @@
 using System.Diagnostics;
 using System.Diagnostics;
 
 
+#if !NET8_0_OR_GREATER
+using Renci.SshNet.Abstractions;
+#endif
 using Renci.SshNet.Common;
 using Renci.SshNet.Common;
 
 
 namespace Renci.SshNet.IntegrationTests.OldIntegrationTests
 namespace Renci.SshNet.IntegrationTests.OldIntegrationTests
@@ -130,7 +133,7 @@ namespace Renci.SshNet.IntegrationTests.OldIntegrationTests
 
 
             Task executeTask = cmd.ExecuteAsync(cts.Token);
             Task executeTask = cmd.ExecuteAsync(cts.Token);
 
 
-            cts.Cancel();
+            await cts.CancelAsync();
 
 
             var tce = await Assert.ThrowsExceptionAsync<TaskCanceledException>(() => executeTask);
             var tce = await Assert.ThrowsExceptionAsync<TaskCanceledException>(() => executeTask);
             Assert.AreSame(executeTask, tce.Task);
             Assert.AreSame(executeTask, tce.Task);

+ 2 - 2
test/Renci.SshNet.IntegrationTests/SshTests.cs

@@ -318,7 +318,7 @@ namespace Renci.SshNet.IntegrationTests
                 {
                 {
                     using (var cmd = sshClient.CreateCommand("chmod 777 " + remoteFile))
                     using (var cmd = sshClient.CreateCommand("chmod 777 " + remoteFile))
                     {
                     {
-                        cmd.Execute();
+                        await cmd.ExecuteAsync();
 
 
                         Assert.AreEqual(0, cmd.ExitStatus, cmd.Error);
                         Assert.AreEqual(0, cmd.ExitStatus, cmd.Error);
                     }
                     }
@@ -333,7 +333,7 @@ namespace Renci.SshNet.IntegrationTests
                         {
                         {
                             var lines = new List<string>();
                             var lines = new List<string>();
                             string line = null;
                             string line = null;
-                            while ((line = reader.ReadLine()) != null)
+                            while ((line = await reader.ReadLineAsync()) != null)
                             {
                             {
                                 lines.Add(line);
                                 lines.Add(line);
                             }
                             }

+ 6 - 2
test/Renci.SshNet.IntegrationTests/TestsFixtures/InfrastructureFixture.cs

@@ -2,6 +2,10 @@
 using DotNet.Testcontainers.Containers;
 using DotNet.Testcontainers.Containers;
 using DotNet.Testcontainers.Images;
 using DotNet.Testcontainers.Images;
 
 
+#if !NET && !NETSTANDARD2_1_OR_GREATER
+using Renci.SshNet.Abstractions;
+#endif
+
 namespace Renci.SshNet.IntegrationTests.TestsFixtures
 namespace Renci.SshNet.IntegrationTests.TestsFixtures
 {
 {
     public sealed class InfrastructureFixture : IDisposable
     public sealed class InfrastructureFixture : IDisposable
@@ -80,8 +84,8 @@ namespace Renci.SshNet.IntegrationTests.TestsFixtures
                 await _sshServerImage.DisposeAsync();
                 await _sshServerImage.DisposeAsync();
             }
             }
 
 
-            _fsOut.Dispose();
-            _fsErr.Dispose();
+            await _fsOut.DisposeAsync();
+            await _fsErr.DisposeAsync();
         }
         }
 
 
         public void Dispose()
         public void Dispose()

+ 8 - 2
test/Renci.SshNet.Tests/Classes/Common/PipeStreamTest.cs

@@ -4,6 +4,9 @@ using System.Threading.Tasks;
 
 
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 
+#if !NET && !NETSTANDARD2_1_OR_GREATER
+using Renci.SshNet.Abstractions;
+#endif
 using Renci.SshNet.Common;
 using Renci.SshNet.Common;
 using Renci.SshNet.Tests.Common;
 using Renci.SshNet.Tests.Common;
 
 
@@ -93,7 +96,7 @@ namespace Renci.SshNet.Tests.Classes.Common
 
 
             Assert.IsFalse(readTask.IsCompleted);
             Assert.IsFalse(readTask.IsCompleted);
 
 
-            pipeStream.Dispose();
+            await pipeStream.DisposeAsync();
 
 
             Assert.AreEqual(0, await readTask);
             Assert.AreEqual(0, await readTask);
         }
         }
@@ -111,7 +114,7 @@ namespace Renci.SshNet.Tests.Classes.Common
 
 
             Assert.IsFalse(readTask.IsCompleted);
             Assert.IsFalse(readTask.IsCompleted);
 
 
-            pipeStream.Dispose();
+            await pipeStream.DisposeAsync();
 
 
             Assert.AreEqual(0, await readTask);
             Assert.AreEqual(0, await readTask);
         }
         }
@@ -130,7 +133,10 @@ namespace Renci.SshNet.Tests.Classes.Common
 
 
             Assert.IsFalse(readTask.IsCompleted);
             Assert.IsFalse(readTask.IsCompleted);
 
 
+            // not using WriteAsync here because it deadlocks the test
+#pragma warning disable S6966 // Awaitable method should be used
             pipeStream.Write(new byte[] { 1, 2, 3, 4 }, 0, 4);
             pipeStream.Write(new byte[] { 1, 2, 3, 4 }, 0, 4);
+#pragma warning restore S6966 // Awaitable method should be used
 
 
             Assert.AreEqual(0, await readTask);
             Assert.AreEqual(0, await readTask);
         }
         }

+ 4 - 12
test/Renci.SshNet.Tests/Classes/Common/TimeSpanExtensionsTest.cs

@@ -21,21 +21,17 @@ namespace Renci.SshNet.Tests.Classes.Common
         }
         }
 
 
         [TestMethod]
         [TestMethod]
-        [ExpectedException(typeof(ArgumentOutOfRangeException))]
         public void AsTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException()
         public void AsTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException()
         {
         {
             var timeSpan = TimeSpan.FromSeconds(-1);
             var timeSpan = TimeSpan.FromSeconds(-1);
-
-            timeSpan.AsTimeout();
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() => timeSpan.AsTimeout());
         }
         }
 
 
         [TestMethod]
         [TestMethod]
-        [ExpectedException(typeof(ArgumentOutOfRangeException))]
         public void AsTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException()
         public void AsTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException()
         {
         {
             var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
             var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
-
-            timeSpan.AsTimeout();
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() => timeSpan.AsTimeout());
         }
         }
 
 
         [TestMethod]
         [TestMethod]
@@ -64,21 +60,17 @@ namespace Renci.SshNet.Tests.Classes.Common
         }
         }
 
 
         [TestMethod]
         [TestMethod]
-        [ExpectedException(typeof(ArgumentOutOfRangeException))]
         public void EnsureValidTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException()
         public void EnsureValidTimeout_NegativeTimeSpan_ThrowsArgumentOutOfRangeException()
         {
         {
             var timeSpan = TimeSpan.FromSeconds(-1);
             var timeSpan = TimeSpan.FromSeconds(-1);
-
-            timeSpan.EnsureValidTimeout();
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() => timeSpan.EnsureValidTimeout());
         }
         }
 
 
         [TestMethod]
         [TestMethod]
-        [ExpectedException(typeof(ArgumentOutOfRangeException))]
         public void EnsureValidTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException()
         public void EnsureValidTimeout_TimeSpanExceedingMaxValue_ThrowsArgumentOutOfRangeException()
         {
         {
             var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
             var timeSpan = TimeSpan.FromMilliseconds((double)int.MaxValue + 1);
-
-            timeSpan.EnsureValidTimeout();
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() => timeSpan.EnsureValidTimeout());
         }
         }
 
 
         [TestMethod]
         [TestMethod]

+ 3 - 1
test/Renci.SshNet.Tests/Common/TestMethodForPlatformAttribute.cs

@@ -1,9 +1,11 @@
-using System.Runtime.InteropServices;
+using System;
+using System.Runtime.InteropServices;
 
 
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 
 namespace Renci.SshNet.Tests.Common
 namespace Renci.SshNet.Tests.Common
 {
 {
+    [AttributeUsage(AttributeTargets.Method)]
     public sealed class TestMethodForPlatformAttribute : TestMethodAttribute
     public sealed class TestMethodForPlatformAttribute : TestMethodAttribute
     {
     {
         public TestMethodForPlatformAttribute(string platform)
         public TestMethodForPlatformAttribute(string platform)