Add an experimental option to avoid partitioning the connection pool by SNI#131153
Add an experimental option to avoid partitioning the connection pool by SNI#131153MihaZupan wants to merge 1 commit into
Conversation
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
There was a problem hiding this comment.
Pull request overview
This PR adds an experimental, non-public opt-in that allows SocketsHttpHandler to avoid partitioning the connection pool by SNI / SslHostName, enabling connection reuse across requests that vary Host while targeting the same URI host/port (e.g., shared infrastructure behind a single IP). It does so by adding an internal flag on HttpRequestMessage, honoring it in HttpConnectionPoolManager when creating the pool key, and adding functional tests across HTTP/1.1, HTTP/2, and HTTP/3.
Changes:
- Add an internal
HttpRequestMessageflag + internal method (intended to be invoked viaUnsafeAccessor) to disable pool partitioning by SNI for a request. - Update
HttpConnectionPoolManager.SendAsyncCoreto use a pool key that omitsSslHostNamewhen the request opts in. - Add new functional tests validating pooling behavior across differing
Hostheaders and ensure the opt-in doesn’t mix with non-opted-in requests; preserve the experimental method from trimming via ILLink descriptors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs | Adds functional tests covering connection reuse behavior across Host/SNI variations for HTTP/1.1, 2, and 3. |
| src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs | Alters pool key computation to optionally ignore SslHostName (SNI) for opted-in requests. |
| src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs | Introduces an internal flag + internal opt-in method used to disable SNI-based partitioning per request. |
| src/libraries/System.Net.Http/src/ILLink/ILLink.Descriptors.LibraryBuild.xml | Preserves the experimental opt-in method for trimmed builds since it’s invoked via UnsafeAccessor. |
| GetUnderlyingSocketsHttpHandler(handler).SslOptions.RemoteCertificateValidationCallback = (sender, _, _, _) => | ||
| { | ||
| string sni = sender switch | ||
| { | ||
| SslStream s => s.TargetHostName, | ||
| QuicConnection q => q.TargetHostName, | ||
| _ => null | ||
| }; | ||
| sniValues.Add(sni); | ||
| return true; | ||
| }; |
| string? sslHostName = key.SslHostName; | ||
|
|
||
| if (sslHostName is not null && request.IsConnectionPoolPartitioningBySniDisabled()) | ||
| { | ||
| // The request is using HTTPS, but has opted out of partitioning the connection pool by SNI. | ||
| // The connection pool will be shared by requests to the same Uri Host, regardless of the Host header. | ||
| // This enables requests where the Uri is set to an IP of shared infrastructure to share connections even if the host names differ. | ||
| // This is a dangerous opt-in where the caller is responsible for ensuring that the server certificate is acceptable for all requests to a given IP. | ||
| key = new HttpConnectionKey(key.Kind, key.Host, key.Port, sslHostName: null, key.ProxyUri, key.Identity); | ||
| } | ||
|
|
||
| HttpConnectionPool? pool; | ||
| while (!_pools.TryGetValue(key, out pool)) | ||
| { | ||
| pool = new HttpConnectionPool(this, key.Kind, key.Host, key.Port, key.SslHostName, key.ProxyUri, GetTelemetryServerAddress(request, key)); | ||
| pool = new HttpConnectionPool(this, key.Kind, key.Host, key.Port, sslHostName, key.ProxyUri, GetTelemetryServerAddress(request, key)); |
| // The connection pool will be shared by requests to the same Uri Host, regardless of the Host header. | ||
| // This enables requests where the Uri is set to an IP of shared infrastructure to share connections even if the host names differ. | ||
| // This is a dangerous opt-in where the caller is responsible for ensuring that the server certificate is acceptable for all requests to a given IP. | ||
| key = new HttpConnectionKey(key.Kind, key.Host, key.Port, sslHostName: null, key.ProxyUri, key.Identity); |
There was a problem hiding this comment.
I know it's experimental and only for when "you know what you're doing", but this will also override sslHostName for tunnels. Is it that intentional?
Wouldn't this condition be better moved here:
Replaces #128653
This adds a way to opt-in requests into sharing connections regardless of SNI, potentially sharing connections with TLS established to a different host.
In practice it'd be used when you know you're talking with shared infrastructure that uses different hosts, and you know you can share connections to the same IP.
It's not exposed as public API for now, we can think about that for .NET 12 if it proves valuable enough.