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

Fix CA2022 warnings (Avoid inexact read with 'Stream.Read') #100352

Merged
merged 7 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
3 changes: 3 additions & 0 deletions eng/CodeAnalysis.src.globalconfig
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,9 @@ dotnet_diagnostic.CA2020.severity = warning
# CA2021: Do not call Enumerable.Cast<T> or Enumerable.OfType<T> with incompatible types
dotnet_diagnostic.CA2021.severity = warning

# CA2022: Avoid inexact read with 'Stream.Read'
dotnet_diagnostic.CA2022.severity = warning

# CA2100: Review SQL queries for security vulnerabilities
dotnet_diagnostic.CA2100.severity = none

Expand Down
3 changes: 3 additions & 0 deletions eng/CodeAnalysis.test.globalconfig
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,9 @@ dotnet_diagnostic.CA2020.severity = none
# CA2021: Do not call Enumerable.Cast<T> or Enumerable.OfType<T> with incompatible types
dotnet_diagnostic.CA2021.severity = none

# CA2022: Avoid inexact read with 'Stream.Read'
dotnet_diagnostic.CA2022.severity = none

# CA2100: Review SQL queries for security vulnerabilities
dotnet_diagnostic.CA2100.severity = none

Expand Down
16 changes: 15 additions & 1 deletion src/libraries/System.IO.Ports/src/System/IO/Ports/SerialPort.cs
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,21 @@ public string ReadExisting()
Buffer.BlockCopy(_inBuffer, _readPos, bytesReceived, 0, CachedBytesToRead);
}

_internalSerialStream.Read(bytesReceived, CachedBytesToRead, bytesReceived.Length - (CachedBytesToRead)); // get everything
#if NET7_0_OR_GREATER
_internalSerialStream.ReadExactly(bytesReceived, CachedBytesToRead, bytesReceived.Length - (CachedBytesToRead)); // get everything
Copy link
Member

Choose a reason for hiding this comment

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

This won't compile as written. This library builds for multiple target frameworks, some of which don't have ReadExactly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've added a check for NET7_0_OR_GREATER before using ReadExactly.

Is there a build target to check locally if I've used a function that is not available in a target framework? I ran the tests locally with .\build.cmd -subset clr+mono+libs+libs.tests -test -rc Release but got no build error.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. That will only end up building for the current platform. There used to be a command for building all flavors of all libraries, but I can't find it in the docs now. @ViktorHofer? In the meantime, you can cd into the src folder for the library and dotnet build will build all targets for it.

Copy link
Member

Choose a reason for hiding this comment

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

build.cmd libs -allconfigurations

buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
#else
int readCount = bytesReceived.Length - CachedBytesToRead;
int totalRead = 0;
while (totalRead < readCount)
{
int bytesRead = _internalSerialStream.Read(bytesReceived, CachedBytesToRead + totalRead, readCount - totalRead);
if (bytesRead <= 0)
{
throw new EndOfStreamException();
}
totalRead += bytesRead;
}
#endif

// Read full characters and leave partial input in the buffer. Encoding.GetCharCount doesn't work because
// it returns fallback characters on partial input, meaning that it overcounts. Instead, we use
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,21 @@
_bufferState = BufferState.Reading;
_buffer = new byte[_stream.Length];
_stream.Position = 0;
_stream.Read(_buffer, 0, _buffer.Length);

#if NET7_0_OR_GREATER
_stream.ReadExactly(_buffer);
Copy link
Member

Choose a reason for hiding this comment

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

Same here

#else
int totalRead = 0;
while (totalRead < _stream.Length)
{
int bytesRead = _stream.Read(_buffer, totalRead, _stream.Length - totalRead);

Check failure on line 96 in src/libraries/System.ServiceModel.Syndication/src/System/ServiceModel/XmlBuffer.cs

View check run for this annotation

Azure Pipelines / runtime-dev-innerloop (Build linux-x64 debug Libraries_AllConfigurations)

src/libraries/System.ServiceModel.Syndication/src/System/ServiceModel/XmlBuffer.cs#L96

src/libraries/System.ServiceModel.Syndication/src/System/ServiceModel/XmlBuffer.cs(96,66): error CS1503: (NETCORE_ENGINEERING_TELEMETRY=Build) Argument 3: cannot convert from 'long' to 'int'

Check failure on line 96 in src/libraries/System.ServiceModel.Syndication/src/System/ServiceModel/XmlBuffer.cs

View check run for this annotation

Azure Pipelines / runtime-dev-innerloop

src/libraries/System.ServiceModel.Syndication/src/System/ServiceModel/XmlBuffer.cs#L96

src/libraries/System.ServiceModel.Syndication/src/System/ServiceModel/XmlBuffer.cs(96,66): error CS1503: (NETCORE_ENGINEERING_TELEMETRY=Build) Argument 3: cannot convert from 'long' to 'int'
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
if (bytesRead <= 0)
{
throw new EndOfStreamException();
}
totalRead += bytesRead;
}
#endif

_writer = null;
_stream = null;
Expand Down
17 changes: 16 additions & 1 deletion src/libraries/System.Speech/src/Internal/Synthesis/AudioBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,22 @@ internal void PlayWaveFile(AudioData audio)
try
{
byte[] data = new byte[(int)audio._stream.Length];
audio._stream.Read(data, 0, data.Length);

#if NET7_0_OR_GREATER
audio._stream.ReadExactly(data);
Copy link
Member

Choose a reason for hiding this comment

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

And here

#else
int totalRead = 0;
while (totalRead < audio._stream.Length)
{
int bytesRead = audio._stream.Read(data, totalRead, audio._stream.Length - totalRead);
Copy link
Member

Choose a reason for hiding this comment

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

NIT:

Suggested change
while (totalRead < audio._stream.Length)
{
int bytesRead = audio._stream.Read(data, totalRead, audio._stream.Length - totalRead);
while (totalRead < data.Length)
{
int bytesRead = audio._stream.Read(data, totalRead, data.Length - totalRead);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I will use data.Length here, but I am not sure why this does not trigger CS1503 as well.

if (bytesRead <= 0)
{
throw new EndOfStreamException();
}
totalRead += bytesRead;
}
#endif

Play(data);
}
finally
Expand Down
17 changes: 16 additions & 1 deletion src/libraries/System.Speech/src/Internal/Synthesis/EngineSite.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,22 @@ public Stream LoadResource(Uri uri, string mediaType)
int cLen = (int)stream.Length;
MemoryStream memStream = new(cLen);
byte[] ab = new byte[cLen];
stream.Read(ab, 0, ab.Length);

#if NET7_0_OR_GREATER
stream.ReadExactly(ab);
Copy link
Member

Choose a reason for hiding this comment

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

Basically all of them :)

#else
int totalRead = 0;
while (totalRead < cLen)
{
int bytesRead = stream.Read(ab, totalRead, cLen - totalRead);
if (bytesRead <= 0)
{
throw new EndOfStreamException();
}
totalRead += bytesRead;
}
#endif

_resourceLoader.UnloadFile(localPath);
memStream.Write(ab, 0, cLen);
memStream.Position = 0;
Expand Down
Loading