Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] make CA2022 an error (#9282)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jonathanpeppers
jonathanpeppers authored Sep 5, 2024
1 parent 87f1cd6 commit ac8a922
Show file tree
Hide file tree
Showing 12 changed files with 39 additions and 83 deletions.
1 change: 1 addition & 0 deletions .editorconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ visual_basic_preferred_modifier_order = Partial,Default,Private,Protected,Public
# Code files
[*.{cs,vb}]

dotnet_diagnostic.CA2022.severity = error # Avoid inexact read with 'System.IO.FileStream.Read(byte[], int, int)'
dotnet_diagnostic.CA2153.severity = error # Do Not Catch Corrupted State Exceptions
dotnet_diagnostic.CA2301.severity = error # Do not call BinaryFormatter.Deserialize without first setting BinaryFormatter.Binder
dotnet_diagnostic.CA2302.severity = error # Ensure BinaryFormatter.Binder is set before calling BinaryFormatter.Deserialize
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/GenerateResourceDesignerAssembly.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ void StrongNameAssembly (AssemblyNameDefinition name)
{
using (Stream stream = typeof (GenerateResourceDesignerAssembly).Assembly.GetManifestResourceStream ("Resource.Designer.snk")) {
byte[] publicKey = new byte[stream.Length];
stream.Read (publicKey, 0, publicKey.Length);
_ = stream.Read (publicKey, 0, publicKey.Length);
name.HashAlgorithm = AssemblyHashAlgorithm.SHA1;
name.PublicKey = SigningHelper.GetPublicKey (publicKey);
name.HasPublicKey = true;
Expand Down
39 changes: 13 additions & 26 deletions ...marin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,9 @@ public void MoveResource ()
{
var proj = new XamarinAndroidApplicationProject ();
BuildItem image = null;
using (var stream = typeof (XamarinAndroidCommonProject).Assembly.GetManifestResourceStream ("Xamarin.ProjectTools.Resources.Base.Icon.png")) {
var image_data = new byte [stream.Length];
stream.Read (image_data, 0, (int)stream.Length);
image = new AndroidItem.AndroidResource ("Resources\\drawable\\Image.png") { BinaryContent = () => image_data };
proj.AndroidResources.Add (image);
}
var image_data = XamarinAndroidCommonProject.GetResourceContents ("Xamarin.ProjectTools.Resources.Base.Icon.png");
image = new AndroidItem.AndroidResource ("Resources\\drawable\\Image.png") { BinaryContent = () => image_data };
proj.AndroidResources.Add (image);
using (var b = CreateApkBuilder ("temp/MoveResource")) {
Assert.IsTrue (b.Build (proj), "First build should have succeeded.");
var oldpath = image.Include ().Replace ('\\', Path.DirectorySeparatorChar);
Expand Down Expand Up @@ -213,14 +210,11 @@ public void RepetiviteBuildUpdateSingleResource ()
var proj = new XamarinAndroidApplicationProject ();
using (var b = CreateApkBuilder ()) {
BuildItem image1, image2;
using (var stream = typeof (XamarinAndroidCommonProject).Assembly.GetManifestResourceStream ("Xamarin.ProjectTools.Resources.Base.Icon.png")) {
var image_data = new byte [stream.Length];
stream.Read (image_data, 0, (int)stream.Length);
image1 = new AndroidItem.AndroidResource ("Resources\\drawable\\Image1.png") { BinaryContent = () => image_data };
proj.AndroidResources.Add (image1);
image2 = new AndroidItem.AndroidResource ("Resources\\drawable\\Image2.png") { BinaryContent = () => image_data };
proj.AndroidResources.Add (image2);
}
var image_data = XamarinAndroidCommonProject.GetResourceContents ("Xamarin.ProjectTools.Resources.Base.Icon.png");
image1 = new AndroidItem.AndroidResource ("Resources\\drawable\\Image1.png") { BinaryContent = () => image_data };
proj.AndroidResources.Add (image1);
image2 = new AndroidItem.AndroidResource ("Resources\\drawable\\Image2.png") { BinaryContent = () => image_data };
proj.AndroidResources.Add (image2);
b.ThrowOnBuildFailure = false;
Assert.IsTrue (b.Build (proj), "First build was supposed to build without errors");
var firstBuildTime = b.LastBuildTime;
Expand Down Expand Up @@ -251,21 +245,14 @@ public void Check9PatchFilesAreProcessed ()
{
var projectPath = Path.Combine ("temp", "Check9PatchFilesAreProcessed");
var libproj = new XamarinAndroidLibraryProject () { ProjectName = "Library1"};
using (var stream = typeof (XamarinAndroidCommonProject).Assembly.GetManifestResourceStream ("Xamarin.ProjectTools.Resources.Base.Image.9.png")) {
var image_data = new byte [stream.Length];
stream.Read (image_data, 0, (int)stream.Length);
var image2 = new AndroidItem.AndroidResource ("Resources\\drawable\\Image2.9.png") { BinaryContent = () => image_data };
libproj.AndroidResources.Add (image2);
}
var image_data = XamarinAndroidCommonProject.GetResourceContents ("Xamarin.ProjectTools.Resources.Base.Image.9.png");
var image2 = new AndroidItem.AndroidResource ("Resources\\drawable\\Image2.9.png") { BinaryContent = () => image_data };
libproj.AndroidResources.Add (image2);
using (var libb = CreateDllBuilder (Path.Combine (projectPath, "Library1"))) {
libb.Build (libproj);
var proj = new XamarinFormsMapsApplicationProject ();
using (var stream = typeof (XamarinAndroidCommonProject).Assembly.GetManifestResourceStream ("Xamarin.ProjectTools.Resources.Base.Image.9.png")) {
var image_data = new byte [stream.Length];
stream.Read (image_data, 0, (int)stream.Length);
var image1 = new AndroidItem.AndroidResource ("Resources\\drawable\\Image1.9.png") { BinaryContent = () => image_data };
proj.AndroidResources.Add (image1);
}
var image1 = new AndroidItem.AndroidResource ("Resources\\drawable\\Image1.9.png") { BinaryContent = () => image_data };
proj.AndroidResources.Add (image1);
proj.References.Add (new BuildItem ("ProjectReference", "..\\Library1\\Library1.csproj"));
using (var b = CreateApkBuilder (Path.Combine (projectPath, "Application1"), false, false)) {
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
Expand Down
6 changes: 1 addition & 5 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AotTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,7 @@ public void BuildBasicApplicationReleaseWithCustomAotProfile ()
};
proj.SetProperty (proj.ActiveConfigurationProperties, "AndroidExtraAotOptions", "--verbose");

byte [] custom_aot_profile;
using (var stream = typeof (XamarinAndroidApplicationProject).Assembly.GetManifestResourceStream ("Xamarin.ProjectTools.Resources.Base.custom.aotprofile")) {
custom_aot_profile = new byte [stream.Length];
stream.Read (custom_aot_profile, 0, (int) stream.Length);
}
byte [] custom_aot_profile = XamarinAndroidCommonProject.GetResourceContents ("Xamarin.ProjectTools.Resources.Base.custom.aotprofile");
proj.OtherBuildItems.Add (new BuildItem ("AndroidAotProfile", "custom.aotprofile") { BinaryContent = () => custom_aot_profile });

using var b = CreateApkBuilder ();
Expand Down
6 changes: 1 addition & 5 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,10 @@ public void BuildReleaseArm64 ([Values (false, true)] bool forms)
proj.SetProperty ("LinkerDumpDependencies", "True");
proj.SetProperty ("AndroidUseAssemblyStore", "False");

byte [] apkDescData;
var flavor = (forms ? "XForms" : "Simple") + "DotNet";
var apkDescFilename = $"BuildReleaseArm64{flavor}.apkdesc";
var apkDescReference = "reference.apkdesc";
using (var stream = typeof (XamarinAndroidApplicationProject).Assembly.GetManifestResourceStream ($"Xamarin.ProjectTools.Resources.Base.{apkDescFilename}")) {
apkDescData = new byte [stream.Length];
stream.Read (apkDescData, 0, (int) stream.Length);
}
byte [] apkDescData = XamarinAndroidCommonProject.GetResourceContents ($"Xamarin.ProjectTools.Resources.Base.{apkDescFilename}");
proj.OtherBuildItems.Add (new BuildItem ("ApkDescFile", apkDescReference) { BinaryContent = () => apkDescData });

// use BuildHelper.CreateApkBuilder so that the test directory is not removed in tearup
Expand Down
6 changes: 1 addition & 5 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ManifestTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,11 +720,7 @@ public void ModifyManifest ([Values (true, false)] bool isRelease)
[Test]
public void MergeLibraryManifest ()
{
byte [] classesJar;
using (var stream = typeof (XamarinAndroidCommonProject).Assembly.GetManifestResourceStream ("Xamarin.ProjectTools.Resources.Base.classes.jar")) {
classesJar = new byte [stream.Length];
stream.Read (classesJar, 0, (int)stream.Length);
}
byte [] classesJar = XamarinAndroidCommonProject.GetResourceContents ("Xamarin.ProjectTools.Resources.Base.classes.jar");
byte [] data;
using (var ms = new MemoryStream ()) {
using (var zip = ZipArchive.Create (ms)) {
Expand Down
7 changes: 2 additions & 5 deletions ...droid.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateResourceCaseMapTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,8 @@ public void CreateResourceDirectory (string path)
Directory.CreateDirectory (Path.Combine (Root, path));
Directory.CreateDirectory (Path.Combine (Root, path, "res", "drawable"));
Directory.CreateDirectory (Path.Combine (Root, path, "res", "values"));
using (var stream = typeof (XamarinAndroidCommonProject).Assembly.GetManifestResourceStream ("Xamarin.ProjectTools.Resources.Base.Icon.png")) {
var icon_binary_mdpi = new byte [stream.Length];
stream.Read (icon_binary_mdpi, 0, (int)stream.Length);
File.WriteAllBytes (Path.Combine (Root, path, "res", "drawable", "IMALLCAPS.png"), icon_binary_mdpi);
}
var icon_binary_mdpi = XamarinAndroidCommonProject.GetResourceContents ("Xamarin.ProjectTools.Resources.Base.Icon.png");
File.WriteAllBytes (Path.Combine (Root, path, "res", "drawable", "IMALLCAPS.png"), icon_binary_mdpi);
}

[Test]
Expand Down
19 changes: 6 additions & 13 deletions ...Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/ManagedResourceParserTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,9 @@ public void CreateResourceDirectory (string path)
File.WriteAllText (Path.Combine (Root, path, "lp", "res", "font", "arial.ttf"), "");
File.WriteAllText (Path.Combine (Root, path, "lp", "res", "values", "strings.xml"), StringsXml2);
File.WriteAllText (Path.Combine (Root, path, "lp", "res", "values", "dimen.xml"), Dimen);
using (var stream = typeof (XamarinAndroidCommonProject).Assembly.GetManifestResourceStream ("Xamarin.ProjectTools.Resources.Base.Icon.png")) {
var icon_binary_mdpi = new byte [stream.Length];
stream.Read (icon_binary_mdpi, 0, (int)stream.Length);
File.WriteAllBytes (Path.Combine (Root, path, "lp", "res", "drawable", "ic_menu_preferences.png"), icon_binary_mdpi);
File.WriteAllBytes (Path.Combine (Root, path, "lp", "res", "mipmap-hdpi", "icon.png"), icon_binary_mdpi);
}
var icon_binary_mdpi = XamarinAndroidCommonProject.GetResourceContents ("Xamarin.ProjectTools.Resources.Base.Icon.png");
File.WriteAllBytes (Path.Combine (Root, path, "lp", "res", "drawable", "ic_menu_preferences.png"), icon_binary_mdpi);
File.WriteAllBytes (Path.Combine (Root, path, "lp", "res", "mipmap-hdpi", "icon.png"), icon_binary_mdpi);
File.WriteAllText (Path.Combine (Root, path, "lp", "res", "menu", "options.xml"), Menu);
File.WriteAllText (Path.Combine (Root, path, "lp", "__res_name_case_map.txt"), "menu/Options.xml;menu/options.xml");
}
Expand All @@ -318,13 +315,9 @@ void BuildLibraryWithResources (string path)
library.AndroidResources.Add (new AndroidItem.AndroidResource (Path.Combine ("Resources", "values", "strings2.xml")) { TextContent = () => StringsXml2 });
library.AndroidResources.Add (new AndroidItem.AndroidResource (Path.Combine ("Resources", "values", "dimen.xml")) { TextContent = () => Dimen });

using (var stream = typeof (XamarinAndroidCommonProject).Assembly.GetManifestResourceStream ("Xamarin.ProjectTools.Resources.Base.Icon.png")) {
var icon_binary_mdpi = new byte [stream.Length];
stream.Read (icon_binary_mdpi, 0, (int)stream.Length);
library.AndroidResources.Add (new AndroidItem.AndroidResource (Path.Combine ("Resources", "drawable", "ic_menu_preferences.png")) { BinaryContent = () => icon_binary_mdpi });
library.AndroidResources.Add (new AndroidItem.AndroidResource (Path.Combine ("Resources", "mipmap-hdpi", "icon.png")) { BinaryContent = () => icon_binary_mdpi });
}

var icon_binary_mdpi = XamarinAndroidCommonProject.GetResourceContents ("Xamarin.ProjectTools.Resources.Base.Icon.png");
library.AndroidResources.Add (new AndroidItem.AndroidResource (Path.Combine ("Resources", "drawable", "ic_menu_preferences.png")) { BinaryContent = () => icon_binary_mdpi });
library.AndroidResources.Add (new AndroidItem.AndroidResource (Path.Combine ("Resources", "mipmap-hdpi", "icon.png")) { BinaryContent = () => icon_binary_mdpi });
library.AndroidResources.Add (new AndroidItem.AndroidResource (Path.Combine ("Resources", "menu", "options.xml")) { TextContent = () => Menu });

using (ProjectBuilder builder = CreateDllBuilder (Path.Combine (Root, path))) {
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/ResourceData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static byte [] GetKeystore (string keyname = "test.keystore")
var assembly = typeof (XamarinAndroidCommonProject).Assembly;
using (var stream = assembly.GetManifestResourceStream ($"Xamarin.ProjectTools.Resources.Base.{keyname}")) {
var data = new byte [stream.Length];
stream.Read (data, 0, (int) stream.Length);
_ = stream.Read (data, 0, (int) stream.Length);
return data;
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...rin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/XamarinAndroidCommonProject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ static XamarinAndroidCommonProject ()
icon_binary_xxxhdpi = GetResourceContents ("mipmap-xxxhdpi/appicon.png");
}

static byte[] GetResourceContents (string resourceName)
public static byte[] GetResourceContents (string resourceName)
{
var assembly = typeof (XamarinAndroidCommonProject).Assembly;
var stream = assembly.GetManifestResourceStream (resourceName) ??
Expand All @@ -36,7 +36,7 @@ static byte[] GetResourceContents (string resourceName)
}
using (stream) {
var contents = new byte [stream.Length];
stream.Read (contents, 0, (int) stream.Length);
_ = stream.Read (contents, 0, (int) stream.Length);
return contents;
}
}
Expand Down
12 changes: 7 additions & 5 deletions src/Xamarin.Android.Build.Tasks/Utilities/AssemblyCompression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,20 @@ public static CompressionResult Compress (AssemblyData data, string outputDirect
// }

data.DestinationPath = Path.Combine (outputDirectory, $"{Path.GetFileName (data.SourcePath)}.lz4");
data.SourceSize = (uint)fi.Length;
data.SourceSize = checked((uint)fi.Length);

int bytesRead;
byte[] sourceBytes = null;
byte[] destBytes = null;
try {
sourceBytes = bytePool.Rent (checked((int)fi.Length));
int fileSize = checked((int)fi.Length);
sourceBytes = bytePool.Rent (fileSize);
using (var fs = File.Open (data.SourcePath, FileMode.Open, FileAccess.Read, FileShare.Read)) {
fs.Read (sourceBytes, 0, (int)fi.Length);
bytesRead = fs.Read (sourceBytes, 0, fileSize);
}

destBytes = bytePool.Rent (LZ4Codec.MaximumOutputSize (sourceBytes.Length));
int encodedLength = LZ4Codec.Encode (sourceBytes, 0, checked((int)fi.Length), destBytes, 0, destBytes.Length, LZ4Level.L12_MAX);
destBytes = bytePool.Rent (LZ4Codec.MaximumOutputSize (bytesRead));
int encodedLength = LZ4Codec.Encode (sourceBytes, 0, bytesRead, destBytes, 0, destBytes.Length, LZ4Level.L12_MAX);
if (encodedLength < 0)
return CompressionResult.EncodingFailed;

Expand Down
18 changes: 3 additions & 15 deletions src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,21 +175,9 @@ public bool Equals (FileInfo x, FileInfo y)
{
if (x.Exists != y.Exists || x.Length != y.Length)
return false;
using (var f1 = File.OpenRead (x.FullName)) {
using (var f2 = File.OpenRead (y.FullName)) {
var b1 = new byte [0x1000];
var b2 = new byte [0x1000];
int total = 0;
while (total < x.Length) {
int size = f1.Read (b1, 0, b1.Length);
total += size;
f2.Read (b2, 0, b2.Length);
if (!b1.Take (size).SequenceEqual (b2.Take (size)))
return false;
}
}
}
return true;
string xHash = Files.HashFile (x.FullName);
string yHash = Files.HashFile (y.FullName);
return xHash == yHash;
}

public int GetHashCode (FileInfo obj)
Expand Down

0 comments on commit ac8a922

Please sign in to comment.