From e25eb418bbb0f803fdee775cb721fadcdff1990e Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 7 Mar 2017 16:23:35 -0800 Subject: [PATCH 1/9] Change non-printable char representation in log messages from <0xXX> to \xXX. --- .../Internal/Infrastructure/HttpUtilities.cs | 2 +- .../BadHttpRequestTests.cs | 2 +- .../Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs index e38ad9fd6e..f93ceafc00 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs @@ -124,7 +124,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure for (i = 0; i < Math.Min(span.Length, maxChars); ++i) { var ch = span[i]; - sb.Append(ch < 0x20 || ch >= 0x7F ? $"<0x{ch:X2}>" : ((char)ch).ToString()); + sb.Append(ch < 0x20 || ch >= 0x7F ? $"\\x{ch:X2}" : ((char)ch).ToString()); } if (span.Length > maxChars) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs index 565b06c2f1..65402a019b 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs @@ -133,7 +133,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests foreach (var requestLine in HttpParsingData.RequestLineInvalidData) { - data.Add(requestLine, $"Invalid request line: {requestLine.Replace("\r", "<0x0D>").Replace("\n", "<0x0A>")}"); + data.Add(requestLine, $"Invalid request line: {requestLine.Replace("\r", "\\x0D").Replace("\n", "\\x0A")}"); } foreach (var requestLine in HttpParsingData.RequestLineWithEncodedNullCharInTargetData) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs index 7d16b5f06c..b63ecc5d83 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs @@ -108,7 +108,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var exception = Assert.Throws(() => parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); - Assert.Equal($"Invalid request line: {requestLine.Replace("\r", "<0x0D>").Replace("\n", "<0x0A>")}", exception.Message); + Assert.Equal($"Invalid request line: {requestLine.Replace("\r", "\\x0D").Replace("\n", "\\x0A")}", exception.Message); Assert.Equal(StatusCodes.Status400BadRequest, (exception as BadHttpRequestException).StatusCode); } From dbcf34388eccef4d8a102f334841a7d801d82013 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 8 Mar 2017 12:38:55 -0800 Subject: [PATCH 2/9] Remove dependency on Utf8String (#1466) - Also remove System.IO.Pipelines.Text.Primitives --- .../Internal/Http/Frame.cs | 17 +++++++++++++++-- .../Microsoft.AspNetCore.Server.Kestrel.csproj | 1 - 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs index 463ce0a237..bc6757b30e 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs @@ -6,7 +6,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.IO.Pipelines; -using System.IO.Pipelines.Text.Primitives; using System.Linq; using System.Net; using System.Runtime.CompilerServices; @@ -1232,7 +1231,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // URI was encoded, unescape and then parse as utf8 int pathLength = UrlEncoder.Decode(path, path); - requestUrlPath = new Utf8String(path.Slice(0, pathLength)).ToString(); + requestUrlPath = GetUtf8String(path.Slice(0, pathLength)); } else { @@ -1282,6 +1281,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } } + private unsafe static string GetUtf8String(Span path) + { + // .NET 451 doesn't have pointer overloads for Encoding.GetString so we + // copy to an array +#if NET451 + return Encoding.UTF8.GetString(path.ToArray()); +#else + fixed (byte* pointer = &path.DangerousGetPinnableReference()) + { + return Encoding.UTF8.GetString(pointer, path.Length); + } +#endif + } + public void OnHeader(Span name, Span value) { _requestHeadersParsed++; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Microsoft.AspNetCore.Server.Kestrel.csproj b/src/Microsoft.AspNetCore.Server.Kestrel/Microsoft.AspNetCore.Server.Kestrel.csproj index 35ccda46b3..beacc7f230 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Microsoft.AspNetCore.Server.Kestrel.csproj +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Microsoft.AspNetCore.Server.Kestrel.csproj @@ -19,7 +19,6 @@ - From bb973decb8c10716a17d1d236729a32c5a22afb7 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 7 Mar 2017 17:25:15 -0800 Subject: [PATCH 3/9] Unify header rejection messages. - Log bad headers with escaped non-vchar characters --- .../BadHttpRequestException.cs | 30 +--- .../Internal/Http/KestrelHttpParser.cs | 44 +++-- .../Internal/Http/RequestRejectionReason.cs | 10 +- .../BadHttpRequestTests.cs | 2 +- .../FrameTests.cs | 19 --- .../HttpParserTests.cs | 9 +- test/shared/HttpParsingData.cs | 161 ++++++++---------- 7 files changed, 109 insertions(+), 166 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs index 54533f05e1..09aaf94a45 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs @@ -22,20 +22,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel BadHttpRequestException ex; switch (reason) { - case RequestRejectionReason.HeadersCorruptedInvalidHeaderSequence: - ex = new BadHttpRequestException("Headers corrupted, invalid header sequence.", StatusCodes.Status400BadRequest); - break; - case RequestRejectionReason.NoColonCharacterFoundInHeaderLine: - ex = new BadHttpRequestException("No ':' character found in header line.", StatusCodes.Status400BadRequest); - break; - case RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName: - ex = new BadHttpRequestException("Whitespace is not allowed in header name.", StatusCodes.Status400BadRequest); - break; - case RequestRejectionReason.HeaderValueMustNotContainCR: - ex = new BadHttpRequestException("Header value must not contain CR characters.", StatusCodes.Status400BadRequest); - break; - case RequestRejectionReason.HeaderValueLineFoldingNotSupported: - ex = new BadHttpRequestException("Header value line folding not supported.", StatusCodes.Status400BadRequest); + case RequestRejectionReason.InvalidRequestHeadersNoCRLF: + ex = new BadHttpRequestException("Invalid request headers: missing final CRLF in header fields.", StatusCodes.Status400BadRequest); break; case RequestRejectionReason.InvalidRequestLine: ex = new BadHttpRequestException("Invalid request line.", StatusCodes.Status400BadRequest); @@ -58,24 +46,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel case RequestRejectionReason.ChunkedRequestIncomplete: ex = new BadHttpRequestException("Chunked request incomplete.", StatusCodes.Status400BadRequest); break; - case RequestRejectionReason.PathContainsNullCharacters: - ex = new BadHttpRequestException("The path contains null characters.", StatusCodes.Status400BadRequest); - break; case RequestRejectionReason.InvalidCharactersInHeaderName: ex = new BadHttpRequestException("Invalid characters in header name.", StatusCodes.Status400BadRequest); break; - case RequestRejectionReason.NonAsciiOrNullCharactersInInputString: - ex = new BadHttpRequestException("The input string contains non-ASCII or null characters.", StatusCodes.Status400BadRequest); - break; case RequestRejectionReason.RequestLineTooLong: ex = new BadHttpRequestException("Request line too long.", StatusCodes.Status414UriTooLong); break; case RequestRejectionReason.HeadersExceedMaxTotalSize: ex = new BadHttpRequestException("Request headers too long.", StatusCodes.Status431RequestHeaderFieldsTooLarge); break; - case RequestRejectionReason.MissingCRInHeaderLine: - ex = new BadHttpRequestException("No CR character found in header line.", StatusCodes.Status400BadRequest); - break; case RequestRejectionReason.TooManyHeaders: ex = new BadHttpRequestException("Request contains too many headers.", StatusCodes.Status431RequestHeaderFieldsTooLarge); break; @@ -95,7 +74,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel switch (reason) { case RequestRejectionReason.InvalidRequestLine: - ex = new BadHttpRequestException($"Invalid request line: {value}", StatusCodes.Status400BadRequest); + ex = new BadHttpRequestException($"Invalid request line: '{value}'", StatusCodes.Status400BadRequest); + break; + case RequestRejectionReason.InvalidRequestHeader: + ex = new BadHttpRequestException($"Invalid request header: '{value}'", StatusCodes.Status400BadRequest); break; case RequestRejectionReason.InvalidContentLength: ex = new BadHttpRequestException($"Invalid content length: {value}", StatusCodes.Status400BadRequest); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index a5ce732aec..79ecd4b9f0 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -316,11 +316,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } // Headers don't end in CRLF line. - RejectRequest(RequestRejectionReason.HeadersCorruptedInvalidHeaderSequence); - } - else if(ch1 == ByteSpace || ch1 == ByteTab) - { - RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); + RejectRequest(RequestRejectionReason.InvalidRequestHeadersNoCRLF); } // We moved the reader so look ahead 2 bytes so reset both the reader @@ -390,7 +386,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static unsafe int FindEndOfName(byte* headerLine, int length) + private unsafe int FindEndOfName(byte* headerLine, int length) { var index = 0; var sawWhitespace = false; @@ -407,14 +403,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } } - if (index == length) + if (index == length || sawWhitespace) { - RejectRequest(RequestRejectionReason.NoColonCharacterFoundInHeaderLine); - } - if (sawWhitespace) - { - RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); + RejectRequestHeader(headerLine, length); } + return index; } @@ -427,11 +420,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http if (headerLine[valueEnd + 2] != ByteLF) { - RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); + RejectRequestHeader(headerLine, length); } if (headerLine[valueEnd + 1] != ByteCR) { - RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); + RejectRequestHeader(headerLine, length); } // Skip colon from value start @@ -446,16 +439,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } else if (ch == ByteCR) { - RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); + RejectRequestHeader(headerLine, length); } } - // Check for CR in value var i = valueStart + 1; if (Contains(headerLine + i, valueEnd - i, ByteCR)) { - RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); + RejectRequestHeader(headerLine, length); } // Ignore end whitespace @@ -557,9 +549,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http Log.IsEnabled(LogLevel.Information) ? span.GetAsciiStringEscaped(MaxRequestLineError) : string.Empty); } + private unsafe void RejectRequestHeader(byte* headerLine, int length) + { + RejectRequestHeader(new Span(headerLine, length)); + } + + private void RejectRequestHeader(Span span) + { + throw GetRejectRequestHeaderException(span); + } + + private BadHttpRequestException GetRejectRequestHeaderException(Span span) + { + const int MaxRequestHeaderError = 128; + return BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestHeader, + Log.IsEnabled(LogLevel.Information) ? span.GetAsciiStringEscaped(MaxRequestHeaderError) : string.Empty); + } + public void Reset() { - } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs index b0aa5a1372..92faea1965 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs @@ -6,12 +6,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http public enum RequestRejectionReason { UnrecognizedHTTPVersion, - HeadersCorruptedInvalidHeaderSequence, - NoColonCharacterFoundInHeaderLine, - WhitespaceIsNotAllowedInHeaderName, - HeaderValueMustNotContainCR, - HeaderValueLineFoldingNotSupported, InvalidRequestLine, + InvalidRequestHeader, + InvalidRequestHeadersNoCRLF, MalformedRequestInvalidHeaders, InvalidContentLength, MultipleContentLengths, @@ -19,12 +16,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http BadChunkSuffix, BadChunkSizeData, ChunkedRequestIncomplete, - PathContainsNullCharacters, InvalidCharactersInHeaderName, - NonAsciiOrNullCharactersInInputString, RequestLineTooLong, HeadersExceedMaxTotalSize, - MissingCRInHeaderLine, TooManyHeaders, RequestTimeout, FinalTransferCodingNotChunked, diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs index 65402a019b..380a51188e 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs @@ -133,7 +133,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests foreach (var requestLine in HttpParsingData.RequestLineInvalidData) { - data.Add(requestLine, $"Invalid request line: {requestLine.Replace("\r", "\\x0D").Replace("\n", "\\x0A")}"); + data.Add(requestLine, $"Invalid request line: '{requestLine.Replace("\r", "\\x0D").Replace("\n", "\\x0A")}'"); } foreach (var requestLine in HttpParsingData.RequestLineWithEncodedNullCharInTargetData) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index 8aeb066862..bbac629985 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -81,25 +81,6 @@ namespace Microsoft.AspNetCore.Server.KestrelTests _pipelineFactory.Dispose(); } - [Fact] - public async Task TakeMessageHeadersThrowsOnHeaderValueWithLineFolding_CharacterNotAvailableOnFirstAttempt() - { - await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header-1: value1\r\n")); - - var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; - Assert.False(_frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); - _input.Reader.Advance(_consumed, _examined); - - await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes(" ")); - - readableBuffer = (await _input.Reader.ReadAsync()).Buffer; - var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); - _input.Reader.Advance(_consumed, _examined); - - Assert.Equal("Whitespace is not allowed in header name.", exception.Message); - Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); - } - [Fact] public async Task TakeMessageHeadersThrowsWhenHeadersExceedTotalSizeLimit() { diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs index b63ecc5d83..6ad29a68cb 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs @@ -108,7 +108,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var exception = Assert.Throws(() => parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); - Assert.Equal($"Invalid request line: {requestLine.Replace("\r", "\\x0D").Replace("\n", "\\x0A")}", exception.Message); + Assert.Equal($"Invalid request line: '{requestLine.Replace("\r", "\\x0D").Replace("\n", "\\x0A")}'", exception.Message); Assert.Equal(StatusCodes.Status400BadRequest, (exception as BadHttpRequestException).StatusCode); } @@ -306,7 +306,12 @@ namespace Microsoft.AspNetCore.Server.KestrelTests [MemberData(nameof(RequestHeaderInvalidData))] public void ParseHeadersThrowsOnInvalidRequestHeaders(string rawHeaders, string expectedExceptionMessage) { - var parser = CreateParser(Mock.Of()); + var mockTrace = new Mock(); + mockTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(true); + + var parser = CreateParser(mockTrace.Object); var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(rawHeaders)); var exception = Assert.Throws(() => diff --git a/test/shared/HttpParsingData.cs b/test/shared/HttpParsingData.cs index 7d84ba37b3..e999fb3170 100644 --- a/test/shared/HttpParsingData.cs +++ b/test/shared/HttpParsingData.cs @@ -203,106 +203,79 @@ namespace Microsoft.AspNetCore.Testing "8charact", }; - public static IEnumerable RequestHeaderInvalidData + public static IEnumerable RequestHeaderInvalidData => new[] { - get - { - // Line folding - var headersWithLineFolding = new[] - { - "Header: line1\r\n line2\r\n\r\n", - "Header: line1\r\n\tline2\r\n\r\n", - "Header: line1\r\n line2\r\n\r\n", - "Header: line1\r\n \tline2\r\n\r\n", - "Header: line1\r\n\t line2\r\n\r\n", - "Header: line1\r\n\t\tline2\r\n\r\n", - "Header: line1\r\n \t\t line2\r\n\r\n", - "Header: line1\r\n \t \t line2\r\n\r\n", - "Header-1: multi\r\n line\r\nHeader-2: value2\r\n\r\n", - "Header-1: value1\r\nHeader-2: multi\r\n line\r\n\r\n", - "Header-1: value1\r\n Header-2: value2\r\n\r\n", - "Header-1: value1\r\n\tHeader-2: value2\r\n\r\n", - }; + // Missing CR + new[] { "Header: value\n\r\n", @"Invalid request header: 'Header: value\x0A'" }, + new[] { "Header-1: value1\nHeader-2: value2\r\n\r\n", @"Invalid request header: 'Header-1: value1\x0A'" }, + new[] { "Header-1: value1\r\nHeader-2: value2\n\r\n", @"Invalid request header: 'Header-2: value2\x0A'" }, - // CR in value - var headersWithCRInValue = new[] - { - "Header-1: value1\r\r\n", - "Header-1: val\rue1\r\n", - "Header-1: value1\rHeader-2: value2\r\n\r\n", - "Header-1: value1\r\nHeader-2: value2\r\r\n", - "Header-1: value1\r\nHeader-2: v\ralue2\r\n", - "Header-1: Value__\rVector16________Vector32\r\n", - "Header-1: Value___Vector16\r________Vector32\r\n", - "Header-1: Value___Vector16_______\rVector32\r\n", - "Header-1: Value___Vector16________Vector32\r\r\n", - "Header-1: Value___Vector16________Vector32_\r\r\n", - "Header-1: Value___Vector16________Vector32Value___Vector16_______\rVector32\r\n", - "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\r\r\n", - "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\r\r\n", - }; + // Line folding + new[] { "Header: line1\r\n line2\r\n\r\n", @"Invalid request header: ' line2\x0D\x0A'" }, + new[] { "Header: line1\r\n\tline2\r\n\r\n", @"Invalid request header: '\x09line2\x0D\x0A'" }, + new[] { "Header: line1\r\n line2\r\n\r\n", @"Invalid request header: ' line2\x0D\x0A'" }, + new[] { "Header: line1\r\n \tline2\r\n\r\n", @"Invalid request header: ' \x09line2\x0D\x0A'" }, + new[] { "Header: line1\r\n\t line2\r\n\r\n", @"Invalid request header: '\x09 line2\x0D\x0A'" }, + new[] { "Header: line1\r\n\t\tline2\r\n\r\n", @"Invalid request header: '\x09\x09line2\x0D\x0A'" }, + new[] { "Header: line1\r\n \t\t line2\r\n\r\n", @"Invalid request header: ' \x09\x09 line2\x0D\x0A'" }, + new[] { "Header: line1\r\n \t \t line2\r\n\r\n", @"Invalid request header: ' \x09 \x09 line2\x0D\x0A'" }, + new[] { "Header-1: multi\r\n line\r\nHeader-2: value2\r\n\r\n", @"Invalid request header: ' line\x0D\x0A'" }, + new[] { "Header-1: value1\r\nHeader-2: multi\r\n line\r\n\r\n", @"Invalid request header: ' line\x0D\x0A'" }, + new[] { "Header-1: value1\r\n Header-2: value2\r\n\r\n", @"Invalid request header: ' Header-2: value2\x0D\x0A'" }, + new[] { "Header-1: value1\r\n\tHeader-2: value2\r\n\r\n", @"Invalid request header: '\x09Header-2: value2\x0D\x0A'" }, - // Missing colon - var headersWithMissingColon = new[] - { - "Header-1 value1\r\n\r\n", - "Header-1 value1\r\nHeader-2: value2\r\n\r\n", - "Header-1: value1\r\nHeader-2 value2\r\n\r\n", - "\n" - }; + // CR in value + new[] { "Header-1: value1\r\r\n", @"Invalid request header: 'Header-1: value1\x0D\x0D\x0A'" }, + new[] { "Header-1: val\rue1\r\n", @"Invalid request header: 'Header-1: val\x0Due1\x0D\x0A'" }, + new[] { "Header-1: value1\rHeader-2: value2\r\n\r\n", @"Invalid request header: 'Header-1: value1\x0DHeader-2: value2\x0D\x0A'" }, + new[] { "Header-1: value1\r\nHeader-2: value2\r\r\n", @"Invalid request header: 'Header-2: value2\x0D\x0D\x0A'" }, + new[] { "Header-1: value1\r\nHeader-2: v\ralue2\r\n", @"Invalid request header: 'Header-2: v\x0Dalue2\x0D\x0A'" }, + new[] { "Header-1: Value__\rVector16________Vector32\r\n", @"Invalid request header: 'Header-1: Value__\x0DVector16________Vector32\x0D\x0A'" }, + new[] { "Header-1: Value___Vector16\r________Vector32\r\n", @"Invalid request header: 'Header-1: Value___Vector16\x0D________Vector32\x0D\x0A'" }, + new[] { "Header-1: Value___Vector16_______\rVector32\r\n", @"Invalid request header: 'Header-1: Value___Vector16_______\x0DVector32\x0D\x0A'" }, + new[] { "Header-1: Value___Vector16________Vector32\r\r\n", @"Invalid request header: 'Header-1: Value___Vector16________Vector32\x0D\x0D\x0A'" }, + new[] { "Header-1: Value___Vector16________Vector32_\r\r\n", @"Invalid request header: 'Header-1: Value___Vector16________Vector32_\x0D\x0D\x0A'" }, + new[] { "Header-1: Value___Vector16________Vector32Value___Vector16_______\rVector32\r\n", @"Invalid request header: 'Header-1: Value___Vector16________Vector32Value___Vector16_______\x0DVector32\x0D\x0A'" }, + new[] { "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\r\r\n", @"Invalid request header: 'Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\x0D\x0D\x0A'" }, + new[] { "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\r\r\n", @"Invalid request header: 'Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\x0D\x0D\x0A'" }, - // Starting with whitespace - var headersStartingWithWhitespace = new[] - { - " Header: value\r\n\r\n", - "\tHeader: value\r\n\r\n", - " Header-1: value1\r\nHeader-2: value2\r\n\r\n", - "\tHeader-1: value1\r\nHeader-2: value2\r\n\r\n", - }; + // Missing colon + new[] { "Header-1 value1\r\n\r\n", @"Invalid request header: 'Header-1 value1\x0D\x0A'" }, + new[] { "Header-1 value1\r\nHeader-2: value2\r\n\r\n", @"Invalid request header: 'Header-1 value1\x0D\x0A'" }, + new[] { "Header-1: value1\r\nHeader-2 value2\r\n\r\n", @"Invalid request header: 'Header-2 value2\x0D\x0A'" }, + new[] { "\n", @"Invalid request header: '\x0A'" }, - // Whitespace in header name - var headersWithWithspaceInName = new[] - { - "Header : value\r\n\r\n", - "Header\t: value\r\n\r\n", - "Header\r: value\r\n\r\n", - "Header_\rVector16: value\r\n\r\n", - "Header__Vector16\r: value\r\n\r\n", - "Header__Vector16_\r: value\r\n\r\n", - "Header_\rVector16________Vector32: value\r\n\r\n", - "Header__Vector16________Vector32\r: value\r\n\r\n", - "Header__Vector16________Vector32_\r: value\r\n\r\n", - "Header__Vector16________Vector32Header_\rVector16________Vector32: value\r\n\r\n", - "Header__Vector16________Vector32Header__Vector16________Vector32\r: value\r\n\r\n", - "Header__Vector16________Vector32Header__Vector16________Vector32_\r: value\r\n\r\n", - "Header 1: value1\r\nHeader-2: value2\r\n\r\n", - "Header 1 : value1\r\nHeader-2: value2\r\n\r\n", - "Header 1\t: value1\r\nHeader-2: value2\r\n\r\n", - "Header 1\r: value1\r\nHeader-2: value2\r\n\r\n", - "Header-1: value1\r\nHeader 2: value2\r\n\r\n", - "Header-1: value1\r\nHeader-2 : value2\r\n\r\n", - "Header-1: value1\r\nHeader-2\t: value2\r\n\r\n", - }; + // Starting with whitespace + new[] { " Header: value\r\n\r\n", @"Invalid request header: ' Header: value\x0D\x0A'" }, + new[] { "\tHeader: value\r\n\r\n", @"Invalid request header: '\x09Header: value\x0D\x0A'" }, + new[] { " Header-1: value1\r\nHeader-2: value2\r\n\r\n", @"Invalid request header: ' Header-1: value1\x0D\x0A'" }, + new[] { "\tHeader-1: value1\r\nHeader-2: value2\r\n\r\n", @"Invalid request header: '\x09Header-1: value1\x0D\x0A'" }, - // Headers not ending in CRLF line - var headersNotEndingInCrLfLine = new[] - { - "Header-1: value1\r\nHeader-2: value2\r\n\r\r", - "Header-1: value1\r\nHeader-2: value2\r\n\r ", - "Header-1: value1\r\nHeader-2: value2\r\n\r \n", - }; + // Whitespace in header name + new[] { "Header : value\r\n\r\n", @"Invalid request header: 'Header : value\x0D\x0A'" }, + new[] { "Header\t: value\r\n\r\n", @"Invalid request header: 'Header\x09: value\x0D\x0A'" }, + new[] { "Header\r: value\r\n\r\n", @"Invalid request header: 'Header\x0D: value\x0D\x0A'" }, + new[] { "Header_\rVector16: value\r\n\r\n", @"Invalid request header: 'Header_\x0DVector16: value\x0D\x0A'" }, + new[] { "Header__Vector16\r: value\r\n\r\n", @"Invalid request header: 'Header__Vector16\x0D: value\x0D\x0A'" }, + new[] { "Header__Vector16_\r: value\r\n\r\n", @"Invalid request header: 'Header__Vector16_\x0D: value\x0D\x0A'" }, + new[] { "Header_\rVector16________Vector32: value\r\n\r\n", @"Invalid request header: 'Header_\x0DVector16________Vector32: value\x0D\x0A'" }, + new[] { "Header__Vector16________Vector32\r: value\r\n\r\n", @"Invalid request header: 'Header__Vector16________Vector32\x0D: value\x0D\x0A'" }, + new[] { "Header__Vector16________Vector32_\r: value\r\n\r\n", @"Invalid request header: 'Header__Vector16________Vector32_\x0D: value\x0D\x0A'" }, + new[] { "Header__Vector16________Vector32Header_\rVector16________Vector32: value\r\n\r\n", @"Invalid request header: 'Header__Vector16________Vector32Header_\x0DVector16________Vector32: value\x0D\x0A'" }, + new[] { "Header__Vector16________Vector32Header__Vector16________Vector32\r: value\r\n\r\n", @"Invalid request header: 'Header__Vector16________Vector32Header__Vector16________Vector32\x0D: value\x0D\x0A'" }, + new[] { "Header__Vector16________Vector32Header__Vector16________Vector32_\r: value\r\n\r\n", @"Invalid request header: 'Header__Vector16________Vector32Header__Vector16________Vector32_\x0D: value\x0D\x0A'" }, + new[] { "Header 1: value1\r\nHeader-2: value2\r\n\r\n", @"Invalid request header: 'Header 1: value1\x0D\x0A'" }, + new[] { "Header 1 : value1\r\nHeader-2: value2\r\n\r\n", @"Invalid request header: 'Header 1 : value1\x0D\x0A'" }, + new[] { "Header 1\t: value1\r\nHeader-2: value2\r\n\r\n", @"Invalid request header: 'Header 1\x09: value1\x0D\x0A'" }, + new[] { "Header 1\r: value1\r\nHeader-2: value2\r\n\r\n", @"Invalid request header: 'Header 1\x0D: value1\x0D\x0A'" }, + new[] { "Header-1: value1\r\nHeader 2: value2\r\n\r\n", @"Invalid request header: 'Header 2: value2\x0D\x0A'" }, + new[] { "Header-1: value1\r\nHeader-2 : value2\r\n\r\n", @"Invalid request header: 'Header-2 : value2\x0D\x0A'" }, + new[] { "Header-1: value1\r\nHeader-2\t: value2\r\n\r\n", @"Invalid request header: 'Header-2\x09: value2\x0D\x0A'" }, - return new[] - { - Tuple.Create(headersWithLineFolding, "Whitespace is not allowed in header name."), - Tuple.Create(headersWithCRInValue, "Header value must not contain CR characters."), - Tuple.Create(headersWithMissingColon, "No ':' character found in header line."), - Tuple.Create(headersStartingWithWhitespace, "Whitespace is not allowed in header name."), - Tuple.Create(headersWithWithspaceInName, "Whitespace is not allowed in header name."), - Tuple.Create(headersNotEndingInCrLfLine, "Headers corrupted, invalid header sequence.") - } - .SelectMany(t => t.Item1.Select(headers => new[] { headers, t.Item2 })); - } - } + // Headers not ending in CRLF line + new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r\r", @"Invalid request headers: missing final CRLF in header fields." }, + new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r ", @"Invalid request headers: missing final CRLF in header fields." }, + new[] { "Header-1: value1\r\nHeader-2: value2\r\n\r \n", @"Invalid request headers: missing final CRLF in header fields." }, + }; } } From 3a1f3d14f9c340606a3dbd5dc9ff97ce68a7d774 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Wed, 8 Mar 2017 17:21:21 -0800 Subject: [PATCH 4/9] Add absolute-uri benchmark and change plaintext, live aspnet, and unicode benchmarks to use origin-form --- .../RequestParsing.cs | 10 ++++++++++ .../RequestParsingData.cs | 14 ++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index 8fc4bc0eea..16f1efdb8d 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -42,6 +42,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance } } + [Benchmark(Baseline = true, OperationsPerInvoke = RequestParsingData.InnerLoopCount)] + public void PlaintextAbsoluteUri() + { + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) + { + InsertData(RequestParsingData.PlaintextAbsoluteUriRequest); + ParseData(); + } + } + [Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount * RequestParsingData.Pipelining)] public void PipelinedPlaintextTechEmpower() { diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsingData.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsingData.cs index bddc1a7a97..45b585a2d0 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsingData.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsingData.cs @@ -21,8 +21,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance "Connection: keep-alive\r\n" + "\r\n"; + // edge-casey - client's don't normally send this + private const string _plaintextAbsoluteUriRequest = + "GET http://localhost/plaintext HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7\r\n" + + "Connection: keep-alive\r\n" + + "\r\n"; + private const string _liveaspnetRequest = - "GET https://live.asp.net/ HTTP/1.1\r\n" + + "GET / HTTP/1.1\r\n" + "Host: live.asp.net\r\n" + "Connection: keep-alive\r\n" + "Upgrade-Insecure-Requests: 1\r\n" + @@ -35,7 +43,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance "\r\n"; private const string _unicodeRequest = - "GET http://stackoverflow.com/questions/40148683/why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric HTTP/1.1\r\n" + + "GET /questions/40148683/why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric HTTP/1.1\r\n" + "Accept: text/html, application/xhtml+xml, image/jxr, */*\r\n" + "Accept-Language: en-US,en-GB;q=0.7,en;q=0.3\r\n" + "User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36 Edge/15.14965\r\n" + @@ -53,6 +61,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public static readonly byte[] PlaintextTechEmpowerPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(_plaintextTechEmpowerRequest, Pipelining))); public static readonly byte[] PlaintextTechEmpowerRequest = Encoding.ASCII.GetBytes(_plaintextTechEmpowerRequest); + public static readonly byte[] PlaintextAbsoluteUriRequest = Encoding.ASCII.GetBytes(_plaintextAbsoluteUriRequest); + public static readonly byte[] LiveaspnetPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(_liveaspnetRequest, Pipelining))); public static readonly byte[] LiveaspnetRequest = Encoding.ASCII.GetBytes(_liveaspnetRequest); From 63e3c9428a71158825b6f282856f915e958244e0 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Wed, 8 Mar 2017 17:28:13 -0800 Subject: [PATCH 5/9] You can't have two benchmarks as the baseline. --- .../RequestParsing.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index 16f1efdb8d..ccaf08ba57 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -42,7 +42,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance } } - [Benchmark(Baseline = true, OperationsPerInvoke = RequestParsingData.InnerLoopCount)] + [Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount)] public void PlaintextAbsoluteUri() { for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) From 49d058a9970cbdb437bc4fad3e8a0e91dcc74dab Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 8 Mar 2017 18:47:52 -0800 Subject: [PATCH 6/9] No mono (#1472) --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4d388988b4..fb5fa0217a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,8 +16,7 @@ env: global: - DOTNET_SKIP_FIRST_TIME_EXPERIENCE: true - DOTNET_CLI_TELEMETRY_OPTOUT: 1 -mono: - - 4.0.5 +mono: none os: - linux - osx From 941d3969427c31fa275f5760bfa7783e8384716b Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Thu, 9 Mar 2017 03:27:56 +0000 Subject: [PATCH 7/9] Speed up ParseRequestLine (#1463) --- .../Internal/Http/Frame.cs | 5 +- .../Internal/Http/IHttpRequestLineHandler.cs | 2 +- .../Internal/Http/KestrelHttpParser.cs | 395 ++++++++---------- .../Internal/Infrastructure/HttpUtilities.cs | 106 +++-- .../FrameParsingOverhead.cs | 2 +- .../KestrelHttpParser.cs | 2 +- .../HttpParserTests.cs | 6 +- 7 files changed, 256 insertions(+), 262 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs index bc6757b30e..c8a5e20033 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs @@ -1216,15 +1216,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http Log.ApplicationError(ConnectionId, ex); } - public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod) + public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, bool pathEncoded) { // URIs are always encoded/escaped to ASCII https://tools.ietf.org/html/rfc3986#page-11 // Multibyte Internationalized Resource Identifiers (IRIs) are first converted to utf8; // then encoded/escaped to ASCII https://www.ietf.org/rfc/rfc3987.txt "Mapping of IRIs to URIs" string requestUrlPath; string rawTarget; - var needDecode = path.IndexOf(BytePercentage) >= 0; - if (needDecode) + if (pathEncoded) { // Read raw target before mutating memory. rawTarget = target.GetAsciiStringNonNullCharacters(); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs index ddb6473882..83481002b3 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs @@ -7,6 +7,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { public interface IHttpRequestLineHandler { - void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod); + void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, bool pathEncoded); } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index 79ecd4b9f0..02363a2325 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -34,219 +34,130 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http consumed = buffer.Start; examined = buffer.End; - ReadCursor end; - Span span; - - // If the buffer is a single span then use it to find the LF - if (buffer.IsSingleSpan) + // Prepare the first span + var span = buffer.First.Span; + var lineIndex = span.IndexOfVectorized(ByteLF); + if (lineIndex >= 0) { - var startLineSpan = buffer.First.Span; - var lineIndex = startLineSpan.IndexOfVectorized(ByteLF); - - if (lineIndex == -1) - { - return false; - } - - end = buffer.Move(consumed, lineIndex + 1); - span = startLineSpan.Slice(0, lineIndex + 1); + consumed = buffer.Move(consumed, lineIndex + 1); + span = span.Slice(0, lineIndex + 1); } - else + else if (buffer.IsSingleSpan || !TryGetNewLineSpan(ref buffer, ref span, out consumed)) { - var start = buffer.Start; - if (ReadCursorOperations.Seek(start, buffer.End, out end, ByteLF) == -1) - { - return false; - } - - // Move 1 byte past the \n - end = buffer.Move(end, 1); - var startLineBuffer = buffer.Slice(start, end); - - span = startLineBuffer.ToSpan(); + // No request line end + return false; } - var pathStart = -1; - var queryStart = -1; - var queryEnd = -1; - var pathEnd = -1; - var versionStart = -1; - - var httpVersion = HttpVersion.Unknown; - HttpMethod method; - Span customMethod; - var i = 0; - var length = span.Length; - var done = false; - + // Fix and parse the span fixed (byte* data = &span.DangerousGetPinnableReference()) { - switch (StartLineState.KnownMethod) + ParseRequestLine(handler, data, span.Length); + } + + examined = consumed; + return true; + } + + private unsafe void ParseRequestLine(T handler, byte* data, int length) where T : IHttpRequestLineHandler + { + int offset; + Span customMethod; + // Get Method and set the offset + var method = HttpUtilities.GetKnownMethod(data, length, out offset); + if (method == HttpMethod.Custom) + { + customMethod = GetUnknownMethod(data, length, out offset); + } + + // Skip space + offset++; + + byte ch = 0; + // Target = Path and Query + var pathEncoded = false; + var pathStart = -1; + for (; offset < length; offset++) + { + ch = data[offset]; + if (ch == ByteSpace) { - case StartLineState.KnownMethod: - if (span.GetKnownMethod(out method, out var methodLength)) - { - // Update the index, current char, state and jump directly - // to the next state - i += methodLength + 1; + if (pathStart == -1) + { + // Empty path is illegal + RejectRequestLine(data, length); + } - goto case StartLineState.Path; - } - goto case StartLineState.UnknownMethod; + break; + } + else if (ch == ByteQuestionMark) + { + if (pathStart == -1) + { + // Empty path is illegal + RejectRequestLine(data, length); + } - case StartLineState.UnknownMethod: - for (; i < length; i++) - { - var ch = data[i]; + break; + } + else if (ch == BytePercentage) + { + if (pathStart == -1) + { + // Path starting with % is illegal + RejectRequestLine(data, length); + } - if (ch == ByteSpace) - { - customMethod = span.Slice(0, i); - - if (customMethod.Length == 0) - { - RejectRequestLine(span); - } - // Consume space - i++; - - goto case StartLineState.Path; - } - - if (!IsValidTokenChar((char)ch)) - { - RejectRequestLine(span); - } - } - - break; - case StartLineState.Path: - for (; i < length; i++) - { - var ch = data[i]; - if (ch == ByteSpace) - { - pathEnd = i; - - if (pathStart == -1) - { - // Empty path is illegal - RejectRequestLine(span); - } - - // No query string found - queryStart = queryEnd = i; - - // Consume space - i++; - - goto case StartLineState.KnownVersion; - } - else if (ch == ByteQuestionMark) - { - pathEnd = i; - - if (pathStart == -1) - { - // Empty path is illegal - RejectRequestLine(span); - } - - queryStart = i; - goto case StartLineState.QueryString; - } - else if (ch == BytePercentage) - { - if (pathStart == -1) - { - RejectRequestLine(span); - } - } - - if (pathStart == -1) - { - pathStart = i; - } - } - break; - case StartLineState.QueryString: - for (; i < length; i++) - { - var ch = data[i]; - if (ch == ByteSpace) - { - queryEnd = i; - - // Consume space - i++; - - goto case StartLineState.KnownVersion; - } - } - break; - case StartLineState.KnownVersion: - // REVIEW: We don't *need* to slice here but it makes the API - // nicer, slicing should be free :) - if (span.Slice(i).GetKnownVersion(out httpVersion, out var versionLenght)) - { - // Update the index, current char, state and jump directly - // to the next state - i += versionLenght + 1; - goto case StartLineState.NewLine; - } - - versionStart = i; - - goto case StartLineState.UnknownVersion; - - case StartLineState.UnknownVersion: - for (; i < length; i++) - { - var ch = data[i]; - if (ch == ByteCR) - { - var versionSpan = span.Slice(versionStart, i - versionStart); - - if (versionSpan.Length == 0) - { - RejectRequestLine(span); - } - else - { - RejectRequest(RequestRejectionReason.UnrecognizedHTTPVersion, - versionSpan.GetAsciiStringEscaped(32)); - } - } - } - break; - case StartLineState.NewLine: - if (data[i] != ByteLF) - { - RejectRequestLine(span); - } - i++; - - goto case StartLineState.Complete; - case StartLineState.Complete: - done = true; - break; + pathEncoded = true; + } + else if (pathStart == -1) + { + pathStart = offset; } } - if (!done) + if (pathStart == -1) { - RejectRequestLine(span); + // End of path not found + RejectRequestLine(data, length); } - var pathBuffer = span.Slice(pathStart, pathEnd - pathStart); - var targetBuffer = span.Slice(pathStart, queryEnd - pathStart); - var query = span.Slice(queryStart, queryEnd - queryStart); + var pathBuffer = new Span(data + pathStart, offset - pathStart); - handler.OnStartLine(method, httpVersion, targetBuffer, pathBuffer, query, customMethod); + var queryStart = offset; + // Query string + if (ch == ByteQuestionMark) + { + // We have a query string + for (; offset < length; offset++) + { + ch = data[offset]; + if (ch == ByteSpace) + { + break; + } + } + } - consumed = end; - examined = consumed; - return true; + var targetBuffer = new Span(data + pathStart, offset - pathStart); + var query = new Span(data + queryStart, offset - queryStart); + + // Consume space + offset++; + + // Version + var httpVersion = HttpUtilities.GetKnownVersion(data + offset, length - offset); + if (httpVersion == HttpVersion.Unknown) + { + RejectUnknownVersion(data, length, offset); + } + + // After version 8 bytes and cr 1 byte, expect lf + if (data[offset + 8 + 1] != ByteLF) + { + RejectRequestLine(data, length); + } + + handler.OnStartLine(method, httpVersion, targetBuffer, pathBuffer, query, customMethod, pathEncoded); } public unsafe bool ParseHeaders(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined, out int consumedBytes) where T : IHttpHeadersHandler @@ -502,6 +413,48 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http return true; } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool TryGetNewLineSpan(ref ReadableBuffer buffer, ref Span span, out ReadCursor end) + { + var start = buffer.Start; + if (ReadCursorOperations.Seek(start, buffer.End, out end, ByteLF) != -1) + { + // Move 1 byte past the \n + end = buffer.Move(end, 1); + span = buffer.Slice(start, end).ToSpan(); + return true; + } + return false; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private unsafe Span GetUnknownMethod(byte* data, int length, out int methodLength) + { + methodLength = 0; + for (var i = 0; i < length; i++) + { + var ch = data[i]; + + if (ch == ByteSpace) + { + if (i == 0) + { + RejectRequestLine(data, length); + } + + methodLength = i; + break; + } + else if (!IsValidTokenChar((char)ch)) + { + RejectRequestLine(data, length); + } + } + + return new Span(data, methodLength); + } + private static bool IsValidTokenChar(char c) { // Determines if a character is valid as a 'token' as defined in the @@ -532,9 +485,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http throw BadHttpRequestException.GetException(reason); } - public static void RejectRequest(RequestRejectionReason reason, string value) + private unsafe void RejectUnknownVersion(byte* data, int length, int versionStart) { - throw BadHttpRequestException.GetException(reason, value); + throw GetRejectUnknownVersion(data, length, versionStart); + } + + private unsafe void RejectRequestLine(byte* data, int length) + { + throw GetRejectRequestLineException(new Span(data, length)); } private void RejectRequestLine(Span span) @@ -549,6 +507,30 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http Log.IsEnabled(LogLevel.Information) ? span.GetAsciiStringEscaped(MaxRequestLineError) : string.Empty); } + private unsafe BadHttpRequestException GetRejectUnknownVersion(byte* data, int length, int versionStart) + { + var span = new Span(data, length); + length -= versionStart; + for (var i = 0; i < length; i++) + { + var ch = span[i + versionStart]; + if (ch == ByteCR) + { + if (i == 0) + { + return GetRejectRequestLineException(span); + } + else + { + return BadHttpRequestException.GetException(RequestRejectionReason.UnrecognizedHTTPVersion, + span.Slice(versionStart, i).GetAsciiStringEscaped(32)); + } + } + } + + return GetRejectRequestLineException(span); + } + private unsafe void RejectRequestHeader(byte* headerLine, int length) { RejectRequestHeader(new Span(headerLine, length)); @@ -578,26 +560,5 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // https://github.com/dotnet/coreclr/issues/7459#issuecomment-253965670 return Vector.AsVectorByte(new Vector(vectorByte * 0x01010101u)); } - - private enum HeaderState - { - Name, - Whitespace, - ExpectValue, - ExpectNewLine, - Complete - } - - private enum StartLineState - { - KnownMethod, - UnknownMethod, - Path, - QueryString, - KnownVersion, - UnknownVersion, - NewLine, - Complete - } } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs index f93ceafc00..182d424eb9 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs @@ -147,34 +147,46 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure /// /// true if the input matches a known string, false otherwise. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool GetKnownMethod(this Span span, out HttpMethod method, out int length) + public static unsafe bool GetKnownMethod(this Span span, out HttpMethod method, out int length) { - if (span.TryRead(out var possiblyGet)) + fixed (byte* data = &span.DangerousGetPinnableReference()) { - if (possiblyGet == _httpGetMethodInt) - { - length = 3; - method = HttpMethod.Get; - return true; - } + method = GetKnownMethod(data, span.Length, out length); + return method != HttpMethod.Custom; } + } - if (span.TryRead(out var value)) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal unsafe static HttpMethod GetKnownMethod(byte* data, int length, out int methodLength) + { + methodLength = 0; + if (length < sizeof(uint)) { + return HttpMethod.Custom; + } + else if (*(uint*)data == _httpGetMethodInt) + { + methodLength = 3; + return HttpMethod.Get; + } + else if (length < sizeof(ulong)) + { + return HttpMethod.Custom; + } + else + { + var value = *(ulong*)data; foreach (var x in _knownMethods) { if ((value & x.Item1) == x.Item2) { - method = x.Item3; - length = x.Item4; - return true; + methodLength = x.Item4; + return x.Item3; } } } - method = HttpMethod.Custom; - length = 0; - return false; + return HttpMethod.Custom; } /// @@ -189,36 +201,56 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure /// /// true if the input matches a known string, false otherwise. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool GetKnownVersion(this Span span, out HttpVersion knownVersion, out byte length) + public static unsafe bool GetKnownVersion(this Span span, out HttpVersion knownVersion, out byte length) { - if (span.TryRead(out var version)) + fixed (byte* data = &span.DangerousGetPinnableReference()) { - if (version == _http11VersionLong) + knownVersion = GetKnownVersion(data, span.Length); + if (knownVersion != HttpVersion.Unknown) { length = sizeof(ulong); - knownVersion = HttpVersion.Http11; - } - else if (version == _http10VersionLong) - { - length = sizeof(ulong); - knownVersion = HttpVersion.Http10; - } - else - { - length = 0; - knownVersion = HttpVersion.Unknown; - return false; - } - - if (span[sizeof(ulong)] == (byte)'\r') - { return true; } + + length = 0; + return false; + } + } + + /// + /// Checks 9 bytes from correspond to a known HTTP version. + /// + /// + /// A "known HTTP version" Is is either HTTP/1.0 or HTTP/1.1. + /// Since those fit in 8 bytes, they can be optimally looked up by reading those bytes as a long. Once + /// in that format, it can be checked against the known versions. + /// The Known versions will be checked with the required '\r'. + /// To optimize performance the HTTP/1.1 will be checked first. + /// + /// true if the input matches a known string, false otherwise. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal unsafe static HttpVersion GetKnownVersion(byte* location, int length) + { + HttpVersion knownVersion; + var version = *(ulong*)location; + if (length < sizeof(ulong) + 1 || location[sizeof(ulong)] != (byte)'\r') + { + knownVersion = HttpVersion.Unknown; + } + else if (version == _http11VersionLong) + { + knownVersion = HttpVersion.Http11; + } + else if (version == _http10VersionLong) + { + knownVersion = HttpVersion.Http10; + } + else + { + knownVersion = HttpVersion.Unknown; } - knownVersion = HttpVersion.Unknown; - length = 0; - return false; + return knownVersion; } public static string VersionToString(HttpVersion httpVersion) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs index acb39238bc..3c077d6bf5 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs @@ -119,7 +119,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public bool ParseRequestLine(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined) where T : IHttpRequestLineHandler { - handler.OnStartLine(HttpMethod.Get, HttpVersion.Http11, new Span(_target), new Span(_target), Span.Empty, Span.Empty); + handler.OnStartLine(HttpMethod.Get, HttpVersion.Http11, new Span(_target), new Span(_target), Span.Empty, Span.Empty, false); consumed = buffer.Start; examined = buffer.End; diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs index f77346ee8b..a4da66cbe6 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs @@ -66,7 +66,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance } } - public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod) + public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, bool pathEncoded) { } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs index 6ad29a68cb..4605f5d8a0 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs @@ -49,14 +49,16 @@ namespace Microsoft.AspNetCore.Server.KestrelTests It.IsAny>(), It.IsAny>(), It.IsAny>(), - It.IsAny>())) - .Callback, Span, Span, Span>((method, version, target, path, query, customMethod) => + It.IsAny>(), + It.IsAny())) + .Callback, Span, Span, Span, bool>((method, version, target, path, query, customMethod, pathEncoded) => { parsedMethod = method != HttpMethod.Custom ? HttpUtilities.MethodToString(method) : customMethod.GetAsciiStringNonNullCharacters(); parsedVersion = HttpUtilities.VersionToString(version); parsedRawTarget = target.GetAsciiStringNonNullCharacters(); parsedRawPath = path.GetAsciiStringNonNullCharacters(); parsedQuery = query.GetAsciiStringNonNullCharacters(); + pathEncoded = false; }); Assert.True(parser.ParseRequestLine(requestLineHandler.Object, buffer, out var consumed, out var examined)); From 49b328d4c20a7a2f39748390022415671e33a6ca Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Thu, 9 Mar 2017 16:54:12 -0800 Subject: [PATCH 8/9] Handle absolute, asterisk, and authority-form request targets Improves compliance with RFC 7230 on the expected handling of requests that have URI or asterisk in the request target. This means rejecting asterisk requests that are not OPTIONS and rejecting authority-form requests taht are not CONNECT. This also means the server will handle the path and query on targets with absolute URIs as request-targets. --- .../BadHttpRequestException.cs | 19 ++ .../Internal/Http/Frame.cs | 189 +++++++++++++++--- .../Internal/Http/HttpScheme.cs | 12 ++ .../Internal/Http/IHttpRequestLineHandler.cs | 2 +- .../Internal/Http/KestrelHttpParser.cs | 8 +- .../Internal/Http/RequestRejectionReason.cs | 4 +- .../Internal/Infrastructure/HttpUtilities.cs | 47 +++++ .../Internal/Infrastructure/UriUtilities.cs | 35 ++++ .../BadHttpRequestTests.cs | 40 +++- .../RequestTests.cs | 66 ++++++ .../FrameParsingOverhead.cs | 10 +- .../KestrelHttpParser.cs | 2 +- .../FrameTests.cs | 51 +++++ .../HttpParserTests.cs | 3 +- .../HttpUtilitiesTest.cs | 14 +- .../RequestTargetProcessingTests.cs | 20 +- test/shared/HttpParsingData.cs | 89 ++++++++- 17 files changed, 557 insertions(+), 54 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/HttpScheme.cs create mode 100644 src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/UriUtilities.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs index 09aaf94a45..e735506074 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs @@ -4,19 +4,32 @@ using System.IO; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; +using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Server.Kestrel { public sealed class BadHttpRequestException : IOException { private BadHttpRequestException(string message, int statusCode) + : this(message, statusCode, null) + { } + + private BadHttpRequestException(string message, int statusCode, HttpMethod? requiredMethod) : base(message) { StatusCode = statusCode; + + if (requiredMethod.HasValue) + { + AllowedHeader = HttpUtilities.MethodToString(requiredMethod.Value); + } } internal int StatusCode { get; } + internal StringValues AllowedHeader { get; } + internal static BadHttpRequestException GetException(RequestRejectionReason reason) { BadHttpRequestException ex; @@ -61,6 +74,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel case RequestRejectionReason.RequestTimeout: ex = new BadHttpRequestException("Request timed out.", StatusCodes.Status408RequestTimeout); break; + case RequestRejectionReason.OptionsMethodRequired: + ex = new BadHttpRequestException("Method not allowed.", StatusCodes.Status405MethodNotAllowed, HttpMethod.Options); + break; + case RequestRejectionReason.ConnectMethodRequired: + ex = new BadHttpRequestException("Method not allowed.", StatusCodes.Status405MethodNotAllowed, HttpMethod.Connect); + break; default: ex = new BadHttpRequestException("Bad request.", StatusCodes.Status400BadRequest); break; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs index c8a5e20033..921646530c 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs @@ -11,7 +11,6 @@ using System.Net; using System.Runtime.CompilerServices; using System.Text; using System.Text.Encodings.Web.Utf8; -using System.Text.Utf8; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -27,6 +26,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { public abstract partial class Frame : IFrameControl, IHttpRequestLineHandler, IHttpHeadersHandler { + private const byte ByteAsterisk = (byte)'*'; + private const byte ByteForwardSlash = (byte)'/'; private const byte BytePercentage = (byte)'%'; private static readonly ArraySegment _endChunkedResponseBytes = CreateAsciiByteArraySegment("0\r\n\r\n"); @@ -39,6 +40,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http private static readonly byte[] _bytesEndHeaders = Encoding.ASCII.GetBytes("\r\n\r\n"); private static readonly byte[] _bytesServer = Encoding.ASCII.GetBytes("\r\nServer: Kestrel"); + private const string EmptyPath = "/"; + private const string Asterisk = "*"; + private readonly object _onStartingSync = new Object(); private readonly object _onCompletedSync = new Object(); @@ -818,7 +822,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // that should take precedence. if (_requestRejectedException != null) { - SetErrorResponseHeaders(statusCode: _requestRejectedException.StatusCode); + SetErrorResponseException(_requestRejectedException); } else { @@ -1077,7 +1081,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { buffer = buffer.Slice(buffer.Start, _remainingRequestHeadersBytesAllowed); - // If we sliced it means the current buffer bigger than what we're + // If we sliced it means the current buffer bigger than what we're // allowed to look at overLength = true; } @@ -1127,6 +1131,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } } + private void SetErrorResponseException(BadHttpRequestException ex) + { + SetErrorResponseHeaders(ex.StatusCode); + + if (!StringValues.IsNullOrEmpty(ex.AllowedHeader)) + { + FrameResponseHeaders.HeaderAllow = ex.AllowedHeader; + } + } + private void SetErrorResponseHeaders(int statusCode) { Debug.Assert(!HasResponseStarted, $"{nameof(SetErrorResponseHeaders)} called after response had already started."); @@ -1179,6 +1193,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http throw BadHttpRequestException.GetException(reason, value); } + private void RejectRequestLine(Span requestLine) + { + Debug.Assert(Log.IsEnabled(LogLevel.Information) == true, "Use RejectRequest instead to improve inlining when log is disabled"); + + const int MaxRequestLineError = 32; + var line = requestLine.GetAsciiStringEscaped(MaxRequestLineError); + throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine, line); + } + public void SetBadRequestState(RequestRejectionReason reason) { SetBadRequestState(BadHttpRequestException.GetException(reason)); @@ -1190,7 +1213,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http if (!HasResponseStarted) { - SetErrorResponseHeaders(ex.StatusCode); + SetErrorResponseException(ex); } _keepAlive = false; @@ -1216,8 +1239,51 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http Log.ApplicationError(ConnectionId, ex); } - public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, bool pathEncoded) + public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, Span line, bool pathEncoded) { + Debug.Assert(target.Length != 0, "Request target must be non-zero length"); + + var ch = target[0]; + if (ch == ByteForwardSlash) + { + // origin-form. + // The most common form of request-target. + // https://tools.ietf.org/html/rfc7230#section-5.3.1 + OnOriginFormTarget(method, version, target, path, query, customMethod, pathEncoded); + } + else if (ch == ByteAsterisk && target.Length == 1) + { + OnAsteriskFormTarget(method); + } + else if (target.GetKnownHttpScheme(out var scheme)) + { + OnAbsoluteFormTarget(target, query, line); + } + else + { + // Assume anything else is considered authority form. + // FYI: this should be an edge case. This should only happen when + // a client mistakenly things this server is a proxy server. + + OnAuthorityFormTarget(method, target, line); + } + + Method = method != HttpMethod.Custom + ? HttpUtilities.MethodToString(method) ?? string.Empty + : customMethod.GetAsciiStringNonNullCharacters(); + HttpVersion = HttpUtilities.VersionToString(version); + + Debug.Assert(RawTarget != null, "RawTarget was not set"); + Debug.Assert(Method != null, "Method was not set"); + Debug.Assert(Path != null, "Path was not set"); + Debug.Assert(QueryString != "QueryString was not set"); + Debug.Assert(HttpVersion != "HttpVersion was not set"); + } + + private void OnOriginFormTarget(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, bool pathEncoded) + { + Debug.Assert(target[0] == ByteForwardSlash, "Should only be called when path starts with /"); + // URIs are always encoded/escaped to ASCII https://tools.ietf.org/html/rfc3986#page-11 // Multibyte Internationalized Resource Identifiers (IRIs) are first converted to utf8; // then encoded/escaped to ASCII https://www.ietf.org/rfc/rfc3987.txt "Mapping of IRIs to URIs" @@ -1249,34 +1315,111 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } } - var normalizedTarget = PathNormalizer.RemoveDotSegments(requestUrlPath); - if (method != HttpMethod.Custom) - { - Method = HttpUtilities.MethodToString(method) ?? string.Empty; - } - else - { - Method = customMethod.GetAsciiStringNonNullCharacters(); - } - QueryString = query.GetAsciiStringNonNullCharacters(); RawTarget = rawTarget; - HttpVersion = HttpUtilities.VersionToString(version); + SetNormalizedPath(requestUrlPath); + } + private void OnAuthorityFormTarget(HttpMethod method, Span target, Span line) + { + // TODO Validate that target is a correct host[:port] string. + // Reject as 400 if not. This is just a quick scan for invalid characters + // but doesn't check that the target fully matches the URI spec. + for (var i = 0; i < target.Length; i++) + { + var ch = target[i]; + if (!UriUtilities.IsValidAuthorityCharacter(ch)) + { + if (Log.IsEnabled(LogLevel.Information)) + { + RejectRequestLine(line); + } + + throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine); + } + } + + // The authority-form of request-target is only used for CONNECT + // requests (https://tools.ietf.org/html/rfc7231#section-4.3.6). + if (method != HttpMethod.Connect) + { + RejectRequest(RequestRejectionReason.ConnectMethodRequired); + } + + // When making a CONNECT request to establish a tunnel through one or + // more proxies, a client MUST send only the target URI's authority + // component(excluding any userinfo and its "@" delimiter) as the + // request - target.For example, + // + // CONNECT www.example.com:80 HTTP/1.1 + // + // Allowed characters in the 'host + port' section of authority. + // See https://tools.ietf.org/html/rfc3986#section-3.2 + + RawTarget = target.GetAsciiStringNonNullCharacters(); + Path = string.Empty; + PathBase = string.Empty; + QueryString = string.Empty; + } + + private void OnAsteriskFormTarget(HttpMethod method) + { + // The asterisk-form of request-target is only used for a server-wide + // OPTIONS request (https://tools.ietf.org/html/rfc7231#section-4.3.7). + if (method != HttpMethod.Options) + { + RejectRequest(RequestRejectionReason.OptionsMethodRequired); + } + + RawTarget = Asterisk; + Path = string.Empty; + PathBase = string.Empty; + QueryString = string.Empty; + } + + private void OnAbsoluteFormTarget(Span target, Span query, Span line) + { + // absolute-form + // https://tools.ietf.org/html/rfc7230#section-5.3.2 + + // This code should be the edge-case. + + // From the spec: + // a server MUST accept the absolute-form in requests, even though + // HTTP/1.1 clients will only send them in requests to proxies. + + RawTarget = target.GetAsciiStringNonNullCharacters(); + + // Validation of absolute URIs is slow, but clients + // should not be sending this form anyways, so perf optimization + // not high priority + + if (!Uri.TryCreate(RawTarget, UriKind.Absolute, out var uri)) + { + if (Log.IsEnabled(LogLevel.Information)) + { + RejectRequestLine(line); + } + + throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine); + } + + SetNormalizedPath(uri.LocalPath); + // don't use uri.Query because we need the unescaped version + QueryString = query.GetAsciiStringNonNullCharacters(); + } + + private void SetNormalizedPath(string requestPath) + { + var normalizedTarget = PathNormalizer.RemoveDotSegments(requestPath); if (RequestUrlStartsWithPathBase(normalizedTarget, out bool caseMatches)) { PathBase = caseMatches ? _pathBase : normalizedTarget.Substring(0, _pathBase.Length); Path = normalizedTarget.Substring(_pathBase.Length); } - else if (rawTarget[0] == '/') // check rawTarget since normalizedTarget can be "" or "/" after dot segment removal - { - Path = normalizedTarget; - } else { - Path = string.Empty; - PathBase = string.Empty; - QueryString = string.Empty; + Path = normalizedTarget; } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/HttpScheme.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/HttpScheme.cs new file mode 100644 index 0000000000..23f0bed5b9 --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/HttpScheme.cs @@ -0,0 +1,12 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http +{ + public enum HttpScheme + { + Unknown = -1, + Http = 0, + Https = 1 + } +} diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs index 83481002b3..ed53d928f3 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs @@ -7,6 +7,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { public interface IHttpRequestLineHandler { - void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, bool pathEncoded); + void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, Span line, bool pathEncoded); } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index 02363a2325..2e118c9809 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -157,7 +157,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http RejectRequestLine(data, length); } - handler.OnStartLine(method, httpVersion, targetBuffer, pathBuffer, query, customMethod, pathEncoded); + var line = new Span(data, length); + + handler.OnStartLine(method, httpVersion, targetBuffer, pathBuffer, query, customMethod, line, pathEncoded); } public unsafe bool ParseHeaders(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined, out int consumedBytes) where T : IHttpHeadersHandler @@ -341,7 +343,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // Skip colon from value start var valueStart = nameEnd + 1; // Ignore start whitespace - for(; valueStart < valueEnd; valueStart++) + for (; valueStart < valueEnd; valueStart++) { var ch = headerLine[valueStart]; if (ch != ByteTab && ch != ByteSpace && ch != ByteCR) @@ -409,7 +411,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } } return false; - found: + found: return true; } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs index 92faea1965..ce68141b93 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs @@ -23,6 +23,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http RequestTimeout, FinalTransferCodingNotChunked, LengthRequired, - LengthRequiredHttp10 + LengthRequiredHttp10, + OptionsMethodRequired, + ConnectMethodRequired, } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs index 182d424eb9..3def317d96 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs @@ -15,7 +15,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure public const string Http10Version = "HTTP/1.0"; public const string Http11Version = "HTTP/1.1"; + public const string HttpUriScheme = "http://"; + public const string HttpsUriScheme = "https://"; + // readonly primitive statics can be Jit'd to consts https://github.com/dotnet/coreclr/issues/1079 + + private readonly static ulong _httpSchemeLong = GetAsciiStringAsLong(HttpUriScheme + "\0"); + private readonly static ulong _httpsSchemeLong = GetAsciiStringAsLong(HttpsUriScheme); private readonly static ulong _httpConnectMethodLong = GetAsciiStringAsLong("CONNECT "); private readonly static ulong _httpDeleteMethodLong = GetAsciiStringAsLong("DELETE \0"); private const uint _httpGetMethodInt = 542393671; // retun of GetAsciiStringAsInt("GET "); const results in better codegen @@ -253,6 +259,34 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure return knownVersion; } + /// + /// Checks 8 bytes from that correspond to 'http://' or 'https://' + /// + /// The span + /// A reference to the known scheme, if the input matches any + /// True when memory starts with known http or https schema + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool GetKnownHttpScheme(this Span span, out HttpScheme knownScheme) + { + if (span.TryRead(out var value)) + { + if ((value & _mask7Chars) == _httpSchemeLong) + { + knownScheme = HttpScheme.Http; + return true; + } + + if (value == _httpsSchemeLong) + { + knownScheme = HttpScheme.Https; + return true; + } + } + + knownScheme = HttpScheme.Unknown; + return false; + } + public static string VersionToString(HttpVersion httpVersion) { switch (httpVersion) @@ -274,5 +308,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure } return null; } + + public static string SchemeToString(HttpScheme scheme) + { + switch (scheme) + { + case HttpScheme.Http: + return HttpUriScheme; + case HttpScheme.Https: + return HttpsUriScheme; + default: + return null; + } + } } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/UriUtilities.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/UriUtilities.cs new file mode 100644 index 0000000000..defcb9eb4f --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/UriUtilities.cs @@ -0,0 +1,35 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure +{ + public class UriUtilities + { + /// + /// Returns true if character is valid in the 'authority' section of a URI. + /// + /// + /// The character + /// + public static bool IsValidAuthorityCharacter(byte ch) + { + // Examples: + // microsoft.com + // hostname:8080 + // [::]:8080 + // [fe80::] + // 127.0.0.1 + // user@host.com + // user:password@host.com + return + (ch >= '0' && ch <= '9') || + (ch >= 'A' && ch <= 'Z') || + (ch >= 'a' && ch <= 'z') || + ch == ':' || + ch == '.' || + ch == '[' || + ch == ']' || + ch == '@'; + } + } +} diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs index 380a51188e..3d3711cad6 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; @@ -90,7 +91,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests $"Invalid content length: {contentLength}"); } - private async Task TestBadRequest(string request, string expectedResponseStatusCode, string expectedExceptionMessage) + [Theory] + [InlineData("GET *", "OPTIONS")] + [InlineData("GET www.host.com", "CONNECT")] + public Task RejectsIncorrectMethods(string request, string allowedMethod) + { + return TestBadRequest( + $"{request} HTTP/1.1\r\n", + "405 Method Not Allowed", + "Method not allowed.", + $"Allow: {allowedMethod}"); + } + + private async Task TestBadRequest(string request, string expectedResponseStatusCode, string expectedExceptionMessage, string expectedAllowHeader = null) { BadHttpRequestException loggedException = null; var mockKestrelTrace = new Mock(); @@ -106,7 +119,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests using (var connection = server.CreateConnection()) { await connection.SendAll(request); - await ReceiveBadRequestResponse(connection, expectedResponseStatusCode, server.Context.DateHeaderValue); + await ReceiveBadRequestResponse(connection, expectedResponseStatusCode, server.Context.DateHeaderValue, expectedAllowHeader); } } @@ -114,15 +127,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests Assert.Equal(expectedExceptionMessage, loggedException.Message); } - private async Task ReceiveBadRequestResponse(TestConnection connection, string expectedResponseStatusCode, string expectedDateHeaderValue) + private async Task ReceiveBadRequestResponse(TestConnection connection, string expectedResponseStatusCode, string expectedDateHeaderValue, string expectedAllowHeader = null) { - await connection.ReceiveForcedEnd( + var lines = new[] + { $"HTTP/1.1 {expectedResponseStatusCode}", "Connection: close", $"Date: {expectedDateHeaderValue}", "Content-Length: 0", + expectedAllowHeader, "", - ""); + "" + }; + + await connection.ReceiveForcedEnd(lines.Where(f => f != null).ToArray()); } public static TheoryData InvalidRequestLineData @@ -131,9 +149,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { var data = new TheoryData(); + string Escape(string line) + { + return line + .Replace("\r", @"\x0D") + .Replace("\n", @"\x0A") + .Replace("\0", @"\x00"); + } + foreach (var requestLine in HttpParsingData.RequestLineInvalidData) { - data.Add(requestLine, $"Invalid request line: '{requestLine.Replace("\r", "\\x0D").Replace("\n", "\\x0A")}'"); + data.Add(requestLine, $"Invalid request line: '{Escape(requestLine)}'"); } foreach (var requestLine in HttpParsingData.RequestLineWithEncodedNullCharInTargetData) @@ -143,7 +169,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests foreach (var requestLine in HttpParsingData.RequestLineWithNullCharInTargetData) { - data.Add(requestLine, "Invalid request line."); + data.Add(requestLine, $"Invalid request line."); } return data; diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs index 77ae582bfa..4e5afb14df 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs @@ -454,6 +454,72 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } + [Theory] + [InlineData("http://localhost/abs/path", "/abs/path", null)] + [InlineData("https://localhost/abs/path", "/abs/path", null)] // handles mismatch scheme + [InlineData("https://localhost:22/abs/path", "/abs/path", null)] // handles mismatched ports + [InlineData("https://differenthost/abs/path", "/abs/path", null)] // handles mismatched hostname + [InlineData("http://localhost/", "/", null)] + [InlineData("http://root@contoso.com/path", "/path", null)] + [InlineData("http://root:password@contoso.com/path", "/path", null)] + [InlineData("https://localhost/", "/", null)] + [InlineData("http://localhost", "/", null)] + [InlineData("http://127.0.0.1/", "/", null)] + [InlineData("http://[::1]/", "/", null)] + [InlineData("http://[::1]:8080/", "/", null)] + [InlineData("http://localhost?q=123&w=xyz", "/", "123")] + [InlineData("http://localhost/?q=123&w=xyz", "/", "123")] + [InlineData("http://localhost/path?q=123&w=xyz", "/path", "123")] + [InlineData("http://localhost/path%20with%20space?q=abc%20123", "/path with space", "abc 123")] + public async Task CanHandleRequestsWithUrlInAbsoluteForm(string requestUrl, string expectedPath, string queryValue) + { + var pathTcs = new TaskCompletionSource(); + var rawTargetTcs = new TaskCompletionSource(); + var hostTcs = new TaskCompletionSource(); + var queryTcs = new TaskCompletionSource(); + + using (var server = new TestServer(async context => + { + pathTcs.TrySetResult(context.Request.Path); + hostTcs.TrySetResult(context.Request.Host); + queryTcs.TrySetResult(context.Request.Query); + rawTargetTcs.TrySetResult(context.Features.Get().RawTarget); + await context.Response.WriteAsync("Done"); + })) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + $"GET {requestUrl} HTTP/1.1", + "Content-Length: 0", + "Host: localhost", + "", + ""); + + await connection.Receive($"HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Transfer-Encoding: chunked", + "", + "4", + "Done") + .TimeoutAfter(TimeSpan.FromSeconds(10)); + + await Task.WhenAll(pathTcs.Task, rawTargetTcs.Task, hostTcs.Task, queryTcs.Task).TimeoutAfter(TimeSpan.FromSeconds(30)); + Assert.Equal(new PathString(expectedPath), pathTcs.Task.Result); + Assert.Equal(requestUrl, rawTargetTcs.Task.Result); + Assert.Equal("localhost", hostTcs.Task.Result.ToString()); + if (queryValue == null) + { + Assert.False(queryTcs.Task.Result.ContainsKey("q")); + } + else + { + Assert.Equal(queryValue, queryTcs.Task.Result["q"]); + } + } + } + } + private async Task TestRemoteIPAddress(string registerAddress, string requestAddress, string expectAddress) { var builder = new WebHostBuilder() diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs index 3c077d6bf5..3c4276b619 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs @@ -94,6 +94,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance private class NullParser : IHttpParser { + private readonly byte[] _startLine = Encoding.ASCII.GetBytes("GET /plaintext HTTP/1.1\r\n"); private readonly byte[] _target = Encoding.ASCII.GetBytes("/plaintext"); private readonly byte[] _hostHeaderName = Encoding.ASCII.GetBytes("Host"); private readonly byte[] _hostHeaderValue = Encoding.ASCII.GetBytes("www.example.com"); @@ -119,7 +120,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public bool ParseRequestLine(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined) where T : IHttpRequestLineHandler { - handler.OnStartLine(HttpMethod.Get, HttpVersion.Http11, new Span(_target), new Span(_target), Span.Empty, Span.Empty, false); + handler.OnStartLine(HttpMethod.Get, + HttpVersion.Http11, + new Span(_target), + new Span(_target), + Span.Empty, + Span.Empty, + new Span(_startLine), + false); consumed = buffer.Start; examined = buffer.End; diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs index a4da66cbe6..e49b613a45 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs @@ -66,7 +66,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance } } - public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, bool pathEncoded) + public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, Span line, bool pathEncoded) { } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index bbac629985..ed28c1d86b 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -15,6 +15,7 @@ using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Internal; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Internal; using Moq; @@ -328,6 +329,37 @@ namespace Microsoft.AspNetCore.Server.KestrelTests Assert.Equal(new InvalidOperationException().Message, exception.Message); } + [Theory] + [MemberData(nameof(RequestLineWithInvalidRequestTargetData))] + public async Task TakeStartLineThrowsWhenRequestTargetIsInvalid(string requestLine) + { + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes(requestLine)); + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; + + var exception = Assert.Throws(() => + _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); + _input.Reader.Advance(_consumed, _examined); + + Assert.Equal($"Invalid request line: '{Escape(requestLine)}'", exception.Message); + } + + + [Theory] + [MemberData(nameof(MethodNotAllowedTargetData))] + public async Task TakeStartLineThrowsWhenMethodNotAllowed(string requestLine, HttpMethod allowedMethod) + { + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes(requestLine)); + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; + + var exception = Assert.Throws(() => + _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); + _input.Reader.Advance(_consumed, _examined); + + Assert.Equal(405, exception.StatusCode); + Assert.Equal("Method not allowed.", exception.Message); + Assert.Equal(HttpUtilities.MethodToString(allowedMethod), exception.AllowedHeader); + } + [Fact] public void RequestProcessingAsyncEnablesKeepAliveTimeout() { @@ -516,6 +548,25 @@ namespace Microsoft.AspNetCore.Server.KestrelTests } } + private string Escape(string requestLine) + { + var ellipsis = requestLine.Length > 32 + ? "..." + : string.Empty; + return requestLine + .Substring(0, Math.Min(32, requestLine.Length)) + .Replace("\r", @"\x0D") + .Replace("\n", @"\x0A") + .Replace("\0", @"\x00") + + ellipsis; + } + + public static TheoryData RequestLineWithInvalidRequestTargetData + => HttpParsingData.RequestLineWithInvalidRequestTarget; + + public static TheoryData MethodNotAllowedTargetData + => HttpParsingData.MethodNotAllowedRequestLine; + public static TheoryData RequestLineWithNullCharInTargetData { get diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs index 4605f5d8a0..bd27800679 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs @@ -50,8 +50,9 @@ namespace Microsoft.AspNetCore.Server.KestrelTests It.IsAny>(), It.IsAny>(), It.IsAny>(), + It.IsAny>(), It.IsAny())) - .Callback, Span, Span, Span, bool>((method, version, target, path, query, customMethod, pathEncoded) => + .Callback, Span, Span, Span, Span, bool>((method, version, target, path, query, customMethod, line, pathEncoded) => { parsedMethod = method != HttpMethod.Custom ? HttpUtilities.MethodToString(method) : customMethod.GetAsciiStringNonNullCharacters(); parsedVersion = HttpUtilities.VersionToString(version); diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpUtilitiesTest.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpUtilitiesTest.cs index 6dd176e553..18b7a111c9 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpUtilitiesTest.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpUtilitiesTest.cs @@ -84,11 +84,23 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { TestKnownStringsInterning(input, expected, span => { - HttpUtilities.GetKnownVersion(span, out var version, out var lenght); + HttpUtilities.GetKnownVersion(span, out var version, out var _); return HttpUtilities.VersionToString(version); }); } + [Theory] + [InlineData("https://host/", "https://")] + [InlineData("http://host/", "http://")] + public void KnownSchemesAreInterned(string input, string expected) + { + TestKnownStringsInterning(input, expected, span => + { + HttpUtilities.GetKnownHttpScheme(span, out var scheme); + return HttpUtilities.SchemeToString(scheme); + }); + } + [Theory] [InlineData("CONNECT / HTTP/1.1", "CONNECT")] [InlineData("DELETE / HTTP/1.1", "DELETE")] diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/RequestTargetProcessingTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/RequestTargetProcessingTests.cs index 7a1d479f29..2da9a8144c 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/RequestTargetProcessingTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/RequestTargetProcessingTests.cs @@ -6,6 +6,8 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; using Xunit; @@ -93,19 +95,9 @@ namespace Microsoft.AspNetCore.Server.KestrelTests } [Theory] - [InlineData("*")] - [InlineData("*/?arg=value")] - [InlineData("*?arg=value")] - [InlineData("DoesNotStartWith/")] - [InlineData("DoesNotStartWith/?arg=value")] - [InlineData("DoesNotStartWithSlash?arg=value")] - [InlineData("./")] - [InlineData("../")] - [InlineData("../.")] - [InlineData(".././")] - [InlineData("../..")] - [InlineData("../../")] - public async Task NonPathRequestTargetSetInRawTarget(string requestTarget) + [InlineData(HttpMethod.Options, "*")] + [InlineData(HttpMethod.Connect, "host")] + public async Task NonPathRequestTargetSetInRawTarget(HttpMethod method, string requestTarget) { var testContext = new TestServiceContext(); @@ -123,7 +115,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var connection = server.CreateConnection()) { await connection.Send( - $"GET {requestTarget} HTTP/1.1", + $"{HttpUtilities.MethodToString(method)} {requestTarget} HTTP/1.1", "", ""); await connection.ReceiveEnd( diff --git a/test/shared/HttpParsingData.cs b/test/shared/HttpParsingData.cs index e999fb3170..9e5df5e426 100644 --- a/test/shared/HttpParsingData.cs +++ b/test/shared/HttpParsingData.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; using System; using System.Collections.Generic; using System.Linq; @@ -33,6 +34,26 @@ namespace Microsoft.AspNetCore.Testing Tuple.Create("/%C3%A5/bc", "/\u00E5/bc"), Tuple.Create("/%25", "/%"), Tuple.Create("/%2F", "/%2F"), + Tuple.Create("http://host/abs/path", "/abs/path"), + Tuple.Create("http://host/abs/path/", "/abs/path/"), + Tuple.Create("http://host/a%20b%20c/", "/a b c/"), + Tuple.Create("https://host/abs/path", "/abs/path"), + Tuple.Create("https://host/abs/path/", "/abs/path/"), + Tuple.Create("https://host:22/abs/path", "/abs/path"), + Tuple.Create("https://user@host:9080/abs/path", "/abs/path"), + Tuple.Create("http://host/", "/"), + Tuple.Create("http://host", "/"), + Tuple.Create("https://host/", "/"), + Tuple.Create("https://host", "/"), + Tuple.Create("http://user@host/", "/"), + Tuple.Create("http://127.0.0.1/", "/"), + Tuple.Create("http://user@127.0.0.1/", "/"), + Tuple.Create("http://user@127.0.0.1:8080/", "/"), + Tuple.Create("http://127.0.0.1:8080/", "/"), + Tuple.Create("http://[::1]", "/"), + Tuple.Create("http://[::1]/path", "/path"), + Tuple.Create("http://[::1]:8080/", "/"), + Tuple.Create("http://user@[::1]:8080/", "/"), }; var queryStrings = new[] { @@ -173,9 +194,73 @@ namespace Microsoft.AspNetCore.Testing "GET /%E8%01%00 HTTP/1.1\r\n", }; + public static TheoryData RequestLineWithInvalidRequestTarget => new TheoryData + { + // Invalid absolute-form requests + "GET http:// HTTP/1.1\r\n", + "GET http:/ HTTP/1.1\r\n", + "GET https:/ HTTP/1.1\r\n", + "GET http:/// HTTP/1.1\r\n", + "GET https:// HTTP/1.1\r\n", + "GET http://// HTTP/1.1\r\n", + "GET http://:80 HTTP/1.1\r\n", + "GET http://:80/abc HTTP/1.1\r\n", + "GET http://user@ HTTP/1.1\r\n", + "GET http://user@/abc HTTP/1.1\r\n", + "GET http://abc%20xyz/abc HTTP/1.1\r\n", + "GET http://%20/abc?query=%0A HTTP/1.1\r\n", + // Valid absolute-form but with unsupported schemes + "GET otherscheme://host/ HTTP/1.1\r\n", + "GET ws://host/ HTTP/1.1\r\n", + "GET wss://host/ HTTP/1.1\r\n", + // Must only have one asterisk + "OPTIONS ** HTTP/1.1\r\n", + // Relative form + "GET ../../ HTTP/1.1\r\n", + "GET ..\\. HTTP/1.1\r\n", + }; + + public static TheoryData MethodNotAllowedRequestLine + { + get + { + var methods = new[] + { + "GET", + "PUT", + "DELETE", + "POST", + "HEAD", + "TRACE", + "PATCH", + "CONNECT", + //"OPTIONS", + "CUSTOM", + }; + + var theoryData = new TheoryData(); + foreach (var line in methods + .Select(m => Tuple.Create($"{m} * HTTP/1.1\r\n", HttpMethod.Options)) + .Concat(new[] + { + // CONNECT required for authority-form targets + Tuple.Create("GET http:80 HTTP/1.1\r\n", HttpMethod.Connect), + Tuple.Create("GET http: HTTP/1.1\r\n", HttpMethod.Connect), + Tuple.Create("GET https: HTTP/1.1\r\n", HttpMethod.Connect), + Tuple.Create("GET . HTTP/1.1\r\n", HttpMethod.Connect), + })) + { + theoryData.Add(line.Item1, line.Item2); + } + + return theoryData; + } + } + public static IEnumerable RequestLineWithNullCharInTargetData => new[] { - "GET \0 HTTP/1.1\r\n", + // TODO re-enable after we get both #1469 and #1470 merged + // "GET \0 HTTP/1.1\r\n", "GET /\0 HTTP/1.1\r\n", "GET /\0\0 HTTP/1.1\r\n", "GET /%C8\0 HTTP/1.1\r\n", @@ -183,6 +268,8 @@ namespace Microsoft.AspNetCore.Testing public static TheoryData UnrecognizedHttpVersionData => new TheoryData { + " ", + "/", "H", "HT", "HTT", From 3c8ee39f1de441daf24c63943dc335af92bdf215 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Fri, 10 Mar 2017 20:04:28 +0000 Subject: [PATCH 9/9] Fxi writing perf test (#1478) --- test/Microsoft.AspNetCore.Server.Kestrel.Performance/Writing.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Writing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Writing.cs index 04c8e414f9..9e641715e6 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Writing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Writing.cs @@ -107,6 +107,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance Output = new MockSocketOutput(), ConnectionControl = Mock.Of() }; + connectionContext.ListenerContext.ServiceContext.HttpParserFactory = f => new Internal.Http.KestrelHttpParser(log: null); var frame = new TestFrame(application: null, context: connectionContext); frame.Reset();