diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 9f70362b493b..eb2915bbf946 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -544,7 +544,7 @@ public void OnTrailer(ReadOnlySpan name, ReadOnlySpan value) IncrementRequestHeadersCount(); string key = name.GetHeaderName(); - var valueStr = value.GetRequestHeaderString(key, HttpRequestHeaders.EncodingSelector, checkForNewlineChars: false); + var valueStr = value.GetRequestHeaderString(key, HttpRequestHeaders.EncodingSelector, checkForNewlineChars: true); RequestTrailers.Append(key, valueStr); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 58de076a029f..4cbc9c72ae78 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -1527,7 +1527,7 @@ private void OnHeaderCore(HeaderType headerType, int? staticTableIndex, ReadOnly { UpdateHeaderParsingState(value, GetPseudoHeaderField(name)); - _currentHeadersStream.OnHeader(name, value, checkForNewlineChars: false); + _currentHeadersStream.OnHeader(name, value, checkForNewlineChars: true); } break; case HeaderType.NameAndValue: diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index 5b2fbaf46e62..0d8b2b68aebe 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -5591,25 +5591,46 @@ public static TheoryData IllegalTrailerData get { // We can't use HPackEncoder here because it will convert header names to lowercase - var data = new TheoryData(); + var data = new TheoryData + { + // Indexed Header Field - :method: GET + { new byte[] { 0x82 }, CoreStrings.HttpErrorTrailersContainPseudoHeaderField }, - // Indexed Header Field - :method: GET - data.Add(new byte[] { 0x82 }, CoreStrings.HttpErrorTrailersContainPseudoHeaderField); + // Indexed Header Field - :path: / + { new byte[] { 0x84 }, CoreStrings.HttpErrorTrailersContainPseudoHeaderField }, - // Indexed Header Field - :path: / - data.Add(new byte[] { 0x84 }, CoreStrings.HttpErrorTrailersContainPseudoHeaderField); + // Indexed Header Field - :scheme: http + { new byte[] { 0x86 }, CoreStrings.HttpErrorTrailersContainPseudoHeaderField }, - // Indexed Header Field - :scheme: http - data.Add(new byte[] { 0x86 }, CoreStrings.HttpErrorTrailersContainPseudoHeaderField); + // Literal Header Field without Indexing - Indexed Name - :authority: 127.0.0.1 + { + new byte[] { 0x01, 0x09 }.Concat(Encoding.ASCII.GetBytes("127.0.0.1")).ToArray(), + CoreStrings.HttpErrorTrailersContainPseudoHeaderField + }, - // Literal Header Field without Indexing - Indexed Name - :authority: 127.0.0.1 - data.Add(new byte[] { 0x01, 0x09 }.Concat(Encoding.ASCII.GetBytes("127.0.0.1")).ToArray(), CoreStrings.HttpErrorTrailersContainPseudoHeaderField); + // Literal Header Field without Indexing - New Name - contains-Uppercase: 0 + { + new byte[] { 0x00, 0x12 }.Concat(Encoding.ASCII.GetBytes("contains-Uppercase")).Concat(new byte[] { 0x01, (byte)'0' }).ToArray(), + CoreStrings.HttpErrorTrailerNameUppercase + }, + + //Invalid header CR - contains-cr: \r + { + new byte[]{0x00, 0x0B}.Concat(Encoding.ASCII.GetBytes("contains-cr")).Concat(new byte[]{0x01, 0x0D}).ToArray(), + CoreStrings.BadRequest_MalformedRequestInvalidHeaders + }, - // Literal Header Field without Indexing - New Name - contains-Uppercase: 0 - data.Add(new byte[] { 0x00, 0x12 } - .Concat(Encoding.ASCII.GetBytes("contains-Uppercase")) - .Concat(new byte[] { 0x01, (byte)'0' }) - .ToArray(), CoreStrings.HttpErrorTrailerNameUppercase); + //Invalid header LF - contains-lf: \n + { + new byte[]{0x00, 0x0B}.Concat(Encoding.ASCII.GetBytes("contains-lf")).Concat(new byte[]{0x01, 0x0A}).ToArray(), + CoreStrings.BadRequest_MalformedRequestInvalidHeaders + }, + //Invalid header CR and LF - contains-crlf: \r\n + { + new byte[]{0x00, 0x0D}.Concat(Encoding.ASCII.GetBytes("contains-crlf")).Concat(new byte[]{0x02, 0x0D, 0x0A}).ToArray(), + CoreStrings.BadRequest_MalformedRequestInvalidHeaders + } + }; return data; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 0e1968c5677c..a963c5e5ed8a 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -5374,4 +5374,30 @@ await ExpectAsync(Http2FrameType.RST_STREAM, Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[InternalHeaderNames.Status]); } + + [Theory] + [InlineData("\r")] + [InlineData("\n")] + [InlineData("\r\n")] + public async Task OnDynamicIndexedHeader_NewlineCharactersInValue_ThrowsConnectionError(string newlineCharacters) + { + await InitializeConnectionAsync(_noopApplication); + + // Start a request header block without END_HEADERS so the connection has an active stream receiving headers. + await SendHeadersAsync(streamId: 1, flags: Http2HeadersFrameFlags.END_STREAM, headers: _browserRequestHeaders); + + var value = Encoding.ASCII.GetBytes(newlineCharacters); + + // Simulate HPackDecoder resolving a fully indexed dynamic-table entry. + var exception = Assert.Throws(() => + _connection.OnDynamicIndexedHeader(index: null, name: "contains-newline"u8, value)); + + Assert.Equal(Http2ErrorCode.PROTOCOL_ERROR, exception.ErrorCode); + + Assert.Contains(CoreStrings.BadRequest_MalformedRequestInvalidHeaders, exception.Message); + + // Finish the intentionally incomplete header block and shut down normally. + await SendEmptyContinuationFrameAsync(streamId: 1, flags: Http2ContinuationFrameFlags.END_HEADERS); + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: true); + } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs index a59a261e39fe..5ba31bc822c0 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs @@ -2028,6 +2028,33 @@ public async Task RequestTrailers_CanReadTrailersFromRequest() await requestStream.ExpectReceiveEndOfStream(); } + [Theory] + [InlineData("\r")] + [InlineData("\n")] + [InlineData("\r\n")] + public async Task RequestTrailers_ContainsNewlines(string newlineChars) + { + + var headers = new[] + { + new KeyValuePair(InternalHeaderNames.Method, "GET"), + new KeyValuePair(InternalHeaderNames.Path, "/"), + new KeyValuePair(InternalHeaderNames.Scheme, "http"), + }; + + var trailerWithNewlines = new[] + { + new KeyValuePair("contains-newlines", newlineChars), + }; + + var requestStream = await Http3Api.InitializeConnectionAndStreamsAsync(_echoApplication, headers, endStream: false); + await requestStream.SendHeadersAsync(trailerWithNewlines, endStream: true); + + await requestStream.WaitForStreamErrorAsync( + Http3ErrorCode.MessageError, + expectedErrorMessage: CoreStrings.BadRequest_MalformedRequestInvalidHeaders); + } + [Fact] public async Task FrameAfterTrailers_UnexpectedFrameError() {