Update

JamesNK · JamesNK · commit 79a133866234 · 2021-06-17T20:38:40.000+12:00
diff --git a/src/Servers/Kestrel/Core/src/Http3Limits.cs b/src/Servers/Kestrel/Core/src/Http3Limits.cs
@@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
     /// </summary>
     public class Http3Limits
     {
-        internal const int DefaultMaxRequestHeaderFieldSize = 8192;
+        internal const int DefaultMaxRequestHeaderFieldSize = 16 * 1024;
 
         private int _headerTableSize;
         private int _maxRequestHeaderFieldSize = DefaultMaxRequestHeaderFieldSize;
@@ -39,7 +39,7 @@ internal int HeaderTableSize
         /// <summary>
         /// Indicates the size of the maximum allowed size of a request header field sequence. This limit applies to both name and value sequences in their compressed and uncompressed representations.
         /// <para>
-        /// Value must be greater than 0, defaults to 8192.
+        /// Value must be greater than 0, defaults to 2^14 (16,384).
         /// </para>
         /// </summary>
         public int MaxRequestHeaderFieldSize
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3FrameWriter.cs
@@ -265,7 +265,7 @@ internal static int WriteHeader(Http3RawFrame frame, PipeWriter output)
             return totalLength;
         }
 
-        public ValueTask<FlushResult> WriteResponseTrailers(HttpResponseTrailers headers)
+        public ValueTask<FlushResult> WriteResponseTrailersAsync(long streamId, HttpResponseTrailers headers)
         {
             lock (_writeLock)
             {
@@ -283,7 +283,7 @@ public ValueTask<FlushResult> WriteResponseTrailers(HttpResponseTrailers headers
                 }
                 catch (QPackEncodingException ex)
                 {
-                    //_log.HPackEncodingError(_connectionId, streamId, hex);
+                    _log.QPackEncodingError(_connectionId, streamId, ex);
                     _http3Stream.Abort(new ConnectionAbortedException(ex.Message, ex), Http3ErrorCode.InternalError);
                 }
 
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3OutputProducer.cs
@@ -352,7 +352,7 @@ private async ValueTask<FlushResult> ProcessDataWrites()
                         }
 
                         _stream.ResponseTrailers.SetReadOnly();
-                        flushResult = await _frameWriter.WriteResponseTrailers(_stream.ResponseTrailers);
+                        flushResult = await _frameWriter.WriteResponseTrailersAsync(_stream.StreamId, _stream.ResponseTrailers);
                     }
                     else if (readResult.IsCompleted)
                     {
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs
@@ -487,8 +487,16 @@ private async Task ProcessHeadersFrameAsync<TContext>(IHttpApplication<TContext>
                 _requestHeaderParsingState = RequestHeaderParsingState.Trailers;
             }
 
-            QPackDecoder.Decode(payload, handler: this);
-            QPackDecoder.Reset();
+            try
+            {
+                QPackDecoder.Decode(payload, handler: this);
+                QPackDecoder.Reset();
+            }
+            catch (QPackDecodingException ex)
+            {
+                Log.QPackDecodingError(ConnectionId, StreamId, ex);
+                throw new Http3StreamErrorException(ex.Message, Http3ErrorCode.InternalError);
+            }
 
             switch (_requestHeaderParsingState)
             {
diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IKestrelTrace.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/IKestrelTrace.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Net.Http;
 using System.Net.Http.HPack;
+using System.Net.Http.QPack;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3;
@@ -92,5 +93,9 @@ internal interface IKestrelTrace : ILogger
         void Http3FrameReceived(string connectionId, long streamId, Http3RawFrame frame);
 
         void Http3FrameSending(string connectionId, long streamId, Http3RawFrame frame);
+
+        void QPackDecodingError(string connectionId, long streamId, QPackDecodingException ex);
+
+        void QPackEncodingError(string connectionId, long streamId, QPackEncodingException ex);
     }
 }
diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Net.Http;
 using System.Net.Http.HPack;
+using System.Net.Http.QPack;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3;
@@ -151,6 +152,14 @@ internal class KestrelTrace : IKestrelTrace
                 @"Connection id ""{ConnectionId}"" sending {type} frame for stream ID {id} with length {length}.",
                 skipEnabledCheck: true);
 
+        private static readonly Action<ILogger, string, long, Exception> _qpackDecodingError =
+            LoggerMessage.Define<string, long>(LogLevel.Debug, new EventId(48, "QPackDecodingError"),
+                @"Connection id ""{ConnectionId}"": QPACK decoding error while decoding headers for stream ID {StreamId}.");
+
+        private static readonly Action<ILogger, string, long, Exception> _qpackEncodingError =
+            LoggerMessage.Define<string, long>(LogLevel.Information, new EventId(49, "QPackEncodingError"),
+                @"Connection id ""{ConnectionId}"": QPACK encoding error while encoding headers for stream ID {StreamId}.");
+
         protected readonly ILogger _generalLogger;
         protected readonly ILogger _badRequestsLogger;
         protected readonly ILogger _connectionsLogger;
@@ -382,6 +391,16 @@ public void Http3FrameSending(string connectionId, long streamId, Http3RawFrame
             }
         }
 
+        public virtual void QPackDecodingError(string connectionId, long streamId, QPackDecodingException ex)
+        {
+            _qpackDecodingError(_http3Logger, connectionId, streamId, ex);
+        }
+
+        public virtual void QPackEncodingError(string connectionId, long streamId, QPackEncodingException ex)
+        {
+            _qpackEncodingError(_http3Logger, connectionId, streamId, ex);
+        }
+
         public virtual void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
             => _generalLogger.Log(logLevel, eventId, state, exception, formatter);
 
diff --git a/src/Servers/Kestrel/perf/Microbenchmarks/Mocks/MockTrace.cs b/src/Servers/Kestrel/perf/Microbenchmarks/Mocks/MockTrace.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Net.Http;
 using System.Net.Http.HPack;
+using System.Net.Http.QPack;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
@@ -66,5 +67,7 @@ public void Http3ConnectionClosed(string connectionId, long highestOpenedStreamI
         public void Http3StreamAbort(string traceIdentifier, Http3ErrorCode error, ConnectionAbortedException abortReason) { }
         public void Http3FrameReceived(string connectionId, long streamId, Http3RawFrame frame) { }
         public void Http3FrameSending(string connectionId, long streamId, Http3RawFrame frame) { }
+        public void QPackDecodingError(string connectionId, long streamId, QPackDecodingException ex) { }
+        public void QPackEncodingError(string connectionId, long streamId, QPackEncodingException ex) { }
     }
 }
diff --git a/src/Servers/Kestrel/shared/test/CompositeKestrelTrace.cs b/src/Servers/Kestrel/shared/test/CompositeKestrelTrace.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Net.Http;
 using System.Net.Http.HPack;
+using System.Net.Http.QPack;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Server.Kestrel.Core;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal;
@@ -280,5 +281,17 @@ public void Http3FrameSending(string connectionId, long streamId, Http3RawFrame
             _trace1.Http3FrameSending(connectionId, streamId, frame);
             _trace2.Http3FrameSending(connectionId, streamId, frame);
         }
+
+        public void QPackDecodingError(string connectionId, long streamId, QPackDecodingException ex)
+        {
+            _trace1.QPackDecodingError(connectionId, streamId, ex);
+            _trace2.QPackDecodingError(connectionId, streamId, ex);
+        }
+
+        public void QPackEncodingError(string connectionId, long streamId, QPackEncodingException ex)
+        {
+            _trace1.QPackEncodingError(connectionId, streamId, ex);
+            _trace2.QPackEncodingError(connectionId, streamId, ex);
+        }
     }
 }
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs
@@ -111,16 +111,16 @@ public async Task RequestHeadersMaxRequestHeaderFieldSize_EndsStream()
                 new KeyValuePair<string, string>(HeaderNames.Path, "/"),
                 new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
                 new KeyValuePair<string, string>(HeaderNames.Authority, "localhost:80"),
-                new KeyValuePair<string, string>("test", new string('a', 10000))
+                new KeyValuePair<string, string>("test", new string('a', 20000))
             };
 
             var requestStream = await InitializeConnectionAndStreamsAsync(_echoApplication);
 
             await requestStream.SendHeadersAsync(headers);
-            await requestStream.SendDataAsync(Encoding.ASCII.GetBytes("Hello world"));
 
-            // TODO figure out how to test errors for request streams that would be set on the Quic Stream.
-            await requestStream.ExpectReceiveEndOfStream();
+            await requestStream.WaitForStreamErrorAsync(
+                Http3ErrorCode.InternalError,
+                "The HTTP headers length exceeded the set limit of 16384 bytes.");
         }
 
         [Fact]

Original file line number	Diff line number	Diff line change
`@@ -265,7 +265,7 @@ internal static int WriteHeader(Http3RawFrame frame, PipeWriter output)`
`265`	`265`	`return totalLength;`
`266`	`266`	`}`
`267`	`267`
`268`		`- public ValueTask<FlushResult> WriteResponseTrailers(HttpResponseTrailers headers)`
	`268`	`+ public ValueTask<FlushResult> WriteResponseTrailersAsync(long streamId, HttpResponseTrailers headers)`
`269`	`269`	`{`
`270`	`270`	`lock (_writeLock)`
`271`	`271`	`{`
`@@ -283,7 +283,7 @@ public ValueTask<FlushResult> WriteResponseTrailers(HttpResponseTrailers headers`
`283`	`283`	`}`
`284`	`284`	`catch (QPackEncodingException ex)`
`285`	`285`	`{`
`286`		`- //_log.HPackEncodingError(_connectionId, streamId, hex);`
	`286`	`+ _log.QPackEncodingError(_connectionId, streamId, ex);`
`287`	`287`	`_http3Stream.Abort(new ConnectionAbortedException(ex.Message, ex), Http3ErrorCode.InternalError);`
`288`	`288`	`}`
`289`	`289`
Original file line number	Diff line number	Diff line change
`@@ -352,7 +352,7 @@ private async ValueTask<FlushResult> ProcessDataWrites()`
`352`	`352`	`}`
`353`	`353`
`354`	`354`	`_stream.ResponseTrailers.SetReadOnly();`
`355`		`- flushResult = await _frameWriter.WriteResponseTrailers(_stream.ResponseTrailers);`
	`355`	`+ flushResult = await _frameWriter.WriteResponseTrailersAsync(_stream.StreamId, _stream.ResponseTrailers);`
`356`	`356`	`}`
`357`	`357`	`else if (readResult.IsCompleted)`
`358`	`358`	`{`