Skip to content

Commit

Permalink
Update RSPEC before 9.32 release (#9632)
Browse files Browse the repository at this point in the history
  • Loading branch information
sebastien-marichal authored Aug 9, 2024
1 parent 1a49bd3 commit b9e9573
Show file tree
Hide file tree
Showing 22 changed files with 322 additions and 159 deletions.
2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S2053.html
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ <h4>Compliant solution</h4>

public static void hash(string password)
{
var hashed = new Rfc2898DeriveBytes(password, 16);
var hashed = new Rfc2898DeriveBytes(password, 32, 10000, HashAlgorithmName.SHA256);
}
</pre>
<h3>How does this work?</h3>
Expand Down
3 changes: 1 addition & 2 deletions analyzers/rspec/cs/S2245.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ <h2>See</h2>
<li> OWASP - <a href="https://owasp.org/Top10/A02_2021-Cryptographic_Failures/">Top 10 2021 Category A2 - Cryptographic Failures</a> </li>
<li> OWASP - <a href="https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure">Top 10 2017 Category A3 - Sensitive Data
Exposure</a> </li>
<li> <a href="https://mobile-security.gitbook.io/masvs/security-requirements/0x08-v3-cryptography_verification_requirements">Mobile AppSec
Verification Standard - Cryptography Requirements</a> </li>
<li> <a href="https://mas.owasp.org/checklists/MASVS-CRYPTO/">Mobile AppSec Verification Standard - Cryptography Requirements</a> </li>
<li> OWASP - <a href="https://owasp.org/www-project-mobile-top-10/2016-risks/m5-insufficient-cryptography">Mobile Top 10 2016 Category M5 -
Insufficient Cryptography</a> </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/338">CWE-338 - Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)</a>
Expand Down
58 changes: 38 additions & 20 deletions analyzers/rspec/cs/S2325.html
Original file line number Diff line number Diff line change
@@ -1,11 +1,37 @@
<h2>Why is this an issue?</h2>
<p>Methods and properties that don’t access instance data can be <code>static</code> to prevent any misunderstanding about the contract of the
method.</p>
<h3>Noncompliant code example</h3>
<pre>
<p>Methods and properties that don’t access instance data should be marked as <code>static</code> for the following reasons:</p>
<ul>
<li> Clarity and Intent: Marking a method/property as static makes it clear that the method does not depend on instance data and can be called
without creating an instance of the class. This improves the readability of the code by clearly conveying the member’s intended use. </li>
<li> Performance: Instance methods/properties in C# require an instance of the class to be called. This means that even if the it doesn’t use any
instance data, the runtime still needs to pass a reference to the instance during the call. For static methods and properties, this overhead is
avoided, leading to slightly better performance. </li>
<li> Memory Usage: Since instance methods implicitly carry a reference to the instance (the caller object), they can potentially prevent the garbage
collector from collecting the instance whem it is not otherwise referenced. Static members do not carry this overhead, potentially reducing memory
usage. </li>
<li> Testing: Static members can be easier to test since they do not require an instance of the class. This can simplify unit testing and reduce the
amount of boilerplate code needed to set up tests. </li>
</ul>
<h3>Exceptions</h3>
<p>Methods with the following names are excluded because they can’t be made <code>static</code>:</p>
<ul>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.web.httpapplication.authenticaterequest">Application_AuthenticateRequest</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.web.httpapplication.beginrequest">Application_BeginRequest</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/previous-versions/aspnet/ms178473(v=vs.100)">Application_End</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.web.httpapplication.endrequest">Application_EndRequest</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/previous-versions/aspnet/24395wz3(v=vs.100)">Application_Error</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/previous-versions/aspnet/ms178473(v=vs.100)">Application_Init</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/previous-versions/aspnet/ms178473(v=vs.100)">Application_Start</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.web.sessionstate.sessionstatemodule.end">Session_End</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.web.sessionstate.sessionstatemodule.start">Session_Start</a> </li>
</ul>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
public class Utilities
{
public int MagicNum // Noncompliant
public int MagicNum // Noncompliant - only returns a constant value
{
get
{
Expand All @@ -14,7 +40,7 @@ <h3>Noncompliant code example</h3>
}

private static string magicWord = "please";
public string MagicWord // Noncompliant
public string MagicWord // Noncompliant - only accesses a static field
{
get
{
Expand All @@ -26,14 +52,14 @@ <h3>Noncompliant code example</h3>
}
}

public int Sum(int a, int b) // Noncompliant
public int Sum(int a, int b) // Noncompliant - doesn't access instance data, only the method parameters
{
return a + b;
}
}
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
public class Utilities
{
public static int MagicNum
Expand Down Expand Up @@ -63,17 +89,9 @@ <h3>Compliant solution</h3>
}
}
</pre>
<h3>Exceptions</h3>
<p>Methods with the following names are excluded because they can’t be made <code>static</code>:</p>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Application_AuthenticateRequest </li>
<li> Application_BeginRequest </li>
<li> Application_End </li>
<li> Application_EndRequest </li>
<li> Application_Error </li>
<li> Application_Init </li>
<li> Application_Start </li>
<li> Session_End </li>
<li> Session_Start </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/static">The static modifier</a> </li>
</ul>

65 changes: 40 additions & 25 deletions analyzers/rspec/cs/S2674.html
Original file line number Diff line number Diff line change
@@ -1,38 +1,53 @@
<h2>Why is this an issue?</h2>
<p>You cannot assume that any given stream reading call will fill the <code>byte[]</code> passed in to the method with the number of bytes requested.
Instead, you must check the value returned by the read method to see how many bytes were read. Fail to do so, and you introduce a bug that is both
harmful and difficult to reproduce.</p>
<p>This rule raises an issue when a <code>Stream.Read</code> or a <code>Stream.ReadAsync</code> method is called, but the return value is not
checked.</p>
<h3>Noncompliant code example</h3>
<pre>
public void DoSomething(string fileName)
<p>Invoking a stream reading method without verifying the number of bytes read can lead to erroneous assumptions. A Stream can represent any I/O
operation, such as reading a file, network communication, or inter-process communication. As such, it is not guaranteed that the <code>byte[]</code>
passed into the method will be filled with the requested number of bytes. Therefore, inspecting the value returned by the reading method is important
to ensure the number of bytes read.</p>
<p>Neglecting the returned length read can result in a bug that is difficult to reproduce.</p>
<p>This rule raises an issue when the returned value is ignored for the following methods:</p>
<ul>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.read">Stream.Read</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readasync">Stream.ReadAsync</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readatleast">Stream.ReadAtLeast</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readatleastasync">Stream.ReadAtLeastAsync</a> </li>
</ul>
<h2>How to fix it</h2>
<p>Check the return value of stream reading methods to verify the actual number of bytes read, and use this value when processing the data to avoid
potential bugs.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
public byte[] ReadFile(string fileName)
{
using (var stream = File.Open(fileName, FileMode.Open))
{
using var stream = File.Open(fileName, FileMode.Open);
var result = new byte[stream.Length];

stream.Read(result, 0, (int)stream.Length); // Noncompliant
// ... do something with result
}

return result;
}
</pre>
<h3>Compliant solution</h3>
<pre>
public void DoSomething(string fileName)
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
public byte[] ReadFile(string fileName)
{
using (var stream = File.Open(fileName, FileMode.Open))
{
using var stream = File.Open(fileName, FileMode.Open);
using var ms = new MemoryStream();
var buffer = new byte[1024];
using (var ms = new MemoryStream())
int read;

while ((read = stream.Read(buffer, 0, buffer.Length)) &gt; 0)
{
int read;
while ((read = stream.Read(buffer, 0, buffer.Length)) &gt; 0)
{
ms.Write(buffer, 0, read);
}
// ... do something with ms
ms.Write(buffer, 0, read);
}
}

return ms.ToArray();
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.read">Stream.Read Method</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readasync">Stream.ReadAsync Method</a> </li>
</ul>

106 changes: 83 additions & 23 deletions analyzers/rspec/cs/S3431.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,100 @@ <h2>Why is this an issue?</h2>
<p>It should be clear to a casual reader what code a test is testing and what results are expected. Unfortunately, that’s not usually the case with
the <code>ExpectedException</code> attribute since an exception could be thrown from almost any line in the method.</p>
<p>This rule detects MSTest and NUnit <code>ExpectedException</code> attribute.</p>
<h3>Noncompliant code example</h3>
<h3>Exceptions</h3>
<p>This rule ignores:</p>
<ul>
<li> single-line tests, since it is obvious in such methods where the exception is expected to be thrown </li>
<li> tests when it tests control flow and assertion are present in either a <code>catch</code> or <code>finally</code> clause </li>
</ul>
<pre>
[TestMethod]
[ExpectedException(typeof(ArgumentNullException))] // Noncompliant
public void TestNullArg()
[ExpectedException(typeof(InvalidOperationException))]
public void UsingTest()
{
//...
Console.ForegroundColor = ConsoleColor.Black;
try
{
using var _ = new ConsoleAlert();
Assert.AreEqual(ConsoleColor.Red, Console.ForegroundColor);
throw new InvalidOperationException();
}
finally
{
Assert.AreEqual(ConsoleColor.Black, Console.ForegroundColor); // The exception itself is not relevant for the test.
}
}

public sealed class ConsoleAlert : IDisposable
{
private readonly ConsoleColor previous;

public ConsoleAlert()
{
previous = Console.ForegroundColor;
Console.ForegroundColor = ConsoleColor.Red;
}

public void Dispose() =&gt;
Console.ForegroundColor = previous;
}
</pre>
<h3>Compliant solution</h3>
<pre>
<h2>How to fix it in MSTest</h2>
<p>Remove the <code>ExpectedException</code> attribute in favor of using the <a
href="https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.testtools.unittesting.assert.throwsexception">Assert.ThrowsException</a>
assertion.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
[TestMethod]
public void TestNullArg()
[ExpectedException(typeof(ArgumentNullException))] // Noncompliant
public void Method_NullParam()
{
bool callFailed = false;
try
{
//...
}
catch (ArgumentNullException)
{
callFailed = true;
}
Assert.IsTrue(callFailed, "Expected call to MyMethod to fail with ArgumentNullException");
var sut = new MyService();
sut.Method(null);
}
</pre>
<p>or</p>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
[TestMethod]
public void TestNullArg()
public void Method_NullParam()
{
Assert.ThrowsException&lt;ArgumentNullException&gt;(() =&gt; /*...*/);
var sut = new MyService();
Assert.ThrowsException&lt;ArgumentNullException&gt;(() =&gt; sut.Method(null));
}
</pre>
<h3>Exceptions</h3>
<p>This rule ignores one-line test methods, since it is obvious in such methods where the exception is expected to be thrown.</p>
<h2>How to fix it in NUnit</h2>
<p>Remove the <code>ExpectedException</code> attribute in favor of using the <a
href="https://docs.nunit.org/articles/nunit/writing-tests/assertions/classic-assertions/Assert.Throws.html">Assert.Throws</a> assertion.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="2" data-diff-type="noncompliant">
[Test]
[ExpectedException(typeof(ArgumentNullException))] // Noncompliant
public void Method_NullParam()
{
var sut = new MyService();
sut.Method(null);
}
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="2" data-diff-type="compliant">
[Test]
public void Method_NullParam()
{
var sut = new MyService();
Assert.Throws&lt;ArgumentNullException&gt;(() =&gt; sut.Method(null));
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.testtools.unittesting.assert.throwsexception">Assert.ThrowsException
Method</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.testtools.unittesting.expectedexceptionattribute">ExpectedExceptionAttribute Class</a> </li>
<li> NUnit - <a href="https://docs.nunit.org/articles/nunit/writing-tests/assertions/classic-assertions/Assert.Throws.html">Assert.Throws</a> </li>
<li> NUnit - <a href="https://docs.nunit.org/2.4/exception.html">ExpectedExceptionAttribute</a> </li>
</ul>

Loading

0 comments on commit b9e9573

Please sign in to comment.