New rule S6962 for C#: You should pool HTTP connections with HttpClientFactory

analyzers/rspec/cs/S6962.html

@@ -0,0 +1,94 @@
+<p>In frequently used code paths, such as controller actions, you should avoid using the <a
+href="https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient">HttpClient</a> directly and opt for <a
+href="https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory">one of the IHttpClientFactory-based mechanisms</a> instead. This
+way, you avoid wasting resources and creating performance overhead.</p>
+<h2>Why is this an issue?</h2>
+<p>If a code path that creates and disposes of HttpClient objects is frequently used, then the following issues can occur:</p>
+<ul>
+  <li> Under heavy load, there’s the risk of <a
+  href="https://learn.microsoft.com/en-us/dotnet/fundamentals/networking/http/httpclient-guidelines#pooled-connections">running out of available
+  sockets</a>, leading to <a href="https://learn.microsoft.com/en-us/dotnet/api/system.net.sockets.socketexception">SocketException</a> errors. This
+  is because each HttpClient instance uses a separate network connection, and there’s a limit to the number of connections that can be opened
+  simultaneously. Note that even after you dispose of an HttpClient <a
+  href="https://learn.microsoft.com/en-us/dotnet/architecture/microservices/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests#issues-with-the-original-httpclient-class-available-in-net">its sockets are not immediately freed up</a>. </li>
+  <li> Each HttpClient has its own set of resources (like headers, base address, timeout, etc.) that must be managed. Creating a new HttpClient for
+  every request means these resources are not being reused, leading to resource waste. </li>
+  <li> You introduce a significant performance overhead when creating a new HttpClient for every HTTP request. </li>
+</ul>
+<h2>How to fix it</h2>
+<p>The <a href="https://learn.microsoft.com/en-us/dotnet/api/system.net.http.ihttpclientfactory"><code>IHttpClientFactory</code></a> was introduced in
+ASP.NET Core 2.1 to solve these problems. It handles pooling HTTP connections to optimize performance and reliability.</p>
+<p>There are <a href="https://learn.microsoft.com/en-us/aspnet/core/fundamentals/http-requests#consumption-patterns">several ways</a> that you can use
+IHttpClientFactory in your application:</p>
+<ul>
+  <li> <a href="https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory#basic-usage">Basic usage</a> </li>
+  <li> <a href="https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory#named-clients">Named Clients</a> </li>
+  <li> <a href="https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory#typed-clients">Typed Clients</a> </li>
+  <li> <a href="https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory#generated-clients">Generated Clients</a> </li>
+</ul>
+<p>Alternatively, you may cache the HttpClient in a singleton or a static field. You should be aware that by default, the HttpClient doesn’t respect
+the DNS’s Time To Live (TTL) settings. If the IP address associated with a domain name changes, HttpClient might still use the old, cached IP address,
+leading to failed requests.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+[ApiController]
+[Route("controller")]
+public class FooController : Controller
+{
+    [HttpGet]
+    public async Task&lt;string&gt; Foo()
+    {
+        using var client = new HttpClient();  //Noncompliant
+        return await client.GetStringAsync(_url);
+    }
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+// File: Startup.cs
+public class Startup
+{
+    public void ConfigureServices(IServiceCollection services)
+    {
+        services.AddHttpClient();
+        // ...
+    }
+}
+
+[ApiController]
+[Route("controller")]
+public class FooController : Controller
+{
+    private readonly IHttpClientFactory _clientFactory;
+
+    public FooController(IHttpClientFactory clientFactory)
+    {
+        _clientFactory = clientFactory;
+    }
+
+    [HttpGet]
+    public async Task&lt;string&gt; Foo()
+    {
+        using var client = _clientFactory.CreateClient(); // Compliant (Basic usage)
+        return await client.GetStringAsync(_url);
+    }
+}
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> <a href="https://rules.sonarsource.com/csharp/RSPEC-6420/">{rule:csharpsquid:S6420} - Client instances should not be recreated on each Azure
+  Function invocation</a> </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.net.http.ihttpclientfactory">IHttpClientFactory Interface</a>
+  </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient">HttpClient Class</a> </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory">IHttpClientFactory with .NET</a> </li>
+  <li> Microsoft Learn - <a
+  href="https://learn.microsoft.com/en-us/dotnet/architecture/microservices/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests">Use IHttpClientFactory to implement resilient HTTP requests</a> </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/aspnet/core/fundamentals/http-requests">Make HTTP requests using
+  IHttpClientFactory in ASP.NET Core</a> </li>
+  <li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/fundamentals/networking/http/httpclient-guidelines">Guidelines for using
+  HttpClient</a> </li>
+</ul>
+

analyzers/rspec/cs/S6962.json

@@ -0,0 +1,21 @@
+{
+  "title": "You should pool HTTP connections with HttpClientFactory",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "1h"
+  },
+  "tags": [],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6962",
+  "sqKey": "S6962",
+  "scope": "Main",
+  "quickfix": "infeasible",
+  "code": {
+    "impacts": {
+      "RELIABILITY": "MEDIUM"
+    },
+    "attribute": "COMPLETE"
+  }
+}

analyzers/rspec/cs/Sonar_way_profile.json

@@ -317,6 +317,7 @@
     "S6931",
     "S6934",
     "S6961",
+    "S6962",
     "S6965"
   ]
 }

analyzers/src/SonarAnalyzer.CSharp/ReuseClientBase.cs

@@ -0,0 +1,56 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2024 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+namespace SonarAnalyzer.Rules.CSharp
+{
+    public abstract class ReuseClientBase : SonarDiagnosticAnalyzer
+    {
+        protected abstract ImmutableArray<KnownType> ReusableClients { get; }
+
+        protected static bool IsAssignedForReuse(SonarSyntaxNodeReportingContext context) =>
+            !IsInVariableDeclaration(context.Node)
+            && (IsInConditionalCode(context.Node) || IsInFieldOrPropertyInitializer(context.Node) || IsAssignedToStaticFieldOrProperty(context));
+
+        protected bool IsReusableClient(SonarSyntaxNodeReportingContext context)
+        {
+            var objectCreation = ObjectCreationFactory.Create(context.Node);
+            return ReusableClients.Any(x => objectCreation.IsKnownType(x, context.SemanticModel));
+        }
+
+        private static bool IsInVariableDeclaration(SyntaxNode node) =>
+            node.Parent is EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax { Parent: VariableDeclarationSyntax { Parent: LocalDeclarationStatementSyntax or UsingStatementSyntax } } };
+
+        private static bool IsInFieldOrPropertyInitializer(SyntaxNode node) =>
+            node.Ancestors().Any(x => x.IsAnyKind(SyntaxKind.FieldDeclaration, SyntaxKind.PropertyDeclaration))
+            && !node.Ancestors().Any(x => x.IsAnyKind(SyntaxKind.GetAccessorDeclaration, SyntaxKind.SetAccessorDeclaration))
+            && !node.Parent.IsKind(SyntaxKind.ArrowExpressionClause);
+
+        private static bool IsInConditionalCode(SyntaxNode node) =>
+            node.Ancestors().Any(x => x.IsAnyKind(SyntaxKind.IfStatement,
+                SyntaxKind.SwitchStatement,
+                SyntaxKindEx.SwitchExpression,
+                SyntaxKind.ConditionalExpression,
+                SyntaxKindEx.CoalesceAssignmentExpression));
+
+        private static bool IsAssignedToStaticFieldOrProperty(SonarSyntaxNodeReportingContext context) =>
+            context.Node.Parent.WalkUpParentheses() is AssignmentExpressionSyntax assignment
+                && context.SemanticModel.GetSymbolInfo(assignment.Left, context.Cancel).Symbol is { IsStatic: true, Kind: SymbolKind.Field or SymbolKind.Property };
+    }
+}

analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/ControllersReuseClient.cs

@@ -0,0 +1,71 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2024 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+namespace SonarAnalyzer.Rules.CSharp;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class ControllersReuseClient : ReuseClientBase
+{
+    private const string DiagnosticId = "S6962";
+    private const string MessageFormat = "Reuse HttpClient instances rather than create new ones with each controller action invocation.";
+
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    protected override ImmutableArray<KnownType> ReusableClients => ImmutableArray.Create(KnownType.System_Net_Http_HttpClient);
+
+    protected override void Initialize(SonarAnalysisContext context) =>
+        context.RegisterCompilationStartAction(compilationStartContext =>
+        {
+            if (!compilationStartContext.Compilation.ReferencesControllers())
+            {
+                return;
+            }
+            compilationStartContext.RegisterSymbolStartAction(symbolStartContext =>
+            {
+                var symbol = (INamedTypeSymbol)symbolStartContext.Symbol;
+                if (symbol.IsControllerType())
+                {
+                    symbolStartContext.RegisterSyntaxNodeAction(nodeContext =>
+                    {
+                        var node = nodeContext.Node;
+                        if (IsInPublicMethod(node)
+                            && IsReusableClient(nodeContext)
+                            && !IsInsideConstructor(node)
+                            && !IsAssignedForReuse(nodeContext))
+                        {
+                            nodeContext.ReportIssue(Rule, node);
+                        }
+                    },
+                    SyntaxKind.ObjectCreationExpression,
+                    SyntaxKindEx.ImplicitObjectCreationExpression);
+                }
+            }, SymbolKind.NamedType);
+        });
+
+    public static bool IsInPublicMethod(SyntaxNode node) =>
+        node.FirstAncestorOrSelf<MethodDeclarationSyntax>() is not { } method
+        || (method.FirstAncestorOrSelf<PropertyDeclarationSyntax>() is null
+            && method.Modifiers.Any(x => x.IsKind(SyntaxKind.PublicKeyword)));
+
+    private static bool IsInsideConstructor(SyntaxNode node) =>
+        node.Ancestors().Any(x => x.IsAnyKind(SyntaxKind.ConstructorDeclaration, SyntaxKindEx.PrimaryConstructorBaseType));
+}

analyzers/src/SonarAnalyzer.CSharp/Rules/CloudNative/AzureFunctionsReuseClients.cs

@@ -21,14 +21,16 @@
 namespace SonarAnalyzer.Rules.CSharp
 {
     [DiagnosticAnalyzer(LanguageNames.CSharp)]
-    public sealed class AzureFunctionsReuseClients : SonarDiagnosticAnalyzer
+    public sealed class AzureFunctionsReuseClients : ReuseClientBase
     {
         private const string DiagnosticId = "S6420";
         private const string MessageFormat = "Reuse client instances rather than creating new ones with each function invocation.";
 
         private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
-        private static readonly KnownType[] ReusableClients =
-            {
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+        protected override ImmutableArray<KnownType> ReusableClients => ImmutableArray.Create(
                 KnownType.System_Net_Http_HttpClient,
                 // ComosDb (DocumentClient is superseded by CosmosClient)
                 KnownType.Microsoft_Azure_Documents_Client_DocumentClient,
@@ -48,41 +50,18 @@ public sealed class AzureFunctionsReuseClients : SonarDiagnosticAnalyzer
                 KnownType.Azure_Storage_Files_Shares_ShareServiceClient,
                 KnownType.Azure_Storage_Files_DataLake_DataLakeServiceClient,
                 // Resource manager
-                KnownType.Azure_ResourceManager_ArmClient
-            };
-
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+                KnownType.Azure_ResourceManager_ArmClient);
 
         protected override void Initialize(SonarAnalysisContext context) =>
             context.RegisterNodeAction(c =>
             {
                 if (c.AzureFunctionMethod() is not null
-                    && IsResuableClient(c)
+                    && IsReusableClient(c)
                     && !IsAssignedForReuse(c))
                 {
                     c.ReportIssue(Rule, c.Node);
                 }
             },
             SyntaxKind.ObjectCreationExpression, SyntaxKindEx.ImplicitObjectCreationExpression);
-
-        private static bool IsAssignedForReuse(SonarSyntaxNodeReportingContext context) =>
-            !IsInVariableDeclaration(context.Node)
-            && (IsInFieldOrPropertyInitializer(context.Node) || IsAssignedToStaticFieldOrProperty(context));
-
-        private static bool IsInVariableDeclaration(SyntaxNode node) =>
-            node.Parent is EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax { Parent: VariableDeclarationSyntax { Parent: LocalDeclarationStatementSyntax or UsingStatementSyntax } } };
-
-        private static bool IsInFieldOrPropertyInitializer(SyntaxNode node) =>
-            node.Ancestors().Any(x => x.IsAnyKind(SyntaxKind.FieldDeclaration, SyntaxKind.PropertyDeclaration));
-
-        private static bool IsAssignedToStaticFieldOrProperty(SonarSyntaxNodeReportingContext context) =>
-            context.Node.Parent.WalkUpParentheses() is AssignmentExpressionSyntax assignment
-                && context.SemanticModel.GetSymbolInfo(assignment.Left, context.Cancel).Symbol is { IsStatic: true, Kind: SymbolKind.Field or SymbolKind.Property };
-
-        private static bool IsResuableClient(SonarSyntaxNodeReportingContext context)
-        {
-            var objectCreation = ObjectCreationFactory.Create(context.Node);
-            return ReusableClients.Any(x => objectCreation.IsKnownType(x, context.SemanticModel));
-        }
     }
 }

analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs

@@ -6886,7 +6886,7 @@ internal static class RuleTypeMappingCS
         // ["S6959"],
         // ["S6960"],
         ["S6961"] = "CODE_SMELL",
-        // ["S6962"],
+        ["S6962"] = "CODE_SMELL",
         // ["S6963"],
         // ["S6964"],
         ["S6965"] = "CODE_SMELL",

analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/ControllersReuseClientTest.cs

@@ -0,0 +1,68 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2024 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+using SonarAnalyzer.Rules.CSharp;
+
+namespace SonarAnalyzer.Test.Rules;
+
+#if NET
+
+[TestClass]
+public class ControllersReuseClientTest
+{
+    private readonly VerifierBuilder builder = new VerifierBuilder<ControllersReuseClient>()
+        .WithBasePath("AspNet")
+        .AddReferences(
+        [
+            AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcAbstractions,
+            AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcCore,
+            AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcViewFeatures,
+            .. MetadataReferenceFacade.SystemThreadingTasks,
+            .. NuGetMetadataReference.SystemNetHttp(),
+            .. NuGetMetadataReference.MicrosoftExtensionsHttp()
+        ]);
+
+    [TestMethod]
+    public void ControllersReuseClient_CS() =>
+        builder
+            .AddPaths("ControllersReuseClient.cs")
+            .Verify();
+
+    [TestMethod]
+    public void ControllersReuseClient_CS8() =>
+        builder
+            .AddPaths("ControllersReuseClient.CSharp8.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp8)
+            .Verify();
+
+    [TestMethod]
+    public void ControllersReuseClient_CS9() =>
+        builder
+            .AddPaths("ControllersReuseClient.CSharp9.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp9)
+            .Verify();
+
+    [TestMethod]
+    public void ControllersReuseClient_CS12() =>
+        builder
+            .AddPaths("ControllerReuseClient.CSharp12.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp12).Verify();
+}
+#endif