Use Httpclientfactory

ID

csharp.use_httpclientfactory

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Best Practice

Language

CSharp

Tags

best_practice, http, resource-management, scalability

Description

Reports new HttpClient(…​) on the request path: inside an ASP.NET request handler, or inside a controller type where a using disposes the client when the scope ends. Each client owns a connection pool, so creating and disposing them per request exhausts the machine’s sockets.

Rationale

HttpClient looks like a request object and is in fact the owner of a connection pool. Constructing one per call multiplies those pools, and disposing it does not release its sockets straight away: each closed connection sits in the operating system’s wait state for a couple of minutes before its port is reclaimed. A service that opens clients faster than the system reclaims ports runs out of ephemeral ports and starts failing on every outbound call, not only the one that caused the exhaustion.

The shape of the failure is what makes it expensive. It is invisible below a traffic threshold and sudden above it, it survives deployment because nothing about the code looks wrong, and it presents as unrelated network errors across the whole process. The disposal is the aggravating factor rather than the mitigation — code that carefully wraps the client in a using churns sockets faster than code that leaks it.

Keeping one client forever avoids that but introduces the opposite defect: a long-lived client caches its connections and never repeats name resolution, so it keeps sending traffic to an endpoint that has been moved, scaled or failed over. A client factory resolves both — it pools the underlying handlers, reuses connections across logically separate clients, and rotates handlers on a lifetime so name resolution happens again.

The trigger is deliberately narrow, and it is the request path that defines it. A request handler qualifies on its own, because it runs once per request and that is what turns one construction into an allocation rate. A scoped using is a strengthener rather than a trigger: it establishes the disposal that causes the churn but not the repetition, so it counts only inside a controller type, where the surrounding member — a constructor, a property accessor, a local function — is still on that path. Construction anywhere else, in a console entry point, a migration or a one-shot job, exhausts nothing however it is disposed, and reporting it would put a finding on every legitimate use of the constructor. A static readonly field is not reported either: that is the singleton pattern, and it is the accepted answer where a factory is not available. Neither is a type whose name marks it as a factory, nor test code.

using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;

public class OrdersController : ControllerBase
{
    private readonly IHttpClientFactory _factory;

    public OrdersController(IHttpClientFactory factory) => _factory = factory;

    [HttpGet("bad")]
    public async Task<string> Bad(string url)
    {
        HttpClient client = new HttpClient();   // FLAW — a new connection pool on every request
        return await client.GetStringAsync(url);
    }

    [HttpGet("good")]
    public async Task<string> Good(string url)
    {
        HttpClient client = _factory.CreateClient();   // OK, pooled handler, rotated lifetime
        return await client.GetStringAsync(url);
    }
}

public class Probe
{
    private static readonly HttpClient Shared = new HttpClient();   // OK, the singleton pattern

    public Task<string> Once(string url) => Shared.GetStringAsync(url);
}

public class Program
{
    public static async Task Main(string[] args)
    {
        using var client = new HttpClient();   // OK — a console entry point runs once
        await client.GetStringAsync(args[0]);
    }
}

Remediation

Register the factory once during start-up and take IHttpClientFactory as a dependency, then ask it for a client where the request is made. The client it returns is cheap to create and must not be cached: the factory owns the handler behind it and the short-lived wrapper is the point.

Two refinements are worth adopting at the same time:

  • register a named or typed client for each downstream service, so its base address, default headers and timeout live in one place instead of being repeated at every call site;

  • attach retry or circuit-breaker behaviour to that registration rather than to the call sites, since the factory’s handler pipeline is where cross-cutting policy belongs.

Where the factory is genuinely unavailable — a library with no dependency-injection container, a small console program — keep a single static readonly client for the lifetime of the process and set a connection lifetime on its handler so that name resolution is refreshed.