address review feedback

Commit migrated from https://github.com/dotnet/corefx/commit/46785d3ae61987ae8ae389b2138645e82b54efb0

diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx
index af92019..2a2ce95 100644 (file)
--- a/src/libraries/System.Net.Http/src/Resources/Strings.resx
+++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx
@@ -397,6 +397,6 @@
     <value>The path '{0}' is too long, or a component of the specified path is too long.</value>
   </data>
   <data name="net_http_proxy_tunnel_failed" xml:space="preserve">
-    <value>Unable to establish a tunnel though proxy {0}.  The proxy returned status code {1}.</value>
+    <value>Unable to establish a tunnel though proxy '{0}'.  The proxy returned status code {1}.</value>
   </data>
 </root>

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs
index 8abe00e..fbe6b1a 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs
@@ -19,9 +19,7 @@ namespace System.Net.Http
         private static readonly HttpMethod s_optionsMethod = new HttpMethod("OPTIONS");
         private static readonly HttpMethod s_traceMethod = new HttpMethod("TRACE");
         private static readonly HttpMethod s_patchMethod = new HttpMethod("PATCH");
-
-        // Don't expose CONNECT as static property, since it's used by the transport to connect to a proxy.
-        // CONNECT is not used by users directly.
+        private static readonly HttpMethod s_connectMethod = new HttpMethod("CONNECT");
 
         public static HttpMethod Get
         {
@@ -63,6 +61,14 @@ namespace System.Net.Http
             get { return s_patchMethod; }
         }
 
+        // Don't expose CONNECT as static property, since it's used by the transport to connect to a proxy.
+        // CONNECT is not used by users directly.
+
+        internal static HttpMethod Connect
+        {
+            get { return s_connectMethod; }
+        }
+
         public string Method
         {
             get { return _method; }

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
index 96ae7af..c04b76c 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
@@ -49,6 +49,12 @@ namespace System.Net.Http
         /// 
         public HttpConnectionPool(HttpConnectionPoolManager poolManager, string host, int port, string sslHostName, Uri proxyUri, int maxConnections)
         {
+            Debug.Assert(proxyUri == null ?
+                    host != null && port != 0 :         // direct http or https connection
+                    (sslHostName == null ?
+                        host == null && port == 0 :     // proxy connection 
+                        host != null && port != 0));    // SSL proxy tunnel
+
             _poolManager = poolManager;
             _host = host;
             _port = port;
@@ -200,12 +206,12 @@ namespace System.Net.Http
 
         private async ValueTask<HttpConnection> CreateConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
-            Stream stream =
-                _proxyUri != null ?
-                    (_sslOptions != null ?
-                        await EstablishProxyTunnel(cancellationToken) :
-                        await ConnectHelper.ConnectAsync(_proxyUri.IdnHost, _proxyUri.Port, cancellationToken)) :
-                    await ConnectHelper.ConnectAsync(_host, _port, cancellationToken);
+            Stream stream = await
+                (_proxyUri == null ?
+                    ConnectHelper.ConnectAsync(_host, _port, cancellationToken) :
+                    (_sslOptions == null ?
+                        ConnectHelper.ConnectAsync(_proxyUri.IdnHost, _proxyUri.Port, cancellationToken) :
+                        EstablishProxyTunnel(cancellationToken))).ConfigureAwait(false);
 
             TransportContext transportContext = null;
             if (_sslOptions != null)
@@ -220,22 +226,25 @@ namespace System.Net.Http
                 new HttpConnectionWithFinalizer(this, stream, transportContext); // finalizer needed to signal the pool when a connection is dropped
         }
 
+        // TODO (#23136):
+        // CONNECT is not yet supported, so this code will not succeed currently.
+
         private async ValueTask<Stream> EstablishProxyTunnel(CancellationToken cancellationToken)
         {
             // Send a CONNECT request to the proxy server to establish a tunnel.
-            HttpRequestMessage tunnelRequest = new HttpRequestMessage(new HttpMethod("CONNECT"), _proxyUri);
+            HttpRequestMessage tunnelRequest = new HttpRequestMessage(HttpMethod.Connect, _proxyUri);
             tunnelRequest.Headers.Host = $"{_host}:{_port}";    // This specifies destination host/port to connect to
 
             // TODO: For now, we don't support proxy authentication in this scenario.
             // This will get fixed when we refactor proxy auth handling.
 
-            HttpResponseMessage tunnelResponse = await _poolManager.SendAsync(tunnelRequest, null, cancellationToken);
+            HttpResponseMessage tunnelResponse = await _poolManager.SendAsync(tunnelRequest, null, cancellationToken).ConfigureAwait(false);
             if (tunnelResponse.StatusCode != HttpStatusCode.OK)
             {
                 throw new HttpRequestException(SR.Format(SR.net_http_proxy_tunnel_failed, _proxyUri, tunnelResponse.StatusCode));
             }
 
-            return await tunnelResponse.Content.ReadAsStreamAsync();
+            return await tunnelResponse.Content.ReadAsStreamAsync().ConfigureAwait(false);
         }
 
         /// <summary>Enqueues a waiter to the waiters list.</summary>

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs
index fc60e70..103bb13 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs
@@ -106,21 +106,21 @@ namespace System.Net.Http
             if (HttpUtilities.IsSupportedSecureScheme(uri.Scheme))
             {
                 string hostHeader = request.Headers.Host;
-                if (hostHeader == null)
+                if (hostHeader != null)
                 {
-                    // No explicit Host header.  Use host from uri.
-                    sslHostName = uri.IdnHost;
+                    sslHostName = ParseHostNameFromHeader(hostHeader);
                 }
                 else
                 {
-                    sslHostName = ParseHostNameFromHeader(hostHeader);
+                    // No explicit Host header.  Use host from uri.
+                    sslHostName = uri.IdnHost;
                 }
             }
 
             if (proxyUri != null)
             {
                 Debug.Assert(HttpUtilities.IsSupportedNonSecureScheme(proxyUri.Scheme));
-                if (sslHostName != null)
+                if (sslHostName == null)
                 {
                     // Standard HTTP proxy usage for non-secure requests
                     // The destination host and port are ignored here, since these connections
@@ -230,10 +230,9 @@ namespace System.Net.Http
 
             // In the common case, SslHostName (when present) is equal to Host.  If so, don't include in hash.
             public override int GetHashCode() =>
-                (Host?.GetHashCode() ?? 0) ^
-                Port.GetHashCode() ^
-                (ProxyUri?.GetHashCode() ?? 0) ^
-                (SslHostName != null && SslHostName != Host ? SslHostName.GetHashCode() : 0);
+                (SslHostName == Host ?
+                    HashCode.Combine(Host, Port, ProxyUri) :
+                    HashCode.Combine(Host, Port, SslHostName, ProxyUri));
 
             public override bool Equals(object obj) =>
                 obj != null &&