Skip to content
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

Modifiy S2674: Promote it to SonarWay #4120

Merged
merged 2 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 1 addition & 4 deletions rules/S2674/csharp/metadata.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
{
"title": "The length returned from a stream read should be checked",
"defaultQualityProfiles": [

]
"title": "The length returned from a stream read should be checked"
}
62 changes: 39 additions & 23 deletions rules/S2674/csharp/rule.adoc
Original file line number Diff line number Diff line change
@@ -1,47 +1,63 @@
== Why is this an issue?

You cannot assume that any given stream reading call will fill the ``++byte[]++`` 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.
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, reading a file, network communication or inter-process communication, as such it is not guaranteed that the `byte[]` passed into the method will be filled with the requested number of bytes. Therefore, it is crucial to inspect the value returned by the read method to ascertain the actual number of bytes read.

Neglecting the returned length read can result in a bug that is difficult to reproduce.

This rule raises an issue when a ``++Stream.Read++`` or a ``++Stream.ReadAsync++`` method is called, but the return value is not checked.
This rule raises an issue when the returned value is ignored for the following methods:
* https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.read[Stream.Read]
* https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readasync[Stream.ReadAsync]
* https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readatleast[Stream.ReadAtLeast]
* https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readatleastasync[Stream.ReadAtLeastAsync]

=== Noncompliant code example
== How to fix it

[source,csharp]
Ensure to check the return value of `Stream.Read` methods to verify the actual number of bytes read, and use this value when processing the data to avoid potential bugs.

=== Code examples

==== Noncompliant code example

[source,csharp,diff-id=1,diff-type=noncompliant]
----
public void DoSomething(string fileName)
public byte[] DoSomething(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;
}
----

=== Compliant solution
==== Compliant solution

[source,csharp]
[source,csharp,diff-id=1,diff-type=compliant]
----
public void DoSomething(string fileName)
byte[] DoSomething(string fileName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
byte[] DoSomething(string fileName)
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)) > 0)
{
int read;
while ((read = stream.Read(buffer, 0, buffer.Length)) > 0)
{
ms.Write(buffer, 0, read);
}
// ... do something with ms
}
}
ms.Write(buffer, 0, read);
}

return ms.ToArray();
}
----

== Resources

=== Documentation

* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.read[Stream.Read Method]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readasync[Stream.ReadAsync Method]


ifdef::env-github,rspecator-view[]

Expand Down