Merged PR 20331: [5.0] MSRC 68921 - Remove unused headers from collec…

…tion in HTTP/2

MSRC Case Opened: 68921 - ASP.NET  Core - Kestrel overpooling of HTTP/2 and HTTP/3 request headers leads to DoS CRM:0802002372

Summary of the changes (Less than 80 chars)

## Description

Kestrel now correctly calls OnHeadersComplete after parsing request headers so unused headers are removed from headers collection. Prevents headers building up over time and exhausting memory.

Note that the change to the 5.0 branch only changes HTTP/2. HTTP/3 in .NET 5 is very experimental and isn't used.

## Customer Impact

Potential DoS attack

## Regression?

- [ ] Yes
- [x] No

[If yes, specify the version the behavior has regressed from]

## Risk

- [ ] High
- [x] Medium
- [ ] Low

The change is small but this code is on the critical path of a request. It is executed with every HTTP/2 and HTTP/3 request.

## Verification

- [X] Manual (required)
- [x] Automated

## Packaging changes reviewed?

- [ ] Yes
- [ ] No
- [x] N/A

----

## When servicing release/2.1

- [ ] Make necessary changes in eng/PatchConfig.props

src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs

@@ -975,6 +975,8 @@ private Task DecodeHeadersAsync(bool endHeaders, in ReadOnlySequence<byte> paylo
 
                 if (endHeaders)
                 {
+                    _currentHeadersStream.OnHeadersComplete();
+
                     StartStream();
                     ResetRequestHeaderParsingState();
                 }

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs

@@ -8,6 +8,7 @@
 using System.Linq;
 using System.Net.Http;
 using System.Net.Http.HPack;
+using System.Reflection;
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
@@ -207,6 +208,73 @@ public async Task RequestHeaderStringReuse_MultipleStreams_KnownHeaderReused()
             await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);
         }
 
+        [Fact]
+        public async Task RequestHeaderStringReuse_MultipleStreams_KnownHeaderClearedIfNotReused()
+        {
+            const BindingFlags privateFlags = BindingFlags.NonPublic | BindingFlags.Instance;
+
+            IEnumerable<KeyValuePair<string, string>> requestHeaders1 = new[]
+            {
+                new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/hello"),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+                new KeyValuePair<string, string>(HeaderNames.Authority, "localhost:80"),
+                new KeyValuePair<string, string>(HeaderNames.ContentType, "application/json")
+            };
+
+            // Note: No content-type
+            IEnumerable<KeyValuePair<string, string>> requestHeaders2 = new[]
+            {
+                new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/hello"),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+                new KeyValuePair<string, string>(HeaderNames.Authority, "localhost:80")
+            };
+
+            await InitializeConnectionAsync(_noopApplication);
+
+            await StartStreamAsync(1, requestHeaders1, endStream: true);
+
+            await ExpectAsync(Http2FrameType.HEADERS,
+                withLength: 36,
+                withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
+                withStreamId: 1);
+
+            // TriggerTick will trigger the stream to be returned to the pool so we can assert it
+            TriggerTick();
+
+            // Stream has been returned to the pool
+            Assert.Equal(1, _connection.StreamPool.Count);
+            Assert.True(_connection.StreamPool.TryPeek(out var stream1));
+
+            // Hacky but required because header references is private.
+            var headerReferences1 = typeof(HttpRequestHeaders).GetField("_headers", privateFlags).GetValue(stream1.RequestHeaders);
+            var contentTypeValue1 = (StringValues)headerReferences1.GetType().GetField("_ContentType").GetValue(headerReferences1);
+
+            await StartStreamAsync(3, requestHeaders2, endStream: true);
+
+            await ExpectAsync(Http2FrameType.HEADERS,
+                withLength: 6,
+                withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
+                withStreamId: 3);
+
+            // TriggerTick will trigger the stream to be returned to the pool so we can assert it
+            TriggerTick();
+
+            // Stream has been returned to the pool
+            Assert.Equal(1, _connection.StreamPool.Count);
+            Assert.True(_connection.StreamPool.TryPeek(out var stream2));
+
+            // Hacky but required because header references is private.
+            var headerReferences2 = typeof(HttpRequestHeaders).GetField("_headers", privateFlags).GetValue(stream2.RequestHeaders);
+            var contentTypeValue2 = (StringValues)headerReferences2.GetType().GetField("_ContentType").GetValue(headerReferences2);
+
+            Assert.Equal("application/json", contentTypeValue1);
+            Assert.Equal(StringValues.Empty, contentTypeValue2);
+
+            await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);
+        }
+
         [Fact]
         public async Task StreamPool_SingleStream_ReturnedToPool()
         {