Remove connection-specific headeres for HTTP/2 and HTTP/3

src/Http/Headers/src/HeaderNames.cs

@@ -65,6 +65,7 @@ public static class HeaderNames
         public static readonly string Pragma = "Pragma";
         public static readonly string ProxyAuthenticate = "Proxy-Authenticate";
         public static readonly string ProxyAuthorization = "Proxy-Authorization";
+        public static readonly string ProxyConnection = "Proxy-Connection";
         public static readonly string Range = "Range";
         public static readonly string Referer = "Referer";
         public static readonly string RetryAfter = "Retry-After";

src/Http/Headers/src/PublicAPI.Unshipped.txt

@@ -1,3 +1,3 @@
 #nullable enable
 Microsoft.Net.Http.Headers.MediaTypeHeaderValue.MatchesMediaType(Microsoft.Extensions.Primitives.StringSegment otherMediaType) -> bool
-
+static readonly Microsoft.Net.Http.Headers.HeaderNames.ProxyConnection -> string!

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

@@ -1088,6 +1088,26 @@ private HttpResponseHeaders CreateResponseHeaders(bool appCompleted)
             var hasConnection = responseHeaders.HasConnection;
             var hasTransferEncoding = responseHeaders.HasTransferEncoding;
 
+            // We opt to remove the following headers from an HTTP/2+ response since their presence would be considered a protocol violation.
+            // This is done quietly because these headers are valid in other contexts and this saves the app from being broken by
+            // low level protocol details. Http.Sys also removes these headers silently.
+            //
+            // https://tools.ietf.org/html/rfc7540#section-8.1.2.2
+            // "This means that an intermediary transforming an HTTP/1.x message to HTTP/2 will need to remove any header fields
+            // nominated by the Connection header field, along with the Connection header field itself.
+            // Such intermediaries SHOULD also remove other connection-specific header fields, such as Keep-Alive,
+            // Proxy-Connection, Transfer-Encoding, and Upgrade, even if they are not nominated by the Connection header field."
+            //
+            // Http/3 has a similar requirement: https://quicwg.org/base-drafts/draft-ietf-quic-http.html#name-field-formatting-and-compre
+            if (_httpVersion > Http.HttpVersion.Http11 && responseHeaders.HasInvalidH2H3Headers)
+            {
+                responseHeaders.ClearInvalidH2H3Headers();
+                hasTransferEncoding = false;
+                hasConnection = false;
+
+                Log.InvalidResponseHeaderRemoved();
+            }
+
             if (_keepAlive &&
                 hasConnection &&
                 (HttpHeaders.ParseConnection(responseHeaders.HeaderConnection) & ConnectionOptions.KeepAlive) == 0)

src/Servers/Kestrel/Core/src/Internal/Infrastructure/IKestrelTrace.cs

@@ -76,5 +76,7 @@ internal interface IKestrelTrace : ILogger
         void Http2FrameSending(string connectionId, Http2Frame frame);
 
         void Http2MaxConcurrentStreamsReached(string connectionId);
+
+        void InvalidResponseHeaderRemoved();
     }
 }

src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs

@@ -117,6 +117,10 @@ internal class KestrelTrace : IKestrelTrace
             LoggerMessage.Define<string>(LogLevel.Debug, new EventId(40, nameof(Http2MaxConcurrentStreamsReached)),
                 @"Connection id ""{ConnectionId}"" reached the maximum number of concurrent HTTP/2 streams allowed.");
 
+        private static readonly Action<ILogger, Exception> _invalidResponseHeaderRemoved =
+            LoggerMessage.Define(LogLevel.Warning, new EventId(41, nameof(InvalidResponseHeaderRemoved)),
+                "One or more of the following response headers have been removed because they are invalid for HTTP/2 and HTTP/3 responses: 'Connection', 'Transfer-Encoding', 'Keep-Alive', 'Upgrade' and 'Proxy-Connection'.");
+
         protected readonly ILogger _logger;
 
         public KestrelTrace(ILogger logger)
@@ -295,6 +299,11 @@ public void Http2MaxConcurrentStreamsReached(string connectionId)
             _http2MaxConcurrentStreamsReached(_logger, connectionId, null);
         }
 
+        public void InvalidResponseHeaderRemoved()
+        {
+            _invalidResponseHeaderRemoved(_logger, null);
+        }
+
         public virtual void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
             => _logger.Log(logLevel, eventId, state, exception, formatter);
 

src/Servers/Kestrel/perf/Kestrel.Performance/Mocks/MockTrace.cs

@@ -56,5 +56,6 @@ public void Http2ConnectionClosed(string connectionId, int highestOpenedStreamId
         public void Http2FrameReceived(string connectionId, Http2Frame frame) { }
         public void Http2FrameSending(string connectionId, Http2Frame frame) { }
         public void Http2MaxConcurrentStreamsReached(string connectionId) { }
+        public void InvalidResponseHeaderRemoved() { }
     }
 }

src/Servers/Kestrel/shared/KnownHeaders.cs

@@ -8,7 +8,6 @@
 using System.Linq;
 using System.Net.Http.HPack;
 using System.Text;
-using Microsoft.Net.Http.Headers;
 
 namespace CodeGenerator
 {
@@ -17,6 +16,7 @@ public class KnownHeaders
         public readonly static KnownHeader[] RequestHeaders;
         public readonly static KnownHeader[] ResponseHeaders;
         public readonly static KnownHeader[] ResponseTrailers;
+        public readonly static long InvalidH2H3ResponseHeadersBits;
 
         static KnownHeaders()
         {
@@ -159,6 +159,7 @@ static KnownHeaders()
                 "ETag",
                 "Location",
                 "Proxy-Authenticate",
+                "Proxy-Connection",
                 "Retry-After",
                 "Server",
                 "Set-Cookie",
@@ -198,6 +199,20 @@ static KnownHeaders()
                 PrimaryHeader = responsePrimaryHeaders.Contains(header)
             })
             .ToArray();
+
+            var invalidH2H3ResponseHeaders = new[]
+            {
+                "Connection",
+                "Transfer-Encoding",
+                "Keep-Alive",
+                "Upgrade",
+                "Proxy-Connection"
+            };
+
+            InvalidH2H3ResponseHeadersBits = ResponseHeaders
+                .Where(header => invalidH2H3ResponseHeaders.Contains(header.Name))
+                .Select(header => 1L << header.Index)
+                .Aggregate((a, b) => a | b);
         }
 
         static string Each<T>(IEnumerable<T> values, Func<T, string> formatter)
@@ -1015,6 +1030,11 @@ protected override bool CopyToFast(KeyValuePair<string, StringValues>[] array, i
             return true;
         }}
         {(loop.ClassName == "HttpResponseHeaders" ? $@"
+        internal bool HasInvalidH2H3Headers => (_bits & {InvalidH2H3ResponseHeadersBits}) != 0;
+        internal void ClearInvalidH2H3Headers()
+        {{
+            _bits &= ~{InvalidH2H3ResponseHeadersBits};
+        }}
         internal unsafe void CopyToFast(ref BufferWriter<PipeWriter> output)
         {{
             var tempBits = (ulong)_bits | (_contentLength.HasValue ? {"0x" + (1L << 63).ToString("x" , CultureInfo.InvariantCulture)}L : 0);

src/Servers/Kestrel/shared/test/CompositeKestrelTrace.cs

@@ -235,5 +235,11 @@ public void Http2MaxConcurrentStreamsReached(string connectionId)
             _trace1.Http2MaxConcurrentStreamsReached(connectionId);
             _trace2.Http2MaxConcurrentStreamsReached(connectionId);
         }
+
+        public void InvalidResponseHeaderRemoved()
+        {
+            _trace1.InvalidResponseHeaderRemoved();
+            _trace2.InvalidResponseHeaderRemoved();
+        }
     }
 }

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

@@ -4785,5 +4785,48 @@ await WaitForConnectionErrorAsync<Http2ConnectionErrorException>(
                 expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR,
                 expectedErrorMessage: CoreStrings.BadRequest_MalformedRequestInvalidHeaders);
         }
+
+        [Fact]
+        public async Task RemoveConnectionSpecificHeaders()
+        {
+            await InitializeConnectionAsync(async context =>
+            {
+                var response = context.Response;
+
+                response.Headers.Add(HeaderNames.TransferEncoding, "chunked");
+                response.Headers.Add(HeaderNames.Upgrade, "websocket");
+                response.Headers.Add(HeaderNames.Connection, "Keep-Alive");
+                response.Headers.Add(HeaderNames.KeepAlive, "timeout=5, max=1000");
+                response.Headers.Add(HeaderNames.ProxyConnection, "keep-alive");
+                await response.WriteAsync("hello, world");
+            });
+
+            await StartStreamAsync(1, _browserRequestHeaders, endStream: true);
+
+            var headersFrame = await ExpectAsync(Http2FrameType.HEADERS,
+                withLength: 32,
+                withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS,
+                withStreamId: 1);
+            var dataFrame1 = await ExpectAsync(Http2FrameType.DATA,
+                withLength: 12,
+                withFlags: (byte)Http2DataFrameFlags.NONE,
+                withStreamId: 1);
+            await ExpectAsync(Http2FrameType.DATA,
+                withLength: 0,
+                withFlags: (byte)Http2DataFrameFlags.END_STREAM,
+                withStreamId: 1);
+
+            await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
+
+            _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this);
+
+            Assert.Equal(2, _decodedHeaders.Count);
+            Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
+            Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
+
+            Assert.True(_helloWorldBytes.AsSpan().SequenceEqual(dataFrame1.PayloadSequence.ToArray()));
+
+            Assert.Contains(LogMessages, m => m.Message.Equals("One or more of the following response headers have been removed because they are invalid for HTTP/2 and HTTP/3 responses: 'Connection', 'Transfer-Encoding', 'Keep-Alive', 'Upgrade' and 'Proxy-Connection'."));
+        }
     }
 }

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs

@@ -4,6 +4,7 @@
 using System.Net.Http;
 using System.Text;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Testing;
 using Microsoft.Net.Http.Headers;
@@ -586,6 +587,41 @@ public async Task ContentLength_Received_MultipleDataFrame_ReadViaPipe_Verified(
             Assert.Equal("0", responseHeaders[HeaderNames.ContentLength]);
         }
 
+        [Fact]
+        public async Task RemoveConnectionSpecificHeaders()
+        {
+            var headers = new[]
+            {
+                new KeyValuePair<string, string>(HeaderNames.Method, "Custom"),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/"),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+                new KeyValuePair<string, string>(HeaderNames.Authority, "localhost:80"),
+            };
+
+            var requestStream = await InitializeConnectionAndStreamsAsync(async context =>
+            {
+                var response = context.Response;
+
+                response.Headers.Add(HeaderNames.TransferEncoding, "chunked");
+                response.Headers.Add(HeaderNames.Upgrade, "websocket");
+                response.Headers.Add(HeaderNames.Connection, "Keep-Alive");
+                response.Headers.Add(HeaderNames.KeepAlive, "timeout=5, max=1000");
+                response.Headers.Add(HeaderNames.ProxyConnection, "keep-alive");
+
+                await response.WriteAsync("Hello world");
+            });
+
+            var doneWithHeaders = await requestStream.SendHeadersAsync(headers, endStream: true);
+
+            var responseHeaders = await requestStream.ExpectHeadersAsync();
+            Assert.Equal(2, responseHeaders.Count);
+
+            var responseData = await requestStream.ExpectDataAsync();
+            Assert.Equal("Hello world", Encoding.ASCII.GetString(responseData.ToArray()));
+
+            Assert.Contains(LogMessages, m => m.Message.Equals("One or more of the following response headers have been removed because they are invalid for HTTP/2 and HTTP/3 responses: 'Connection', 'Transfer-Encoding', 'Keep-Alive', 'Upgrade' and 'Proxy-Connection'."));
+        }
+
         [Fact(Skip = "Http3OutputProducer.Complete is called before input recognizes there is an error. Why is this different than HTTP/2?")]
         public async Task ContentLength_Received_NoDataFrames_Reset()
         {