dotnet / android

.NET for Android provides open-source bindings of the Android SDK for use with .NET managed languages such as C#
MIT License
1.93k stars 530 forks source link

[Xamarin.Android.Build.Tasks] make CA2022 an error #9282

Closed jonathanpeppers closed 2 months ago

jonathanpeppers commented 2 months ago

I was building Xamarin.Android.Build.Tasks and noticed some CA2022 warnings that scared me:

src\Xamarin.Android.Build.Tasks\Utilities\AssemblyCompression.cs(77,6):
warning CA2022: Avoid inexact read with 'System.IO.FileStream.Read(byte[], int, int)' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2022)
src\Xamarin.Android.Build.Tasks\Utilities\MonoAndroidHelper.cs(186,8):
warning CA2022: Avoid inexact read with 'System.IO.FileStream.Read(byte[], int, int)' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2022)

Some cases like this were actually ok:

byte[] publicKey = new byte[stream.Length];
stream.Read (publicKey, 0, publicKey.Length);
// ...
name.PublicKey = SigningHelper.GetPublicKey (publicKey);

Because it uses stream.Length for the byte[] size, we don't need to use the return value of Read().

Looking at another place, however:

sourceBytes = bytePool.Rent (checked((int)fi.Length));
// ...
    fs.Read (sourceBytes, 0, (int)fi.Length);
// ...
destBytes = bytePool.Rent (LZ4Codec.MaximumOutputSize (sourceBytes.Length));

This actually is a bug, as it rents a destBytes array potentially a bit larger than bytes read.

This made me think, we should make CA2022 an error and fix them all.

I also updated MonoAndroidHelper.SizeAndContentFileComparer to just use the Files.HashFile() method. This is probably faster than the previous code, anyway.

jonathanpeppers commented 2 months ago

There are some APK tests that failed with:

System.Net.Http.HttpRequestException : net_http_message_not_success_statuscode_reason, 503, Service Temporarily Unavailable

But I don't think these changes could affect this, so going to merge.