HTTP/3 RequestHeadersTimeout

Author: JamesNK

src/Servers/Kestrel/Core/src/CoreStrings.resx

@@ -683,4 +683,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
   <data name="Http3StreamErrorRequestEndedNoHeaders" xml:space="preserve">
     <value>Request stream ended without headers.</value>
   </data>
+  <data name="Http3ControlStreamHeaderTimeout" xml:space="preserve">
+    <value>Reading the control stream header timed out.</value>
+  </data>
 </root>

src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
@@ -14,14 +15,15 @@
 using Microsoft.AspNetCore.Connections.Features;
 using Microsoft.AspNetCore.Hosting.Server;
 using Microsoft.AspNetCore.Http.Features;
+using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
 {
-    internal class Http3Connection : ITimeoutHandler
+    internal class Http3Connection : ITimeoutHandler, IHttp3StreamLifetimeHandler
     {
-        internal readonly Dictionary<long, Http3Stream> _streams = new Dictionary<long, Http3Stream>();
+        internal readonly Dictionary<long, IHttp3Stream> _streams = new Dictionary<long, IHttp3Stream>();
 
         private long _highestOpenedStreamId;
         private readonly object _sync = new object(); 
@@ -84,9 +86,6 @@ public async Task ProcessStreamsAsync<TContext>(IHttpApplication<TContext> httpA
         {
             try
             {
-                // Ensure TimeoutControl._lastTimestamp is initialized before anything that could set timeouts runs.
-                _timeoutControl.Initialize(_systemClock.UtcNowTicks);
-
                 var connectionHeartbeatFeature = _context.ConnectionFeatures.Get<IConnectionHeartbeatFeature>();
                 var connectionLifetimeNotificationFeature = _context.ConnectionFeatures.Get<IConnectionLifetimeNotificationFeature>();
 
@@ -198,11 +197,42 @@ public void Tick()
                 return;
             }
 
-            // It's safe to use UtcNowUnsynchronized since Tick is called by the Heartbeat.
-            var now = _systemClock.UtcNowUnsynchronized;
-            _timeoutControl.Tick(now);
+            UpdateStartingStreams();
+        }
+
+        private void UpdateStartingStreams()
+        {
+            var now = _systemClock.UtcNow.Ticks;
+
+            lock (_streams)
+            {
+                foreach (var stream in _streams.Values)
+                {
+                    if (stream.ReceivedHeader)
+                    {
+                        continue;
+                    }
+
+                    if (stream.HeaderTimeoutTicks == default)
+                    {
+                        // On expiration overflow, use max value.
+                        var expirationTicks = now + _context.ServiceContext.ServerOptions.Limits.RequestHeadersTimeout.Ticks;
+                        stream.HeaderTimeoutTicks = expirationTicks >= 0 ? expirationTicks : long.MaxValue;
+                    }
 
-            // TODO cancel process stream loop to update logic.
+                    if (stream.HeaderTimeoutTicks < now)
+                    {
+                        if (stream.IsRequestStream)
+                        {
+                            stream.Abort(new ConnectionAbortedException(CoreStrings.BadRequest_RequestHeadersTimeout), Http3ErrorCode.RequestRejected);
+                        }
+                        else
+                        {
+                            stream.Abort(new ConnectionAbortedException(CoreStrings.Http3ControlStreamHeaderTimeout), Http3ErrorCode.StreamCreationError);
+                        }
+                    }
+                }
+            }
         }
 
         public void OnTimeout(TimeoutReason reason)
@@ -213,13 +243,11 @@ public void OnTimeout(TimeoutReason reason)
             // TODO what timeouts should we handle here? Is keep alive something we should care about?
             switch (reason)
             {
-                case TimeoutReason.KeepAlive:
-                    SendGoAway(GetHighestStreamId()).Preserve();
-                    break;
                 case TimeoutReason.TimeoutFeature:
                     SendGoAway(GetHighestStreamId()).Preserve();
                     break;
-                case TimeoutReason.RequestHeaders:
+                case TimeoutReason.RequestHeaders: // Request header timeout is handled in starting stream queue
+                case TimeoutReason.KeepAlive:  // Keep-alive is handled by msquic
                 case TimeoutReason.ReadDataRate:
                 case TimeoutReason.WriteDataRate:
                 case TimeoutReason.RequestBodyDrain:
@@ -245,8 +273,6 @@ internal async Task InnerProcessStreamsAsync<TContext>(IHttpApplication<TContext
             // TODO should we await the control stream task?
             var controlTask = CreateControlStream(application);
 
-            _timeoutControl.SetTimeout(Limits.KeepAliveTimeout.Ticks, TimeoutReason.KeepAlive);
-
             try
             {
                 while (_isClosed == 0)
@@ -277,29 +303,36 @@ internal async Task InnerProcessStreamsAsync<TContext>(IHttpApplication<TContext
                             streamContext.LocalEndPoint as IPEndPoint,
                             streamContext.RemoteEndPoint as IPEndPoint,
                             streamContext.Transport,
+                            this,
                             streamContext,
                             _serverSettings);
                         httpConnectionContext.TimeoutControl = _context.TimeoutControl;
 
+                        var streamId = streamIdFeature.StreamId;
+
                         if (!quicStreamFeature.CanWrite)
                         {
                             // Unidirectional stream
-                            var stream = new Http3ControlStream<TContext>(application, this, httpConnectionContext);
+                            var stream = new Http3ControlStream<TContext>(application, httpConnectionContext);
+                            lock (_streams)
+                            {
+                                _streams[streamId] = stream;
+                            }
+
                             ThreadPool.UnsafeQueueUserWorkItem(stream, preferLocal: false);
                         }
                         else
                         {
-                            var streamId = streamIdFeature.StreamId;
-
+                            // Request stream
                             UpdateHighestStreamId(streamId);
 
-                            var http3Stream = new Http3Stream<TContext>(application, this, httpConnectionContext);
-                            var stream = http3Stream;
+                            var stream = new Http3Stream<TContext>(application, httpConnectionContext);
                             lock (_streams)
                             {
                                 _activeRequestCount++;
-                                _streams[streamId] = http3Stream;
+                                _streams[streamId] = stream;
                             }
+
                             KestrelEventSource.Log.RequestQueuedStart(stream, AspNetCore.Http.HttpProtocol.Http3);
                             ThreadPool.UnsafeQueueUserWorkItem(stream, preferLocal: false);
                         }
@@ -363,8 +396,6 @@ internal async Task InnerProcessStreamsAsync<TContext>(IHttpApplication<TContext
                     {
                         await _streamCompletionAwaitable;
                     }
-
-                    _timeoutControl.CancelTimeout();
                 }
                 catch
                 {
@@ -410,17 +441,6 @@ private void UpdateConnectionState()
                         SendGoAway(GetHighestStreamId()).Preserve();
                     }
                 }
-                else
-                {
-                    // TODO should keep-alive timeout be a thing for HTTP/3? MsQuic currently tracks this for us?
-                    if (_timeoutControl.TimerReason == TimeoutReason.None)
-                    {
-                        _timeoutControl.SetTimeout(Limits.KeepAliveTimeout.Ticks, TimeoutReason.KeepAlive);
-                    }
-
-                    // Only reason should be keep-alive.
-                    Debug.Assert(_timeoutControl.TimerReason == TimeoutReason.KeepAlive);
-                }
             }
         }
 
@@ -450,11 +470,12 @@ private async ValueTask<Http3ControlStream> CreateNewUnidirectionalStreamAsync<T
                 streamContext.LocalEndPoint as IPEndPoint,
                 streamContext.RemoteEndPoint as IPEndPoint,
                 streamContext.Transport,
+                this,
                 streamContext,
                 _serverSettings);
             httpConnectionContext.TimeoutControl = _context.TimeoutControl;
 
-            return new Http3ControlStream<TContext>(application, this, httpConnectionContext);
+            return new Http3ControlStream<TContext>(application, httpConnectionContext);
         }
 
         private ValueTask<FlushResult> SendGoAway(long id)
@@ -466,33 +487,10 @@ private ValueTask<FlushResult> SendGoAway(long id)
                     return OutboundControlStream.SendGoAway(id);
                 }
             }
-            return new ValueTask<FlushResult>();
+            return default;
         }
 
-        public void ApplyMaxHeaderListSize(long value)
-        {
-        }
-
-        internal void ApplyBlockedStream(long value)
-        {
-        }
-
-        internal void ApplyMaxTableCapacity(long value)
-        {
-        }
-
-        internal void RemoveStream(long streamId)
-        {
-            lock (_streams)
-            {
-                _activeRequestCount--;
-                _streams.Remove(streamId);
-            }
-
-            _streamCompletionAwaitable.Complete();
-        }
-
-        public bool SetInboundControlStream(Http3ControlStream stream)
+        public bool OnInboundControlStream(Http3ControlStream stream)
         {
             lock (_sync)
             {
@@ -505,7 +503,7 @@ public bool SetInboundControlStream(Http3ControlStream stream)
             }
         }
 
-        public bool SetInboundEncoderStream(Http3ControlStream stream)
+        public bool OnInboundEncoderStream(Http3ControlStream stream)
         {
             lock (_sync)
             {
@@ -518,7 +516,7 @@ public bool SetInboundEncoderStream(Http3ControlStream stream)
             }
         }
 
-        public bool SetInboundDecoderStream(Http3ControlStream stream)
+        public bool OnInboundDecoderStream(Http3ControlStream stream)
         {
             lock (_sync)
             {
@@ -531,6 +529,38 @@ public bool SetInboundDecoderStream(Http3ControlStream stream)
             }
         }
 
+        public void OnStreamCompleted(IHttp3Stream stream)
+        {
+            lock (_streams)
+            {
+                _activeRequestCount--;
+                _streams.Remove(stream.StreamId);
+            }
+
+            _streamCompletionAwaitable.Complete();
+        }
+
+        public void OnStreamConnectionError(Http3ConnectionErrorException ex)
+        {
+            Log.Http3ConnectionError(ConnectionId, ex);
+            Abort(new ConnectionAbortedException(ex.Message, ex), ex.ErrorCode);
+        }
+
+        public void OnInboundControlStreamSetting(Http3SettingType type, long value)
+        {
+            switch (type)
+            {
+                case Http3SettingType.QPackMaxTableCapacity:
+                    break;
+                case Http3SettingType.MaxFieldSectionSize:
+                    break;
+                case Http3SettingType.QPackBlockedStreams:
+                    break;
+                default:
+                    throw new InvalidOperationException("Unexpected setting: " + type);
+            }
+        }
+
         private static class GracefulCloseInitiator
         {
             public const int None = 0;