Implement SocketsHttpHandler's Expect100ContinueTimeout and ConnectTimeout (#27242)

With the expectation that we'll want to expose this in 2.1, implement Expect100ContinueTimeout and ConnectTimeout.  The members are currently internal but can be flipped public easily once the APIs are approved.  This also fixes an issue with cancellation around the connect phase, where a cancellation request that came in during the SSL auth phase would not be respected.

Author: stephentoub

src/Common/src/System/Net/Http/HttpHandlerDefaults.cs

@@ -24,5 +24,7 @@ internal static class HttpHandlerDefaults
         public const bool DefaultCheckCertificateRevocationList = false;
         public static readonly TimeSpan DefaultPooledConnectionLifetime = Timeout.InfiniteTimeSpan;
         public static readonly TimeSpan DefaultPooledConnectionIdleTimeout = TimeSpan.FromMinutes(2);
+        public static readonly TimeSpan DefaultExpect100ContinueTimeout = TimeSpan.FromSeconds(1);
+        public static readonly TimeSpan DefaultConnectTimeout = Timeout.InfiniteTimeSpan;
     }
 }

src/System.Net.Http/src/ILLinkTrim.xml

@@ -2,5 +2,6 @@
   <assembly fullname="System.Net.Http">
     <!-- Anonymous types are used with DiagnosticSource logging and subscribers reflect over those, calling their public getters. -->
     <type fullname="*f__AnonymousType*" />
+    <type fullname="System.Net.Http.SocketsHttpHandler" /> <!-- TODO #27235, #27145: Remove once public -->
   </assembly>
 </linker>

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

@@ -28,11 +28,6 @@ internal partial class HttpConnection : IDisposable
         /// <summary>Default size of the write buffer used for the connection.</summary>
         private const int InitialWriteBufferSize = InitialReadBufferSize;
         /// <summary>
-        /// Delay after which we'll send the request payload for ExpectContinue if
-        /// the server hasn't yet responded.
-        /// </summary>
-        private const int Expect100TimeoutMilliseconds = 1000;
-        /// <summary>
         /// Size after which we'll close the connection rather than send the payload in response
         /// to final error status code sent by the server when using Expect: 100-continue.
         /// </summary>
@@ -353,7 +348,7 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, Can
                         allowExpect100ToContinue = new TaskCompletionSource<bool>();
                         var expect100Timer = new Timer(
                             s => ((TaskCompletionSource<bool>)s).TrySetResult(true),
-                            allowExpect100ToContinue, TimeSpan.FromMilliseconds(Expect100TimeoutMilliseconds), Timeout.InfiniteTimeSpan);
+                            allowExpect100ToContinue, _pool.Settings._expect100ContinueTimeout, Timeout.InfiniteTimeSpan);
                         _sendRequestContentTask = SendRequestContentWithExpect100ContinueAsync(
                             request, allowExpect100ToContinue.Task, stream, expect100Timer, cancellationToken);
                     }
@@ -580,10 +575,10 @@ private CancellationTokenRegistration RegisterCancellation(CancellationToken can
             }, _weakThisRef);
         }
 
-        private static bool ShouldWrapInOperationCanceledException(Exception error, CancellationToken cancellationToken) =>
+        internal static bool ShouldWrapInOperationCanceledException(Exception error, CancellationToken cancellationToken) =>
             !(error is OperationCanceledException) && cancellationToken.IsCancellationRequested;
 
-        private static Exception CreateOperationCanceledException(Exception error, CancellationToken cancellationToken) =>
+        internal static Exception CreateOperationCanceledException(Exception error, CancellationToken cancellationToken) =>
             new OperationCanceledException(s_cancellationMessage, error, cancellationToken);
 
         private static bool LineIsEmpty(ArraySegment<byte> line) => line.Count == 0;

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

@@ -206,24 +206,60 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, Can
 
         private async ValueTask<HttpConnection> CreateConnectionAsync(HttpRequestMessage request, CancellationToken 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)
+            // If a non-infinite connect timeout has been set, create and use a new CancellationToken that'll be canceled
+            // when either the original token is canceled or a connect timeout occurs.
+            CancellationTokenSource cancellationWithConnectTimeout = null;
+            if (Settings._connectTimeout != Timeout.InfiniteTimeSpan)
             {
-                SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(_sslOptions, request, stream, cancellationToken).ConfigureAwait(false);
-                stream = sslStream;
-                transportContext = sslStream.TransportContext;
+                cancellationWithConnectTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, default);
+                cancellationWithConnectTimeout.CancelAfter(Settings._connectTimeout);
+                cancellationToken = cancellationWithConnectTimeout.Token;
             }
 
-            return _maxConnections == int.MaxValue ?
-                new HttpConnection(this, stream, transportContext) :
-                new HttpConnectionWithFinalizer(this, stream, transportContext); // finalizer needed to signal the pool when a connection is dropped
+            try
+            {
+                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)
+                {
+                    // TODO #25206 and #24430: Register/IsCancellationRequested should be removable once SslStream auth and sockets respect cancellation.
+                    CancellationTokenRegistration ctr = cancellationToken.Register(s => ((Stream)s).Dispose(), stream);
+                    try
+                    {
+                        SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(_sslOptions, request, stream, cancellationToken).ConfigureAwait(false);
+                        stream = sslStream;
+                        transportContext = sslStream.TransportContext;
+                        cancellationToken.ThrowIfCancellationRequested(); // to handle race condition where stream is dispose of by cancellation after auth
+                    }
+                    catch (Exception exc)
+                    {
+                        stream.Dispose(); // in case cancellation occurs after successful SSL auth
+                        if (HttpConnection.ShouldWrapInOperationCanceledException(exc, cancellationToken))
+                        {
+                            throw HttpConnection.CreateOperationCanceledException(exc, cancellationToken);
+                        }
+                        throw;
+                    }
+                    finally
+                    {
+                        ctr.Dispose();
+                    }
+                }
+
+                return _maxConnections == int.MaxValue ?
+                    new HttpConnection(this, stream, transportContext) :
+                    new HttpConnectionWithFinalizer(this, stream, transportContext); // finalizer needed to signal the pool when a connection is dropped
+            }
+            finally
+            {
+                cancellationWithConnectTimeout?.Dispose();
+            }
         }
 
         // TODO (#23136):

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

@@ -30,6 +30,8 @@ internal sealed class HttpConnectionSettings
 
         internal TimeSpan _pooledConnectionLifetime = HttpHandlerDefaults.DefaultPooledConnectionLifetime;
         internal TimeSpan _pooledConnectionIdleTimeout = HttpHandlerDefaults.DefaultPooledConnectionIdleTimeout;
+        internal TimeSpan _expect100ContinueTimeout = HttpHandlerDefaults.DefaultExpect100ContinueTimeout;
+        internal TimeSpan _connectTimeout = HttpHandlerDefaults.DefaultConnectTimeout;
 
         internal SslClientAuthenticationOptions _sslOptions;
 
@@ -48,8 +50,10 @@ public HttpConnectionSettings Clone()
                 _allowAutoRedirect = _allowAutoRedirect,
                 _automaticDecompression = _automaticDecompression,
                 _cookieContainer = _cookieContainer,
+                _connectTimeout = _connectTimeout,
                 _credentials = _credentials,
                 _defaultProxyCredentials = _defaultProxyCredentials,
+                _expect100ContinueTimeout = _expect100ContinueTimeout,
                 _maxAutomaticRedirections = _maxAutomaticRedirections,
                 _maxConnectionsPerServer = _maxConnectionsPerServer,
                 _maxResponseHeadersLength = _maxResponseHeadersLength,

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

@@ -207,6 +207,38 @@ public TimeSpan PooledConnectionIdleTimeout
             }
         }
 
+        internal TimeSpan ConnectTimeout // TODO #27235: Expose publicly
+        {
+            get => _settings._connectTimeout;
+            set
+            {
+                if ((value <= TimeSpan.Zero && value != Timeout.InfiniteTimeSpan) ||
+                    (value.TotalMilliseconds > int.MaxValue))
+                {
+                    throw new ArgumentOutOfRangeException(nameof(value));
+                }
+
+                CheckDisposedOrStarted();
+                _settings._connectTimeout = value;
+            }
+        }
+
+        internal TimeSpan Expect100ContinueTimeout // TODO #27145: Expose publicly
+        {
+            get => _settings._expect100ContinueTimeout;
+            set
+            {
+                if ((value < TimeSpan.Zero && value != Timeout.InfiniteTimeSpan) ||
+                    (value.TotalMilliseconds > int.MaxValue))
+                {
+                    throw new ArgumentOutOfRangeException(nameof(value));
+                }
+
+                CheckDisposedOrStarted();
+                _settings._expect100ContinueTimeout = value;
+            }
+        }
+
         public IDictionary<string, object> Properties =>
             _settings._properties ?? (_settings._properties = new Dictionary<string, object>());