runtime: HttpConnectionPool violates HttpHeaders thread-safety for CONNECT tunnels

In .NET 6, the creation of a connection has been decoupled from the initiating request. That is, a connection attempt may still be ongoing even after the initiating request was canceled or was served by a different connection that became available.

In the case of HttpConnectionKind.SslProxyTunnel (CONNECT tunnel), we copy the original request’s User-Agent header to the tunnel request here. The problem is that we access the header too late. At that point, the original request’s header collection could be enumerated on a different thread (served by a different connection). Or, the SendAsync call associated with the request already returned, seemingly returning ownership back to the user. The user may then attempt to enumerate/inspect the headers, which would again race with the read from EstablishProxyTunnelAsync.

This is a possible explanation for an issue a user hit in https://github.com/dotnet/runtime/issues/61798#issuecomment-1040227700 based on the stack trace.

cc: @Strepto

This specific issue can be worked around by manually forcing the parsing of the User-Agent header:

var socketsHttpHandler = new SocketsHttpHandler();
var customHandler = new CustomHandler(socketsHttpHandler);
var httpClient = new HttpClient(customHandler);

public sealed class CustomHandler : DelegatingHandler
{
    public CustomHandler(HttpMessageHandler innerHandler) : base(innerHandler) { }

    protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        _ = request.Headers.TryGetValues("User-Agent", out _); // Force parsing

        return base.Send(request, cancellationToken);
    }

    protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        _ = request.Headers.TryGetValues("User-Agent", out _); // Force parsing

        return base.SendAsync(request, cancellationToken);
    }
}

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 2
  • Comments: 23 (19 by maintainers)

Most upvoted comments

Can you share how you implemented the workaround? We would expect that it is a 100% reliable fix for this specific issue.

If it’s not, it’s possible you are running into a different header issue (e.g. using outdated NewRelic) that results in similar issues. Notably, you shouldn’t see HttpConnectionPool.EstablishProxyTunnelAsync in the stack trace at all after the workaround.

@MihaZupan thanks for following up. We’ve deployed the workaround and are actively monitoring. The error was transient, so we want to wait a week or so before declaring victory.

Triage: Impact on reliability with proxies, we should address it. Hopefully not too complex.

Somewhat related here: We are forcing the User-Agent to be parsed here because we use TryGetValues. We should avoid this by using NonValidated here.

(I thought there was an issue on this but I couldn’t find it…)