Add Roslyn analyzers and fix some warnings

TBCBank · Jan 14, 2023 · 9a9aacd · 9a9aacd
1 parent c557376
commit 9a9aacd
Show file tree

Hide file tree

Showing 13 changed files with 97 additions and 36 deletions.
diff --git a/src/TBC.OpenBanking.Jws.Http/src/GlobalSuppressions.cs b/src/TBC.OpenBanking.Jws.Http/src/GlobalSuppressions.cs
@@ -0,0 +1,9 @@
+// This file is used by Code Analysis to maintain SuppressMessage
+// attributes that are applied to this project.
+// Project-level suppressions either have no target or are given
+// a specific target and scoped to a namespace, type, member, etc.
+
+using System.Diagnostics.CodeAnalysis;
+
+[assembly: SuppressMessage("Design", "MA0026:Fix TODO comment", Justification = "Reviewed", Scope = "module")]
+[assembly: SuppressMessage("Design", "MA0051:Method is too long", Justification = "Reviewed", Scope = "module")]
diff --git a/src/TBC.OpenBanking.Jws.Http/src/JwsMessageHandler.cs b/src/TBC.OpenBanking.Jws.Http/src/JwsMessageHandler.cs
@@ -48,6 +48,7 @@ namespace TBC.OpenBanking.Jws.Http;
 public sealed class JwsMessageHandler : DelegatingHandler
 {
     private readonly IOptions<JwsClientOptions> jwsOptions;
+    [SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "This class does not own this certificate")]
     private readonly X509Certificate2? signerCertificate;
 
     private readonly HttpSigner<HttpRequestData>? reqSign;
@@ -113,7 +114,7 @@ loggerFactory is null
                     RevocationMode = this.jwsOptions.Value.CheckCertificateRevocationList
                         ? X509RevocationMode.Online
                         : X509RevocationMode.NoCheck,
-                }
+                },
             };
         }
     }
@@ -151,7 +152,7 @@ private async Task<HttpRequestMessage> ProcessRequestAsync(
             httpData.AddHeader("Host", httpData.Uri!.DnsSafeHost);
         }
 
-        httpData.AppendHeaders(request.Headers, true);
+        httpData.AppendHeaders(request.Headers, acceptMultivalue: true);
 
         if (request.Content is not null)
         {
@@ -163,7 +164,7 @@ private async Task<HttpRequestMessage> ProcessRequestAsync(
             httpData.Body = await request.Content!.ReadAsByteArrayAsync().ConfigureAwait(false);
 #endif
 
-            httpData.AppendHeaders(request.Content!.Headers, true);
+            httpData.AppendHeaders(request.Content!.Headers, acceptMultivalue: true);
         }
 
         if (this.reqSign!.CreateSignature(httpData))
@@ -196,14 +197,14 @@ private async Task<HttpResponseMessage> ProcessResponseAsync(
         {
             var httpData = new HttpResponseData
             {
-                StatusCode = ((uint)response.StatusCode).ToString(CultureInfo.InvariantCulture)
+                StatusCode = ((uint)response.StatusCode).ToString(CultureInfo.InvariantCulture),
             };
 
-            httpData.AppendHeaders(response.Headers, true);
+            httpData.AppendHeaders(response.Headers, acceptMultivalue: true);
 
             if (response.Content != null)
             {
-                httpData.AppendHeaders(response.Content!.Headers, true);
+                httpData.AppendHeaders(response.Content!.Headers, acceptMultivalue: true);
 
                 // This is ugly, but there's no better way (so far):
 

diff --git a/src/TBC.OpenBanking.Jws.Http/src/TBC.OpenBanking.Jws.Http.csproj b/src/TBC.OpenBanking.Jws.Http/src/TBC.OpenBanking.Jws.Http.csproj
@@ -45,9 +45,21 @@
     </PropertyGroup>
 
     <ItemGroup>
+        <Using Include="System.Diagnostics.CodeAnalysis" />
         <Using Include="System.Globalization.CultureInfo" Alias="CultureInfo" />
     </ItemGroup>
 
+    <ItemGroup>
+        <PackageReference Include="Meziantou.Analyzer" Version="2.0.*">
+            <PrivateAssets>all</PrivateAssets>
+            <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+        </PackageReference>
+        <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeStyle" Version="4.*">
+            <PrivateAssets>all</PrivateAssets>
+            <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+        </PackageReference>
+    </ItemGroup>
+
     <ItemGroup Condition=" '$(Configuration)' == 'Release' ">
         <PackageReference Include="MinVer" Version="4.2.0">
             <PrivateAssets>all</PrivateAssets>

diff --git a/src/TBC.OpenBanking.Jws.Http/src/X509CertificateLocator.cs b/src/TBC.OpenBanking.Jws.Http/src/X509CertificateLocator.cs
@@ -199,7 +199,7 @@ public X509CertificateLocator(Uri? uri)
             var storeLocation = Enum.Parse<StoreLocation>(WebUtility.UrlDecode(_uri.Segments[1].Trim(TrimSlashes).Trim()), true);
             using var store = new X509Store(storeName, storeLocation, OpenFlags.OpenExistingOnly | OpenFlags.ReadOnly);
 #else
-            var storeLocation = (StoreLocation)Enum.Parse(typeof(StoreLocation), WebUtility.UrlDecode(_uri.Segments[1].Trim(TrimSlashes).Trim()), true);
+            var storeLocation = (StoreLocation)Enum.Parse(typeof(StoreLocation), WebUtility.UrlDecode(_uri.Segments[1].Trim(TrimSlashes).Trim()), ignoreCase: true);
             using var store = new X509Store(storeName, storeLocation);
             store.Open(OpenFlags.OpenExistingOnly | OpenFlags.ReadOnly);
 #endif

diff --git a/src/TBC.OpenBanking.Jws/src/Exceptions/CertificateValidationException.cs b/src/TBC.OpenBanking.Jws/src/Exceptions/CertificateValidationException.cs
@@ -23,6 +23,7 @@
 namespace TBC.OpenBanking.Jws.Exceptions;
 
 using System;
+using System.Runtime.Serialization;
 using System.Security.Cryptography.X509Certificates;
 
 [Serializable]
@@ -35,6 +36,7 @@ public CertificateValidationException(string message)
         : base(message)
     {
         this.SetHResult(ErrorCode);
+        this.message = message;
     }
 
     public CertificateValidationException(X509ChainStatus[] statuses, string message)
@@ -70,10 +72,34 @@ public CertificateValidationException(string message, Exception innerException)
         : base(message, innerException)
     {
         this.SetHResult(ErrorCode);
+        this.message = message;
     }
 
     public CertificateValidationException()
     {
         this.SetHResult(ErrorCode);
     }
+
+    protected CertificateValidationException(SerializationInfo info, StreamingContext context)
+        : base(info, context)
+    {
+        if (info == null)
+        {
+            throw new ArgumentNullException(nameof(info));
+        }
+
+        this.message = info.GetString("message2");
+        this.SetHResult(ErrorCode);
+    }
+
+    public override void GetObjectData(SerializationInfo info, StreamingContext context)
+    {
+        if (info == null)
+        {
+            throw new ArgumentNullException(nameof(info));
+        }
+
+        info.AddValue("message2", this.message, typeof(string));
+        base.GetObjectData(info, context);
+    }
 }
diff --git a/src/TBC.OpenBanking.Jws/src/GlobalSuppressions.cs b/src/TBC.OpenBanking.Jws/src/GlobalSuppressions.cs
@@ -10,3 +10,5 @@
 [assembly: SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores", Justification = "Reviewed", Scope = "module")]
 [assembly: SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Reviewed", Scope = "module")]
 [assembly: SuppressMessage("Performance", "CA1848:Use the LoggerMessage delegates", Justification = "Reviewed", Scope = "module")]
+[assembly: SuppressMessage("Design", "MA0026:Fix TODO comment", Justification = "Reviewed", Scope = "module")]
+[assembly: SuppressMessage("Design", "MA0051:Method is too long", Justification = "Reviewed", Scope = "module")]
diff --git a/src/TBC.OpenBanking.Jws/src/HttpDigest.cs b/src/TBC.OpenBanking.Jws/src/HttpDigest.cs
@@ -73,7 +73,7 @@ internal static HttpDigest CreateDigest(string digestString)
             throw new ArgumentOutOfRangeException(nameof(digestString), "Bad format of digest string. Can't find algorithm prefix");
 
         var algName = digestString.Substring(0, dividerIndex).Trim();
-        var element = supportedAlgorithms.Find(x => x.Prefix == algName);
+        var element = supportedAlgorithms.Find(x => string.Equals(x.Prefix, algName, StringComparison.Ordinal));
         if (element == default)
             throw new ArgumentOutOfRangeException(nameof(digestString), "Unsupported hash algorithm");
 

diff --git a/src/TBC.OpenBanking.Jws/src/HttpRequestData.cs b/src/TBC.OpenBanking.Jws/src/HttpRequestData.cs
@@ -64,8 +64,11 @@ public class HttpRequestData : HttpMessageData
     /// Special header name "(request-target)" is also acceptable and will be processed accordingly.</param>
     /// <param name="additionalHeaders">Additional header name-values</param>
     /// <returns></returns>
+    [SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "Intentional")]
     public override string ComposeHeadersForSignature(IList<string> headers, IDictionary<string, string> additionalHeaders = null)
     {
+        _ = headers ?? throw new ArgumentNullException(nameof(headers));
+
         using var sb = new ValueStringBuilder(initialCapacity: 512);
         foreach (var hn in headers)
         {
@@ -165,6 +168,7 @@ public override void CheckMandatoryHeaders()
         }
     }
 
+    [SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "Intentional")]
     public override List<string> GetHeaderNamesForSignature()
     {
         var headersList = new List<string>(NecessaryHeaders.Count);

diff --git a/src/TBC.OpenBanking.Jws/src/HttpResponseData.cs b/src/TBC.OpenBanking.Jws/src/HttpResponseData.cs
@@ -93,6 +93,7 @@ public override string ComposeHeadersForSignature(IList<string> headers, IDictio
         return sb.ToString();
     }
 
+    [SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "Intentional")]
     public override List<string> GetHeaderNamesForSignature()
     {
         var headersList = new List<string>(NecessaryHeaders.Count);

diff --git a/src/TBC.OpenBanking.Jws/src/HttpSignatureVerifier.cs b/src/TBC.OpenBanking.Jws/src/HttpSignatureVerifier.cs
@@ -63,6 +63,8 @@ private void InitData()
     /// <returns></returns>
     public bool VerifySignature(T httpData, DateTime checkTime)
     {
+        _ = httpData ?? throw new ArgumentNullException(nameof(httpData));
+
         IsSignatureVerified = false;
 
         // Check headers. If any is missing throws exception

diff --git a/src/TBC.OpenBanking.Jws/src/Internals/Helper.cs b/src/TBC.OpenBanking.Jws/src/Internals/Helper.cs
@@ -29,36 +29,28 @@ namespace TBC.OpenBanking.Jws;
 
 static internal class Helper
 {
-    private static readonly JsonSerializerOptions Options;
-
-    static Helper()
+    // These options match Newtonsoft.Json's defaults, more or less
+    private static readonly JsonSerializerOptions s_options = new(JsonSerializerDefaults.Web)
     {
-        // These options match Newtonsoft.Json defaults, more or less.
-
-        var options = new JsonSerializerOptions(JsonSerializerDefaults.Web)
-        {
-            AllowTrailingCommas         = true,
-            DefaultBufferSize           = 81920,  // Keeping under large object heap treshold (85K)
-            MaxDepth                    = 128,  // Newtonsoft has no limit on this
-            DictionaryKeyPolicy         = JsonNamingPolicy.CamelCase,
-            Encoder                     = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
-            IncludeFields               = false,
-            NumberHandling              = JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.AllowNamedFloatingPointLiterals,
-            PropertyNameCaseInsensitive = true,
-            PropertyNamingPolicy        = JsonNamingPolicy.CamelCase,
-            ReadCommentHandling         = JsonCommentHandling.Skip,
-            WriteIndented               = false,
-        };
-
-        Options = options;
-    }
+        AllowTrailingCommas         = true,
+        DefaultBufferSize           = 81920,  // Keeping under large object heap treshold (85K)
+        MaxDepth                    = 128,  // Newtonsoft has no limit on this
+        DictionaryKeyPolicy         = JsonNamingPolicy.CamelCase,
+        Encoder                     = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
+        IncludeFields               = false,
+        NumberHandling              = JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.AllowNamedFloatingPointLiterals,
+        PropertyNameCaseInsensitive = true,
+        PropertyNamingPolicy        = JsonNamingPolicy.CamelCase,
+        ReadCommentHandling         = JsonCommentHandling.Skip,
+        WriteIndented               = false,
+    };
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    static internal string SerializeToJson(object obj) => JsonSerializer.Serialize(obj, options: Options);
+    static internal string SerializeToJson(object obj) => JsonSerializer.Serialize(obj, s_options);
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    static internal string SerializeToJson<T>(T obj) => JsonSerializer.Serialize<T>(obj, options: Options);
+    static internal string SerializeToJson<T>(T obj) => JsonSerializer.Serialize<T>(obj, s_options);
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    static internal T DeserializeFromJson<T>(string jsonString) => JsonSerializer.Deserialize<T>(jsonString, options: Options);
+    static internal T DeserializeFromJson<T>(string jsonString) => JsonSerializer.Deserialize<T>(jsonString, s_options);
 }
diff --git a/src/TBC.OpenBanking.Jws/src/ProtectedHeader.cs b/src/TBC.OpenBanking.Jws/src/ProtectedHeader.cs
@@ -43,16 +43,16 @@ public class DataToBeSignedDescription
         [JsonPropertyName("mId")]
         public string IdentificationMechanism { get; set; } = "http://uri.etsi.org/19182/HttpHeaders";
 
+        [SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "Intentional")]
         public void AddParameter(string value)
         {
             if (value == null) throw new ArgumentNullException(nameof(value));
 
             if (value.Length != 0)
             {
-                value = value.ToLowerInvariant();
                 // Preventing duplication
-                if (!Parameters.Contains(value))
-                    Parameters.Add(value);
+                if (!Parameters.Contains(value, StringComparer.OrdinalIgnoreCase))
+                    Parameters.Add(value.ToLowerInvariant());
             }
         }
     }

diff --git a/src/TBC.OpenBanking.Jws/src/TBC.OpenBanking.Jws.csproj b/src/TBC.OpenBanking.Jws/src/TBC.OpenBanking.Jws.csproj
@@ -46,9 +46,21 @@
     </PropertyGroup>
 
     <ItemGroup>
+        <Using Include="System.Diagnostics.CodeAnalysis" />
         <Using Include="System.Globalization.CultureInfo" Alias="CultureInfo" />
     </ItemGroup>
 
+    <ItemGroup>
+        <PackageReference Include="Meziantou.Analyzer" Version="2.0.*">
+            <PrivateAssets>all</PrivateAssets>
+            <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+        </PackageReference>
+        <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeStyle" Version="4.*">
+            <PrivateAssets>all</PrivateAssets>
+            <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+        </PackageReference>
+    </ItemGroup>
+
     <ItemGroup Condition=" '$(Configuration)' == 'Release' ">
         <PackageReference Include="MinVer" Version="4.2.0">
             <PrivateAssets>all</PrivateAssets>