diff --git a/src/Servers/Kestrel/Core/src/CoreStrings.resx b/src/Servers/Kestrel/Core/src/CoreStrings.resx index c9f39d1da13a..d4993d1abbc5 100644 --- a/src/Servers/Kestrel/Core/src/CoreStrings.resx +++ b/src/Servers/Kestrel/Core/src/CoreStrings.resx @@ -668,4 +668,4 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l Error initializing outbound control stream. - \ No newline at end of file + diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs index 3c66964cafd4..5c48037472c2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs @@ -16,14 +16,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http public class HttpParser : IHttpParser where TRequestHandler : IHttpHeadersHandler, IHttpRequestLineHandler { private readonly bool _showErrorDetails; + private readonly bool _enableLineFeedTerminator; - public HttpParser() : this(showErrorDetails: true) + public HttpParser() : this(showErrorDetails: true, enableLineFeedTerminator: false) { } - public HttpParser(bool showErrorDetails) + public HttpParser(bool showErrorDetails) : this(showErrorDetails, enableLineFeedTerminator: false) + { + } + + internal HttpParser(bool showErrorDetails, bool enableLineFeedTerminator) { _showErrorDetails = showErrorDetails; + _enableLineFeedTerminator = enableLineFeedTerminator; } // byte types don't have a data type annotation so we pre-cast them; to avoid in-place casts @@ -126,9 +132,15 @@ private void ParseRequestLine(TRequestHandler handler, ReadOnlySpan reques // Version + CR is 9 bytes which should take us to .Length // LF should have been dropped prior to method call - if ((uint)offset + 9 != (uint)requestLine.Length || requestLine[offset + sizeof(ulong)] != ByteCR) + if ((uint)offset + 9 != (uint)requestLine.Length || requestLine[offset + 8] != ByteCR) { - RejectRequestLine(requestLine); + // LF should have been dropped prior to method call + // If _enableLineFeedTerminator and offset + 8 is .Length, + // then requestLine is valid since it mean LF was the next char + if (!_enableLineFeedTerminator || (uint)offset + 8 != (uint)requestLine.Length) + { + RejectRequestLine(requestLine); + } } // Version @@ -152,143 +164,135 @@ public bool ParseHeaders(TRequestHandler handler, ref SequenceReader reade while (!reader.End) { var span = reader.UnreadSpan; + + // Size of header in the current span, if known + var length = -1; + while (span.Length > 0) { - var ch1 = (byte)0; - var ch2 = (byte)0; - var readAhead = 0; + // The size of the EOL terminator. Always -1 (no valid EOL), 1 (LF) or 2 (CRLF) + var eolSize = -1; - // Fast path, we're still looking at the same span - if (span.Length >= 2) - { - ch1 = span[0]; - ch2 = span[1]; - } - else if (reader.TryRead(out ch1)) // Possibly split across spans + // length can be set when the span is returned by ParseMultiSpanHeader + if (length == -1) { - // Note if we read ahead by 1 or 2 bytes - readAhead = (reader.TryRead(out ch2)) ? 2 : 1; + length = span.IndexOfAny(ByteCR, ByteLF); } - if (ch1 == ByteCR) + if (length != -1) { - // Check for final CRLF. - if (ch2 == ByteLF) - { - // If we got 2 bytes from the span directly so skip ahead 2 so that - // the reader's state matches what we expect - if (readAhead == 0) - { - reader.Advance(2); - } + // Validate the EOL terminator + eolSize = ParseHeaderLineEnd(span, length); - // Double CRLF found, so end of headers. - handler.OnHeadersComplete(endStream: false); - return true; - } - else if (readAhead == 1) + // Not valid + if (eolSize == -1) { - // Didn't read 2 bytes, reset the reader so we don't consume anything - reader.Rewind(1); - return false; + length = -1; } - - Debug.Assert(readAhead == 0 || readAhead == 2); - // Headers don't end in CRLF line. - - KestrelBadHttpRequestException.Throw(RequestRejectionReason.InvalidRequestHeadersNoCRLF); } - var length = 0; - // We only need to look for the end if we didn't read ahead; otherwise there isn't enough in - // in the span to contain a header. - if (readAhead == 0) + // Empty header (EOL only)? + if (length == 0) { - length = span.IndexOfAny(ByteCR, ByteLF); - // If not found length with be -1; casting to uint will turn it to uint.MaxValue - // which will be larger than any possible span.Length. This also serves to eliminate - // the bounds check for the next lookup of span[length] - if ((uint)length < (uint)span.Length) - { - // Early memory read to hide latency - var expectedCR = span[length]; - // Correctly has a CR, move to next - length++; - - if (expectedCR != ByteCR) - { - // Sequence needs to be CRLF not LF first. - RejectRequestHeader(span[..length]); - } + handler.OnHeadersComplete(endStream: false); + reader.Advance(eolSize); + return true; + } - if ((uint)length < (uint)span.Length) - { - // Early memory read to hide latency - var expectedLF = span[length]; - // Correctly has a LF, move to next - length++; - - if (expectedLF != ByteLF || - length < 5 || - // Exclude the CRLF from the headerLine and parse the header name:value pair - !TryTakeSingleHeader(handler, span[..(length - 2)])) - { - // Sequence needs to be CRLF and not contain an inner CR not part of terminator. - // Less than min possible headerSpan of 5 bytes a:b\r\n - // Not parsable as a valid name:value header pair. - RejectRequestHeader(span[..length]); - } + // If not found length will be -1; casting to uint will turn it to uint.MaxValue + // which will be larger than any possible span.Length. This also serves to eliminate + // the bounds check for the next lookup of span[length] + if ((uint)length < (uint)span.Length) + { + var lineLength = length + eolSize; - // Read the header successfully, skip the reader forward past the headerSpan. - span = span.Slice(length); - reader.Advance(length); - } - else - { - // No enough data, set length to 0. - length = 0; - } + if (length != 0 && !TryTakeSingleHeader(handler, span[..length])) + { + // Sequence needs to be CRLF and not contain an inner CR not part of terminator. + // Not parsable as a valid name:value header pair. + RejectRequestHeader(span[..lineLength]); } + + // Read the header successfully, skip the reader forward past the headerSpan. + span = span[lineLength..]; + reader.Advance(lineLength); } // End found in current span if (length > 0) { + length = -1; continue; } - // We moved the reader to look ahead 2 bytes so rewind the reader - if (readAhead > 0) - { - reader.Rewind(readAhead); - } + // Load next header line to parse as a span + span = ParseMultiSpanHeader(ref reader, out length); - length = ParseMultiSpanHeader(handler, ref reader); - if (length < 0) + // If there any remaining line? + if (length == -1 && span.Length == 0) { - // Not there return false; } - - reader.Advance(length); - // As we crossed spans set the current span to default - // so we move to the next span on the next iteration - span = default; } } return false; } - private int ParseMultiSpanHeader(TRequestHandler handler, ref SequenceReader reader) + // Returns the length of the line terminator (CRLF = 2, LF = 1) + // If no valid EOL is detected then -1 + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private int ParseHeaderLineEnd(ReadOnlySpan headerSpan, int headerLineLength) { + // This method needs to be called with a positive value representing the index of either CR or LF + Debug.Assert(headerLineLength >= 0); + + if (headerSpan[headerLineLength] == ByteCR) + { + // No more chars after CR? Don't consume an incomplete header + if (headerSpan.Length == headerLineLength + 1) + { + return -1; + } + + // CR must be followed by LF in all cases + if (headerSpan[headerLineLength + 1] != ByteLF) + { + if (headerLineLength == 0) + { + KestrelBadHttpRequestException.Throw(RequestRejectionReason.InvalidRequestHeadersNoCRLF); + } + else + { + RejectRequestHeader(headerSpan[..(headerLineLength + 2)]); + } + } + + return 2; + } + + if (_enableLineFeedTerminator) + { + return 1; + } + + // LF but not allowed + RejectRequestHeader(headerSpan[..(headerLineLength + 1)]); + + return 0; + } + + // Returns a span from the remaining sequence until the next valid EOL + private ReadOnlySpan ParseMultiSpanHeader(ref SequenceReader reader, out int length) + { + length = -1; + var currentSlice = reader.UnreadSequence; var lineEndPosition = currentSlice.PositionOfAny(ByteCR, ByteLF); if (lineEndPosition == null) { - // Not there. - return -1; + return ReadOnlySpan.Empty; } SequencePosition lineEnd; @@ -296,44 +300,27 @@ private int ParseMultiSpanHeader(TRequestHandler handler, ref SequenceReader.Empty; } // Advance 2 to include CR{LF?} in lineEnd lineEnd = currentSlice.GetPosition(2, lineEndPosition.Value); headerSpan = currentSlice.Slice(reader.Position, lineEnd).ToSpan(); - if (headerSpan.Length < 5) - { - // Less than min possible headerSpan is 5 bytes a:b\r\n - RejectRequestHeader(headerSpan); - } - - if (headerSpan[^2] != ByteCR) - { - // Sequence needs to be CRLF not LF first. - RejectRequestHeader(headerSpan[..^1]); - } - - if (headerSpan[^1] != ByteLF || - // Exclude the CRLF from the headerLine and parse the header name:value pair - !TryTakeSingleHeader(handler, headerSpan[..^2])) - { - // Sequence needs to be CRLF and not contain an inner CR not part of terminator. - // Not parsable as a valid name:value header pair. - RejectRequestHeader(headerSpan); - } - - return headerSpan.Length; + length = headerSpan.Length - 2; + return headerSpan; } private static bool TryTakeSingleHeader(TRequestHandler handler, ReadOnlySpan headerLine) diff --git a/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs b/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs index bee7ab9fce1e..fce1bab8db54 100644 --- a/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs +++ b/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs @@ -130,7 +130,7 @@ private static ServiceContext CreateServiceContext(IOptions(trace.IsEnabled(LogLevel.Information)), + HttpParser = new HttpParser(trace.IsEnabled(LogLevel.Information), serverOptions.EnableLineFeedTerminator), SystemClock = heartbeatManager, DateHeaderValueManager = dateHeaderValueManager, ConnectionManager = connectionManager, diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index cf930c744cbb..295326344878 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -176,6 +176,21 @@ internal bool EnableInsecureAbsoluteFormHostOverride /// internal bool IsDevCertLoaded { get; set; } + private bool? _enableLineFeedTerminator; + internal bool EnableLineFeedTerminator + { + get + { + if (!_enableLineFeedTerminator.HasValue) + { + _enableLineFeedTerminator = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.EnableLineFeedTerminator", out var enabled) && enabled; + } + + return _enableLineFeedTerminator.Value; + } + set => _enableLineFeedTerminator = value; + } + /// /// Specifies a configuration Action to run for each newly created endpoint. Calling this again will replace /// the prior action. diff --git a/src/Servers/Kestrel/Core/test/HttpParserTests.cs b/src/Servers/Kestrel/Core/test/HttpParserTests.cs index c3c4c84d0a24..0a380af50031 100644 --- a/src/Servers/Kestrel/Core/test/HttpParserTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpParserTests.cs @@ -1,11 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Buffers; -using System.Collections.Generic; -using System.Linq; -using System.Net.Http; using System.Text; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; @@ -13,7 +9,6 @@ using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging; using Moq; -using Xunit; using HttpMethod = Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpMethod; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests @@ -80,6 +75,46 @@ public void ParseRequestLineDoesNotConsumeIncompleteRequestLine(string requestLi [Theory] [MemberData(nameof(RequestLineInvalidData))] public void ParseRequestLineThrowsOnInvalidRequestLine(string requestLine) + { + // These should fail with or without quirk mode + + var mockTrace = new Mock(); + mockTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(true); + + var parser = CreateParser(mockTrace.Object); + var buffer = new ReadOnlySequence(Encoding.ASCII.GetBytes(requestLine)); + var requestHandler = new RequestHandler(); + +#pragma warning disable CS0618 // Type or member is obsolete + var exception = Assert.Throws(() => +#pragma warning restore CS0618 // Type or member is obsolete + ParseRequestLine(parser, requestHandler, buffer, out var consumed, out var examined)); + + Assert.Equal(CoreStrings.FormatBadRequest_InvalidRequestLine_Detail(requestLine[..^1].EscapeNonPrintable()), exception.Message); + Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); + } + + [Theory] + [MemberData(nameof(RequestLineInvalidDataLineFeedTerminator))] + public void ParseRequestSucceedsOnInvalidRequestLineLineFeedTerminator(string requestLine) + { + var mockTrace = new Mock(); + mockTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(true); + + var parser = CreateParser(mockTrace.Object, enableLineFeedTerminator: true); + var buffer = new ReadOnlySequence(Encoding.ASCII.GetBytes(requestLine)); + var requestHandler = new RequestHandler(); + + Assert.True(ParseRequestLine(parser, requestHandler, buffer, out var consumed, out var examined)); + } + + [Theory] + [MemberData(nameof(RequestLineInvalidDataLineFeedTerminator))] + public void ParseRequestLineThrowsOnInvalidRequestLineLineFeedTerminator(string requestLine) { var mockTrace = new Mock(); mockTrace @@ -237,6 +272,8 @@ public void ParseHeadersCanReadHeaderValueWithoutLeadingWhitespace() [InlineData("Cookie:\r\nConnection: close\r\n\r\n", "Cookie", "", "Connection", "close")] [InlineData("Connection: close\r\nCookie: \r\n\r\n", "Connection", "close", "Cookie", "")] [InlineData("Connection: close\r\nCookie:\r\n\r\n", "Connection", "close", "Cookie", "")] + [InlineData("a:b\r\n\r\n", "a", "b", null, null)] + [InlineData("a: b\r\n\r\n", "a", "b", null, null)] public void ParseHeadersCanParseEmptyHeaderValues( string rawHeaders, string expectedHeaderName1, @@ -254,6 +291,115 @@ public void ParseHeadersCanParseEmptyHeaderValues( VerifyRawHeaders(rawHeaders, expectedHeaderNames, expectedHeaderValues); } + [Theory] + [InlineData("Cookie: \n\r\n", "Cookie", "", null, null)] + [InlineData("Cookie:\n\r\n", "Cookie", "", null, null)] + [InlineData("Cookie: \nConnection: close\r\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Cookie: \r\nConnection: close\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Cookie:\nConnection: close\r\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Cookie:\r\nConnection: close\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Connection: close\nCookie: \r\n\r\n", "Connection", "close", "Cookie", "")] + [InlineData("Connection: close\r\nCookie: \n\r\n", "Connection", "close", "Cookie", "")] + [InlineData("Connection: close\nCookie:\r\n\r\n", "Connection", "close", "Cookie", "")] + [InlineData("Connection: close\r\nCookie:\n\r\n", "Connection", "close", "Cookie", "")] + [InlineData("a:b\n\r\n", "a", "b", null, null)] + [InlineData("a: b\n\r\n", "a", "b", null, null)] + [InlineData("a:b\n\n", "a", "b", null, null)] + [InlineData("a: b\n\n", "a", "b", null, null)] + public void ParseHeadersCantParseSingleLineFeedWihtoutLineFeedTerminatorEnabled( + string rawHeaders, + string expectedHeaderName1, + string expectedHeaderValue1, + string expectedHeaderName2, + string expectedHeaderValue2) + { + var expectedHeaderNames = expectedHeaderName2 == null + ? new[] { expectedHeaderName1 } + : new[] { expectedHeaderName1, expectedHeaderName2 }; + var expectedHeaderValues = expectedHeaderValue2 == null + ? new[] { expectedHeaderValue1 } + : new[] { expectedHeaderValue1, expectedHeaderValue2 }; + +#pragma warning disable CS0618 // Type or member is obsolete + Assert.Throws(() => VerifyRawHeaders(rawHeaders, expectedHeaderNames, expectedHeaderValues)); +#pragma warning restore CS0618 // Type or member is obsolete + } + + [Theory] + [InlineData("Cookie: \n\r\n", "Cookie", "", null, null)] + [InlineData("Cookie:\n\r\n", "Cookie", "", null, null)] + [InlineData("Cookie: \nConnection: close\r\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Cookie: \r\nConnection: close\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Cookie:\nConnection: close\r\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Cookie:\r\nConnection: close\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Connection: close\nCookie: \r\n\r\n", "Connection", "close", "Cookie", "")] + [InlineData("Connection: close\r\nCookie: \n\r\n", "Connection", "close", "Cookie", "")] + [InlineData("Connection: close\nCookie:\r\n\r\n", "Connection", "close", "Cookie", "")] + [InlineData("Connection: close\r\nCookie:\n\r\n", "Connection", "close", "Cookie", "")] + public void ParseHeadersCanParseSingleLineFeedWithLineFeedTerminatorEnabled( + string rawHeaders, + string expectedHeaderName1, + string expectedHeaderValue1, + string expectedHeaderName2, + string expectedHeaderValue2) + { + var expectedHeaderNames = expectedHeaderName2 == null + ? new[] { expectedHeaderName1 } + : new[] { expectedHeaderName1, expectedHeaderName2 }; + var expectedHeaderValues = expectedHeaderValue2 == null + ? new[] { expectedHeaderValue1 } + : new[] { expectedHeaderValue1, expectedHeaderValue2 }; + + VerifyRawHeaders(rawHeaders, expectedHeaderNames, expectedHeaderValues, enableLineFeedTerminator: true); + } + + [Theory] + [InlineData("a: b\r\n\n", "a", "b", null, null)] + [InlineData("a: b\n\n", "a", "b", null, null)] + [InlineData("a: b\nc: d\r\n\n", "a", "b", "c", "d")] + [InlineData("a: b\nc: d\n\n", "a", "b", "c", "d")] + public void ParseHeadersCantEndWithLineFeedTerminator( + string rawHeaders, + string expectedHeaderName1, + string expectedHeaderValue1, + string expectedHeaderName2, + string expectedHeaderValue2) + { + var expectedHeaderNames = expectedHeaderName2 == null + ? new[] { expectedHeaderName1 } + : new[] { expectedHeaderName1, expectedHeaderName2 }; + var expectedHeaderValues = expectedHeaderValue2 == null + ? new[] { expectedHeaderValue1 } + : new[] { expectedHeaderValue1, expectedHeaderValue2 }; + +#pragma warning disable CS0618 // Type or member is obsolete + Assert.Throws(() => VerifyRawHeaders(rawHeaders, expectedHeaderNames, expectedHeaderValues)); +#pragma warning restore CS0618 // Type or member is obsolete + } + + [Theory] + [InlineData("a:b\n\r\n", "a", "b", null, null)] + [InlineData("a: b\n\r\n", "a", "b", null, null)] + [InlineData("a: b\nc: d\n\r\n", "a", "b", "c", "d")] + [InlineData("a: b\nc: d\n\n", "a", "b", "c", "d")] + [InlineData("a: b\n\n", "a", "b", null, null)] + public void ParseHeadersCanEndAfterLineFeedTerminator( + string rawHeaders, + string expectedHeaderName1, + string expectedHeaderValue1, + string expectedHeaderName2, + string expectedHeaderValue2) + { + var expectedHeaderNames = expectedHeaderName2 == null + ? new[] { expectedHeaderName1 } + : new[] { expectedHeaderName1, expectedHeaderName2 }; + var expectedHeaderValues = expectedHeaderValue2 == null + ? new[] { expectedHeaderValue1 } + : new[] { expectedHeaderValue1, expectedHeaderValue2 }; + + VerifyRawHeaders(rawHeaders, expectedHeaderNames, expectedHeaderValues, enableLineFeedTerminator: true); + } + [Theory] [InlineData(" value")] [InlineData(" value")] @@ -346,6 +492,31 @@ public void ParseHeadersThrowsOnInvalidRequestHeaders(string rawHeaders, string Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); } + [Theory] + [MemberData(nameof(RequestHeaderInvalidDataLineFeedTerminator))] + public void ParseHeadersThrowsOnInvalidRequestHeadersLineFeedTerminator(string rawHeaders, string expectedExceptionMessage) + { + var mockTrace = new Mock(); + mockTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(true); + + var parser = CreateParser(mockTrace.Object); + var buffer = new ReadOnlySequence(Encoding.ASCII.GetBytes(rawHeaders)); + var requestHandler = new RequestHandler(); + +#pragma warning disable CS0618 // Type or member is obsolete + var exception = Assert.Throws(() => +#pragma warning restore CS0618 // Type or member is obsolete + { + var reader = new SequenceReader(buffer); + parser.ParseHeaders(requestHandler, ref reader); + }); + + Assert.Equal(expectedExceptionMessage, exception.Message); + Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); + } + [Fact] public void ExceptionDetailNotIncludedWhenLogLevelInformationNotEnabled() { @@ -454,11 +625,38 @@ public void ParseHeadersThrowsOnInvalidRequestHeadersWithGratuitouslySplitBuffer Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); } - [Fact] - public void ParseHeadersWithGratuitouslySplitBuffers() + [Theory] + [MemberData(nameof(RequestHeaderInvalidDataLineFeedTerminator))] + public void ParseHeadersThrowsOnInvalidRequestHeadersWithGratuitouslySplitBuffersLineFeedTerminator(string rawHeaders, string expectedExceptionMessage) + { + var mockTrace = new Mock(); + mockTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(true); + + var parser = CreateParser(mockTrace.Object); + var buffer = BytePerSegmentTestSequenceFactory.Instance.CreateWithContent(rawHeaders); + var requestHandler = new RequestHandler(); + +#pragma warning disable CS0618 // Type or member is obsolete + var exception = Assert.Throws(() => +#pragma warning restore CS0618 // Type or member is obsolete + { + var reader = new SequenceReader(buffer); + parser.ParseHeaders(requestHandler, ref reader); + }); + + Assert.Equal(expectedExceptionMessage, exception.Message); + Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); + } + + [Theory] + [InlineData("Host:\r\nConnection: keep-alive\r\n\r\n")] + [InlineData("A:B\r\nB: C\r\n\r\n")] + public void ParseHeadersWithGratuitouslySplitBuffers(string headers) { var parser = CreateParser(_nullTrace); - var buffer = BytePerSegmentTestSequenceFactory.Instance.CreateWithContent("Host:\r\nConnection: keep-alive\r\n\r\n"); + var buffer = BytePerSegmentTestSequenceFactory.Instance.CreateWithContent(headers); var requestHandler = new RequestHandler(); var reader = new SequenceReader(buffer); @@ -467,11 +665,15 @@ public void ParseHeadersWithGratuitouslySplitBuffers() Assert.True(result); } - [Fact] - public void ParseHeadersWithGratuitouslySplitBuffers2() + [Theory] + [InlineData("Host:\nConnection: keep-alive\r\n\r\n")] + [InlineData("Host:\r\nConnection: keep-alive\n\r\n")] + [InlineData("A:B\nB: C\r\n\r\n")] + [InlineData("A:B\r\nB: C\n\r\n")] + public void ParseHeadersWithGratuitouslySplitBuffersQuirkMode(string headers) { - var parser = CreateParser(_nullTrace); - var buffer = BytePerSegmentTestSequenceFactory.Instance.CreateWithContent("A:B\r\nB: C\r\n\r\n"); + var parser = CreateParser(_nullTrace, enableLineFeedTerminator: true); + var buffer = BytePerSegmentTestSequenceFactory.Instance.CreateWithContent(headers); var requestHandler = new RequestHandler(); var reader = new SequenceReader(buffer); @@ -480,7 +682,6 @@ public void ParseHeadersWithGratuitouslySplitBuffers2() Assert.True(result); } - private bool ParseRequestLine(IHttpParser parser, RequestHandler requestHandler, ReadOnlySequence readableBuffer, out SequencePosition consumed, out SequencePosition examined) { var reader = new SequenceReader(readableBuffer); @@ -517,11 +718,11 @@ private void VerifyHeader( Assert.True(buffer.Slice(reader.Position).IsEmpty); } - private void VerifyRawHeaders(string rawHeaders, IEnumerable expectedHeaderNames, IEnumerable expectedHeaderValues) + private void VerifyRawHeaders(string rawHeaders, IEnumerable expectedHeaderNames, IEnumerable expectedHeaderValues, bool enableLineFeedTerminator = false) { Assert.True(expectedHeaderNames.Count() == expectedHeaderValues.Count(), $"{nameof(expectedHeaderNames)} and {nameof(expectedHeaderValues)} sizes must match"); - var parser = CreateParser(_nullTrace); + var parser = CreateParser(_nullTrace, enableLineFeedTerminator); var buffer = new ReadOnlySequence(Encoding.ASCII.GetBytes(rawHeaders)); var requestHandler = new RequestHandler(); @@ -536,7 +737,7 @@ private void VerifyRawHeaders(string rawHeaders, IEnumerable expectedHea Assert.True(buffer.Slice(reader.Position).IsEmpty); } - private IHttpParser CreateParser(IKestrelTrace log) => new HttpParser(log.IsEnabled(LogLevel.Information)); + private IHttpParser CreateParser(IKestrelTrace log, bool enableLineFeedTerminator = false) => new HttpParser(log.IsEnabled(LogLevel.Information), enableLineFeedTerminator); public static IEnumerable RequestLineValidData => HttpParsingData.RequestLineValidData; @@ -544,11 +745,14 @@ private void VerifyRawHeaders(string rawHeaders, IEnumerable expectedHea public static IEnumerable RequestLineInvalidData => HttpParsingData.RequestLineInvalidData.Select(requestLine => new[] { requestLine }); + public static IEnumerable RequestLineInvalidDataLineFeedTerminator => HttpParsingData.RequestLineInvalidDataLineFeedTerminator.Select(requestLine => new[] { requestLine }); + public static IEnumerable MethodWithNonTokenCharData => HttpParsingData.MethodWithNonTokenCharData.Select(method => new[] { method }); public static TheoryData UnrecognizedHttpVersionData => HttpParsingData.UnrecognizedHttpVersionData; public static IEnumerable RequestHeaderInvalidData => HttpParsingData.RequestHeaderInvalidData; + public static IEnumerable RequestHeaderInvalidDataLineFeedTerminator => HttpParsingData.RequestHeaderInvalidDataLineFeedTerminator; private class RequestHandler : IHttpRequestLineHandler, IHttpHeadersHandler { diff --git a/src/Servers/Kestrel/shared/test/HttpParsingData.cs b/src/Servers/Kestrel/shared/test/HttpParsingData.cs index edfe18d39f3a..4cc651b5903b 100644 --- a/src/Servers/Kestrel/shared/test/HttpParsingData.cs +++ b/src/Servers/Kestrel/shared/test/HttpParsingData.cs @@ -205,7 +205,15 @@ public static IEnumerable RequestLineInvalidData "CUSTOM / HTTP/1.1a\n", "CUSTOM / HTTP/1.1a\r\n", "CUSTOM / HTTP/1.1ab\r\n", + "CUSTOM / H\n", + "CUSTOM / HT\n", + "CUSTOM / HTT\n", + "CUSTOM / HTTP\n", + "CUSTOM / HTTP/\n", + "CUSTOM / HTTP/1\n", + "CUSTOM / HTTP/1.\n", "CUSTOM / hello\r\n", + "CUSTOM / hello\n", "CUSTOM ? HTTP/1.1\r\n", "CUSTOM /a?b=cHTTP/1.1\r\n", "CUSTOM /a%20bHTTP/1.1\r\n", @@ -217,6 +225,21 @@ public static IEnumerable RequestLineInvalidData } } + // This list is valid in quirk mode + public static IEnumerable RequestLineInvalidDataLineFeedTerminator + { + get + { + return new[] + { + "GET / HTTP/1.0\n", + "GET / HTTP/1.1\n", + "CUSTOM / HTTP/1.0\n", + "CUSTOM / HTTP/1.1\n", + }; + } + } + // Bad HTTP Methods (invalid according to RFC) public static IEnumerable MethodWithNonTokenCharData { @@ -364,13 +387,16 @@ public static IEnumerable TargetWithNullCharData "8charact", }; - public static IEnumerable RequestHeaderInvalidData => new[] + public static IEnumerable RequestHeaderInvalidDataLineFeedTerminator => new[] { // Missing CR new[] { "Header: value\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header: value\x0A") }, new[] { "Header-1: value1\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0A") }, new[] { "Header-1: value1\r\nHeader-2: value2\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: value2\x0A") }, + }; + public static IEnumerable RequestHeaderInvalidData => new[] + { // Line folding new[] { "Header: line1\r\n line2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@" line2\x0D\x0A") }, new[] { "Header: line1\r\n\tline2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"\x09line2\x0D\x0A") }, @@ -404,7 +430,6 @@ public static IEnumerable TargetWithNullCharData new[] { "Header-1 value1\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1 value1\x0D\x0A") }, new[] { "Header-1 value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1 value1\x0D\x0A") }, new[] { "Header-1: value1\r\nHeader-2 value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2 value2\x0D\x0A") }, - new[] { "\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"\x0A") }, // Starting with whitespace new[] { " Header: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@" Header: value\x0D\x0A") }, @@ -435,7 +460,7 @@ public static IEnumerable TargetWithNullCharData // Headers not ending in CRLF line new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r\r", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF }, - new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r ", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF }, + new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r ", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF }, new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r \n", CoreStrings.BadRequest_InvalidRequestHeadersNoCRLF }, // Empty header name diff --git a/src/Servers/Kestrel/shared/test/TestServiceContext.cs b/src/Servers/Kestrel/shared/test/TestServiceContext.cs index 6825840ba012..cfa595404bdc 100644 --- a/src/Servers/Kestrel/shared/test/TestServiceContext.cs +++ b/src/Servers/Kestrel/shared/test/TestServiceContext.cs @@ -19,17 +19,17 @@ public TestServiceContext() var logger = new TestApplicationErrorLogger(); var kestrelTrace = new TestKestrelTrace(logger); - Initialize(kestrelTrace.LoggerFactory, kestrelTrace); + Initialize(kestrelTrace.LoggerFactory, kestrelTrace, false); } - public TestServiceContext(ILoggerFactory loggerFactory) + public TestServiceContext(ILoggerFactory loggerFactory, bool enableLineFeedTerminator = false) { - Initialize(loggerFactory, CreateLoggingTrace(loggerFactory)); + Initialize(loggerFactory, CreateLoggingTrace(loggerFactory), enableLineFeedTerminator); } - public TestServiceContext(ILoggerFactory loggerFactory, IKestrelTrace kestrelTrace) + public TestServiceContext(ILoggerFactory loggerFactory, IKestrelTrace kestrelTrace, bool enableLineFeedTerminator = false) { - Initialize(loggerFactory, new CompositeKestrelTrace(kestrelTrace, CreateLoggingTrace(loggerFactory))); + Initialize(loggerFactory, new CompositeKestrelTrace(kestrelTrace, CreateLoggingTrace(loggerFactory)), enableLineFeedTerminator); } private static KestrelTrace CreateLoggingTrace(ILoggerFactory loggerFactory) @@ -51,7 +51,7 @@ public void InitializeHeartbeat() SystemClock = heartbeatManager; } - private void Initialize(ILoggerFactory loggerFactory, IKestrelTrace kestrelTrace) + private void Initialize(ILoggerFactory loggerFactory, IKestrelTrace kestrelTrace, bool enableLineFeedTerminator) { LoggerFactory = loggerFactory; Log = kestrelTrace; @@ -60,7 +60,7 @@ private void Initialize(ILoggerFactory loggerFactory, IKestrelTrace kestrelTrace SystemClock = MockSystemClock; DateHeaderValueManager = new DateHeaderValueManager(); ConnectionManager = new ConnectionManager(Log, ResourceCounter.Unlimited); - HttpParser = new HttpParser(Log.IsEnabled(LogLevel.Information)); + HttpParser = new HttpParser(Log.IsEnabled(LogLevel.Information), enableLineFeedTerminator); ServerOptions = new KestrelServerOptions { AddServerHeader = false diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs index 6d4820bbfc78..dfaf3c6a05a4 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Net; +using System.Text; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; @@ -368,6 +370,8 @@ public static TheoryData InvalidRequestLineData public static IEnumerable InvalidRequestHeaderData => HttpParsingData.RequestHeaderInvalidData; + public static IEnumerable InvalidRequestHeaderDataLineFeedTerminator => HttpParsingData.RequestHeaderInvalidDataLineFeedTerminator; + public static TheoryData InvalidHostHeaderData => HttpParsingData.HostHeaderInvalidData; } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs index 691efda3d157..3355ec6f8cdf 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs @@ -2255,6 +2255,49 @@ await connection.Receive( } } + [Fact] + public async Task SingleLineFeedIsSupportedAnywhere() + { + // Exercises all combinations of LF and CRLF as line separators. + // Uses a bit mask for all the possible combinations. + + var lines = new[] + { + $"GET / HTTP/1.1", + "Content-Length: 0", + $"Host: localhost", + "", + }; + + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory, true))) + { + var mask = Math.Pow(2, lines.Length) - 1; + + for (var m = 0; m <= mask; m++) + { + using (var client = server.CreateConnection()) + { + var sb = new StringBuilder(); + + for (var pos = 0; pos < lines.Length; pos++) + { + sb.Append(lines[pos]); + var separator = (m & (1 << pos)) != 0 ? "\n" : "\r\n"; + sb.Append(separator); + } + + var text = sb.ToString(); + var writer = new StreamWriter(client.Stream, Encoding.GetEncoding("iso-8859-1")); + await writer.WriteAsync(text).ConfigureAwait(false); + await writer.FlushAsync().ConfigureAwait(false); + await client.Stream.FlushAsync().ConfigureAwait(false); + + await client.Receive("HTTP/1.1 200"); + } + } + } + } + public static TheoryData HostHeaderData => HttpParsingData.HostHeaderData; private class IntAsClass