{"id":390689,"date":"2024-06-29T09:23:26","date_gmt":"2024-06-29T09:23:26","guid":{"rendered":"http:\/\/savepearlharbor.com\/?p=390689"},"modified":"-0001-11-30T00:00:00","modified_gmt":"-0001-11-29T21:00:00","slug":"","status":"publish","type":"post","link":"https:\/\/savepearlharbor.com\/?p=390689","title":{"rendered":"Errors and suspicious code fragments in .NET 6 sources<\/span>"},"content":{"rendered":"
<\/div>\n
\n
\n
\n
\n

\"0903_NET6\/image1.png\"<\/p>\n

<\/p>\n

The .NET 6 turned out to be much-awaited and major release. If you write for .NET, you could hardly miss such an event. We also couldn’t pass by the new version of this platform. We decided to check what interesting things we can find in the sources of .NET libraries.<\/p>\n

<\/a> <\/p>\n

Details about the check<\/h2>\n

<\/p>\n

I took the sources from the branch of .NET 6 release on GitHub<\/a>. This article covers suspicious places only from the libraries (those that lies in src\/libraries). I didn’t analyze the runtime itself \u2014 maybe next time. \ud83d\ude42<\/p>\n

<\/p>\n

I checked the code with the PVS-Studio static analyzer<\/a>. As you probably guessed from this article, PVS-Studio 7.16 supports the analysis of projects on .NET 6. You can read more about new enhancements of the current release here. The PVS-Studio C# analyzer for Linux and macOS now works on .NET 6 as well.<\/p>\n

<\/p>\n

Over the year, PVS-Studio significantly expanded the functionality of the C# analyzer. In addition to the support of the .NET 6 platform, we added the plugin for Visual Studio 2022 and new security-diagnostics. Besides, we also optimized the C# analyzer’s performance for large projects.<\/p>\n

<\/p>\n

But you came here to read about .NET 6, didn’t you? Let’s not waste time.<\/p>\n

<\/p>\n

Suspicious code fragments<\/h2>\n

<\/p>\n

Miscellaneous<\/h3>\n

<\/p>\n

This section includes various interesting code fragments that I could not group together into common category.<\/p>\n

<\/p>\n

Issue 1<\/strong><\/p>\n

<\/p>\n

Let’s start with something simple.<\/p>\n

<\/p>\n

public enum CompressionLevel {   Optimal,   Fastest,   NoCompression,   SmallestSize }  internal static void GetZipCompressionMethodFromOpcCompressionOption(   CompressionOption compressionOption,   out CompressionLevel compressionLevel) {   switch (compressionOption)   {     case CompressionOption.NotCompressed:       {         compressionLevel = CompressionLevel.NoCompression;       }       break;     case CompressionOption.Normal:       {         compressionLevel = CompressionLevel.Optimal;  \/\/ <=       }       break;     case CompressionOption.Maximum:       {         compressionLevel = CompressionLevel.Optimal;  \/\/ <=       }       break;     case CompressionOption.Fast:       {         compressionLevel = CompressionLevel.Fastest;       }       break;     case CompressionOption.SuperFast:       {         compressionLevel = CompressionLevel.Fastest;       }       break;      \/\/ fall-through is not allowed     default:       {         Debug.Fail(\"Encountered an invalid CompressionOption enum value\");         goto case CompressionOption.NotCompressed;       }   } }<\/code><\/pre>\n
  <\/p>\n
PVS-Studio warning: V3139<\/a> Two or more case-branches perform the same actions. ZipPackage.cs 402<\/p>\n
  <\/p>\n
In fact, this method performs mapping from CompressionOption<\/em> to CompressionLevel<\/em>. The suspicious thing here is that the CompressionOption.Normal<\/em> and CompressionOption.Maximum<\/em> values are mapped to the CompressionLevel.Optimal<\/em> value. <\/p>\n
  <\/p>\n
Probably CompressionOption.Maximum <\/em>should match CompressionLevel.SmallestSize<\/em>.<\/p>\n
  <\/p>\n
Issue 2<\/strong><\/p>\n
  <\/p>\n
Now let’s practice a little. Let’s take the System.Text.Json.Nodes.JsonObject<\/em> for our experiments. If you wish, you can repeat the described operations using the release version of .NET 6 SDK.<\/p>\n
  <\/p>\n
The JsonObject<\/em> type has 2 constructors: one constructor accepts only options, the other \u2014 properties and options. Well, it’s clear what kind of behavior we should expect from them. Documentation is available here<\/a>.<\/p>\n
  <\/p>\n
Let’s create two instances of the JsonObject<\/em> type and use each of the constructors.<\/p>\n
  <\/p>\n
static void JsonObject_Test() {   var properties = new Dictionary<String, JsonNode?>();   var options = new JsonNodeOptions()   {     PropertyNameCaseInsensitive = true   };    var jsonObject1 = new JsonObject(options);   var jsonObject2 = new JsonObject(properties, options); }<\/code><\/pre>\n
  <\/p>\n
Now let’s check the state of the objects we created.<\/p>\n
  <\/p>\n
\"0903_NET6\/image2.png\"<\/p>\n
  <\/p>\n
The jsonObject1<\/em> state is expected, but the jsonObject2<\/em> object state is not. Why the null<\/em> value is written in the _options<\/em> field? It’s a little confusing. Well, let’s open the source code and look at these constructors.<\/p>\n
  <\/p>\n
public sealed partial class JsonObject : JsonNode {   ....   public JsonObject(JsonNodeOptions? options = null) : base(options) { }    public JsonObject(IEnumerable<KeyValuePair<string, JsonNode?>> properties,                      JsonNodeOptions? options = null)   {     foreach (KeyValuePair<string, JsonNode?> node in properties)     {       Add(node.Key, node.Value);     }   }   .... }<\/code><\/pre>\n
  <\/p>\n
In the second constructor, the options<\/em> parameter is simply abandoned \u2014 it is not passed anywhere and is not used in any way. Whereas in the first constructor, options<\/em> are passed to the base class constructor, where they are written to the field:<\/p>\n
  <\/p>\n
internal JsonNode(JsonNodeOptions? options = null) {   _options = options; }<\/code><\/pre>\n
  <\/p>\n
The corresponding PVS-Studio warning: V3117<\/a> Constructor parameter ‘options’ is not used. JsonObject.cs 35<\/p>\n
  <\/p>\n
Issue 3<\/strong><\/p>\n
  <\/p>\n
If we talk about the forgotten parameters, there was another interesting fragment.<\/p>\n
  <\/p>\n
public class ServiceNameCollection : ReadOnlyCollectionBase {   ....   private ServiceNameCollection(IList list, string serviceName)     : this(list, additionalCapacity: 1)   { .... }    private ServiceNameCollection(IList list, IEnumerable serviceNames)     : this(list, additionalCapacity: GetCountOrOne(serviceNames))   { .... }    private ServiceNameCollection(IList list, int additionalCapacity)   {     Debug.Assert(list != null);     Debug.Assert(additionalCapacity >= 0);      foreach (string? item in list)     {       InnerList.Add(item);     }   }   .... }<\/code><\/pre>\n
  <\/p>\n
PVS-Studio warning: V3117<\/a> Constructor parameter ‘additionalCapacity’ is not used. ServiceNameCollection.cs 46<\/p>\n
  <\/p>\n
According to code, the additionalCapacity<\/em> parameter of the last constructor is checked in Debug.Assert<\/em> and not used for anything else. It looks suspicious. It’s especially amusing \u2014 other constructors pass some values for additionalCapacity<\/em> parameter.<\/p>\n
  <\/p>\n
Issue 4<\/strong><\/p>\n
  <\/p>\n
Here’s the test for the ability of foresight (oops, spoilers). Study the following code and try to guess what triggered the analyzer.<\/p>\n
  <\/p>\n
public override void CheckErrors() {   throw new XsltException(SR.Xslt_InvalidXPath,                            new string[] { Expression },                            _baseUri,                            _linePosition,                            _lineNumber,                            null); }<\/code><\/pre>\n
  <\/p>\n
It would seem that an exception is simply thrown. To understand what is wrong here, you need to look at the XsltException<\/em> constructor.<\/p>\n
  <\/p>\n
internal XsltException(string res,                         string?[] args,                         string? sourceUri,                         int lineNumber,                         int linePosition,                         Exception? inner) : base(....) { .... }<\/code><\/pre>\n
  <\/p>\n
If you compare the order of arguments and parameters, it becomes clear what triggered the analyzer. It looks like the line position and the line number switched places.<\/p>\n
  <\/p>\n
Order of arguments:<\/p>\n
  <\/p>\n