reorganize pool manager logic and rework HttpConnectionKey to support ssl proxy tunneling

Author: Geoff Kizer

src/System.Net.Http/src/System.Net.Http.csproj

@@ -138,9 +138,8 @@
     <Compile Include="System\Net\Http\SocketsHttpHandler\EmptyReadStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnection.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionHandler.cs" />
-    <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionKey.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionPool.cs" />
-    <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionPools.cs" />
+    <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionPoolManager.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionResponseContent.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionSettings.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpContentDuplexStream.cs" />
@@ -464,4 +463,4 @@
     <Reference Include="System.Security.Cryptography.Primitives" />
   </ItemGroup>
   <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
-</Project>
+</Project>

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs

@@ -32,11 +32,8 @@ public CertificateCallbackMapper(Func<HttpRequestMessage, X509Certificate2, X509
             }
         }
 
-        public static async ValueTask<Stream> ConnectAsync(HttpConnectionKey key, CancellationToken cancellationToken)
+        public static async ValueTask<Stream> ConnectAsync(string host, int port, CancellationToken cancellationToken)
         {
-            string host = key.Host;
-            int port = key.Port;
-
             try
             {
                 // Rather than creating a new Socket and calling ConnectAsync on it, we use the static

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs

@@ -79,7 +79,7 @@ public HttpConnection(
             _pool = pool;
             _stream = stream;
             _transportContext = transportContext;
-            _usingProxy = pool.Pools.UsingProxy;
+            _usingProxy = pool.UsingProxy;
             _idnHostAsciiBytes = pool.IdnHostAsciiBytes;
 
             _writeBuffer = new byte[InitialWriteBufferSize];
@@ -89,11 +89,11 @@ public HttpConnection(
 
             if (NetEventSource.IsEnabled)
             {
-                if (_stream is SslStream sslStream)
+                if (pool.IsSecure)
                 {
+                    var sslStream = (SslStream)_stream;
                     Trace(
-                        $"Secure connection created to {pool.Key.Host}:{pool.Key.Port}. " +
-                        $"SslHostName:{pool.Key.SslHostName}. " +
+                        $"Secure connection created to {pool}. " +
                         $"SslProtocol:{sslStream.SslProtocol}, " +
                         $"CipherAlgorithm:{sslStream.CipherAlgorithm}, CipherStrength:{sslStream.CipherStrength}, " +
                         $"HashAlgorithm:{sslStream.HashAlgorithm}, HashStrength:{sslStream.HashStrength}, " +
@@ -102,7 +102,7 @@ public HttpConnection(
                 }
                 else
                 {
-                    Trace($"Connection created to {pool.Key.Host}:{pool.Key.Port}.");
+                    Trace($"Connection created to {pool}.");
                 }
             }
         }
@@ -261,9 +261,9 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, Can
 
                 // Determine cookies to send
                 string cookiesFromContainer = null;
-                if (_pool.Pools.Settings._useCookies)
+                if (_pool.Settings._useCookies)
                 {
-                    cookiesFromContainer = _pool.Pools.Settings._cookieContainer.GetCookieHeader(request.RequestUri);
+                    cookiesFromContainer = _pool.Settings._cookieContainer.GetCookieHeader(request.RequestUri);
                     if (cookiesFromContainer == "")
                     {
                         cookiesFromContainer = null;
@@ -357,7 +357,7 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, Can
                 }
 
                 // Start to read response.
-                _allowedReadLineBytes = _pool.Pools.Settings._maxResponseHeadersLength * 1024;
+                _allowedReadLineBytes = _pool.Settings._maxResponseHeadersLength * 1024;
 
                 // We should not have any buffered data here; if there was, it should have been treated as an error
                 // by the previous request handling.  (Note we do not support HTTP pipelining.)
@@ -501,9 +501,9 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, Can
                 if (NetEventSource.IsEnabled) Trace($"Received response: {response}");
 
                 // Process Set-Cookie headers.
-                if (_pool.Pools.Settings._useCookies)
+                if (_pool.Settings._useCookies)
                 {
-                    CookieHelper.ProcessReceivedCookies(response, _pool.Pools.Settings._cookieContainer);
+                    CookieHelper.ProcessReceivedCookies(response, _pool.Settings._cookieContainer);
                 }
 
                 return response;
@@ -1290,7 +1290,7 @@ private static bool EqualsOrdinal(string left, Span<byte> right)
             return true;
         }
 
-        public sealed override string ToString() => $"{nameof(HttpConnection)}({_pool.Key})"; // Description for diagnostic purposes
+        public sealed override string ToString() => $"{nameof(HttpConnection)}({_pool})"; // Description for diagnostic purposes
 
         private static void ThrowInvalidHttpResponse() => throw new HttpRequestException(SR.net_http_invalid_response);
 

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionHandler.cs

@@ -9,70 +9,23 @@ namespace System.Net.Http
 {
     internal sealed class HttpConnectionHandler : HttpMessageHandler
     {
-        private readonly HttpConnectionPools _connectionPools;
+        private readonly HttpConnectionPoolManager _poolManager;
 
-        public HttpConnectionHandler(HttpConnectionSettings settings)
+        public HttpConnectionHandler(HttpConnectionPoolManager poolManager)
         {
-            _connectionPools = new HttpConnectionPools(settings, usingProxy: false);
+            _poolManager = poolManager;
         }
 
         protected internal override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
-            Uri uri = request.RequestUri;
-            HttpConnectionKey key = new HttpConnectionKey(uri.IdnHost, uri.Port, GetSslHostName(request));
-            HttpConnectionPool pool = _connectionPools.GetOrAddPool(key);
-            return pool.SendAsync(request, cancellationToken);
-        }
-
-        private static string GetSslHostName(HttpRequestMessage request)
-        {
-            Uri uri = request.RequestUri;
-
-            if (!HttpUtilities.IsSupportedSecureScheme(uri.Scheme))
-            {
-                // Not using SSL.
-                return null;
-            }
-
-            string hostHeader = request.Headers.Host;
-            if (hostHeader == null)
-            {
-                // No explicit Host header.  Use host from uri.
-                return request.RequestUri.IdnHost;
-            }
-
-            // There is a host header.  Use it, but first see if we need to trim off a port.
-            int colonPos = hostHeader.IndexOf(':');
-            if (colonPos >= 0)
-            {
-                // There is colon, which could either be a port separator or a separator in
-                // an IPv6 address.  See if this is an IPv6 address; if it's not, use everything
-                // before the colon as the host name, and if it is, use everything before the last
-                // colon iff the last colon is after the end of the IPv6 address (otherwise it's a
-                // part of the address).
-                int ipV6AddressEnd = hostHeader.IndexOf(']');
-                if (ipV6AddressEnd == -1)
-                {
-                    return hostHeader.Substring(0, colonPos);
-                }
-                else
-                {
-                    colonPos = hostHeader.LastIndexOf(':');
-                    if (colonPos > ipV6AddressEnd)
-                    {
-                        return hostHeader.Substring(0, colonPos);
-                    }
-                }
-            }
-
-            return hostHeader;
+            return _poolManager.SendAsync(request, null, cancellationToken);
         }
 
         protected override void Dispose(bool disposing)
         {
             if (disposing)
             {
-                _connectionPools.Dispose();
+                _poolManager.Dispose();
             }
 
             base.Dispose(disposing);

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionKey.cs

@@ -17,8 +17,10 @@ namespace System.Net.Http
     /// <summary>Provides a pool of connections to the same endpoint.</summary>
     internal sealed class HttpConnectionPool : IDisposable
     {
-        private readonly HttpConnectionPools _pools;
-        private readonly HttpConnectionKey _key;
+        private readonly HttpConnectionPoolManager _poolManager;
+        private readonly string _host;
+        private readonly int _port;
+        private readonly Uri _proxyUri;
 
         /// <summary>List of idle connections stored in the pool.</summary>
         private readonly List<CachedConnection> _idleConnections = new List<CachedConnection>();
@@ -44,39 +46,38 @@ internal sealed class HttpConnectionPool : IDisposable
 
         /// <summary>Initializes the pool.</summary>
         /// <param name="maxConnections">The maximum number of connections allowed to be associated with the pool at any given time.</param>
-        public HttpConnectionPool(HttpConnectionPools pools, HttpConnectionKey key, int maxConnections = int.MaxValue) // int.MaxValue treated as infinite
+        /// 
+        public HttpConnectionPool(HttpConnectionPoolManager poolManager, string host, int port, string sslHostName, Uri proxyUri, int maxConnections)
         {
-            _pools = pools;
-            _key = key;
+            _poolManager = poolManager;
+            _host = host;
+            _port = port;
+            _proxyUri = proxyUri;
             _maxConnections = maxConnections;
 
-            // Precalculate ASCII bytes for header name
-            // We don't do this for proxy connections because the actual host header varies on a proxy connection.
-            if (!pools.UsingProxy)
+            if (sslHostName != null)
             {
-                // CONSIDER: Cache more than just host name -- port, header name, etc
-
-                // Note the IDN hostname should always be ASCII, since it's already been IDNA encoded.
-                _idnHostAsciiBytes = Encoding.ASCII.GetBytes(key.Host);
-                Debug.Assert(Encoding.ASCII.GetString(_idnHostAsciiBytes) == key.Host);
-            }
-            else
-            {
-                // Proxy connections should never use SSL
-                Debug.Assert(!key.IsSecure);
+                // Precalculate cached SSL options to use for all connections.
+                _sslOptions = _poolManager.Settings._sslOptions?.ShallowClone() ?? new SslClientAuthenticationOptions();
+                _sslOptions.ApplicationProtocols = null; // explicitly ignore any ApplicationProtocols set
+                _sslOptions.TargetHost = sslHostName; // always use the key's name rather than whatever was specified
             }
 
-            if (key.IsSecure)
+            if (_host != null)
             {
-                // Precalculate cached SSL options to use for all connections.
-                _sslOptions = _pools.Settings._sslOptions?.ShallowClone() ?? new SslClientAuthenticationOptions();
-                _sslOptions.ApplicationProtocols = null; // explicitly ignore any ApplicationProtocols set
-                _sslOptions.TargetHost = key.SslHostName; // always use the key's name rather than whatever was specified
+                // Precalculate ASCII bytes for header name
+                // Note that if _host is null, this is a (non-tunneled) proxy connection, and we can't cache the hostname.
+                // CONSIDER: Cache more than just host name -- port, header name, etc
+
+                // Note the IDN hostname should always be ASCII, since it's already been IDNA encoded.
+                _idnHostAsciiBytes = Encoding.ASCII.GetBytes(_host);
+                Debug.Assert(Encoding.ASCII.GetString(_idnHostAsciiBytes) == _host);
             }
         }
 
-        public HttpConnectionKey Key => _key;
-        public HttpConnectionPools Pools => _pools;
+        public HttpConnectionSettings Settings => _poolManager.Settings;
+        public bool IsSecure => _sslOptions != null;
+        public bool UsingProxy => (_proxyUri != null && !IsSecure);     // Tunnel doesn't count, only direct proxy usage
         public byte[] IdnHostAsciiBytes => _idnHostAsciiBytes;
 
         /// <summary>Object used to synchronize access to state in the pool.</summary>
@@ -89,8 +90,8 @@ private ValueTask<HttpConnection> GetConnectionAsync(HttpRequestMessage request,
                 return new ValueTask<HttpConnection>(Task.FromCanceled<HttpConnection>(cancellationToken));
             }
 
-            TimeSpan pooledConnectionLifetime = _pools.Settings._pooledConnectionLifetime;
-            TimeSpan pooledConnectionIdleTimeout = _pools.Settings._pooledConnectionIdleTimeout;
+            TimeSpan pooledConnectionLifetime = _poolManager.Settings._pooledConnectionLifetime;
+            TimeSpan pooledConnectionIdleTimeout = _poolManager.Settings._pooledConnectionIdleTimeout;
             DateTimeOffset now = DateTimeOffset.UtcNow;
             List<CachedConnection> list = _idleConnections;
             lock (SyncObj)
@@ -199,10 +200,15 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, Can
 
         private async ValueTask<HttpConnection> CreateConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
-            Stream stream = await ConnectHelper.ConnectAsync(_key, cancellationToken).ConfigureAwait(false);
+            Stream stream =
+                _proxyUri != null ?
+                    (_sslOptions != null ?
+                        throw new NotSupportedException("SSL Proxy tunneling not currently supported") :
+                        await ConnectHelper.ConnectAsync(_proxyUri.IdnHost, _proxyUri.Port, cancellationToken)) :
+                    await ConnectHelper.ConnectAsync(_host, _port, cancellationToken);
 
             TransportContext transportContext = null;
-            if (_key.IsSecure)
+            if (_sslOptions != null)
             {
                 SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(_sslOptions, request, stream, cancellationToken).ConfigureAwait(false);
                 stream = sslStream;
@@ -473,8 +479,8 @@ public void Dispose()
         /// </returns>
         public bool CleanCacheAndDisposeIfUnused()
         {
-            TimeSpan pooledConnectionLifetime = _pools.Settings._pooledConnectionLifetime;
-            TimeSpan pooledConnectionIdleTimeout = _pools.Settings._pooledConnectionIdleTimeout;
+            TimeSpan pooledConnectionLifetime = _poolManager.Settings._pooledConnectionLifetime;
+            TimeSpan pooledConnectionIdleTimeout = _poolManager.Settings._pooledConnectionIdleTimeout;
 
             List<CachedConnection> list = _idleConnections;
             List<HttpConnection> toDispose = null;
@@ -560,7 +566,16 @@ public bool CleanCacheAndDisposeIfUnused()
             return false;
         }
 
-        public override string ToString() => $"{nameof(HttpConnectionPool)}(Connections:{_associatedConnectionCount})"; // Description for diagnostic purposes
+        // For diagnostic purposes
+        public override string ToString() =>
+            $"{nameof(HttpConnectionPool)}" +
+            (_proxyUri == null ?
+                (_sslOptions == null ?
+                    $"http://{_host}:{_port}" :
+                    $"https://{_host}:{_port}" + (_sslOptions.TargetHost != _host ? $", SSL TargetHost={_sslOptions.TargetHost}" : null)) :
+                (_sslOptions == null ?
+                    $"Proxy {_proxyUri}" :
+                    $"https://{_host}:{_port}/ tunnelled via Proxy {_proxyUri}" + (_sslOptions.TargetHost != _host ? $", SSL TargetHost={_sslOptions.TargetHost}" : null)));
 
         private void Trace(string message, [CallerMemberName] string memberName = null) =>
             NetEventSource.Log.HandlerMessage(