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)
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…)