diff --git a/KestrelHttpServer.sln b/KestrelHttpServer.sln index 60a967cf1a..48cd152ee9 100644 --- a/KestrelHttpServer.sln +++ b/KestrelHttpServer.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 15 -VisualStudioVersion = 15.0.26222.1 +VisualStudioVersion = 15.0.26228.0 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{7972A5D6-3385-4127-9277-428506DD44FF}" ProjectSection(SolutionItems) = preProject @@ -29,6 +29,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "shared", "shared", "{0EF2AC test\shared\MockSocketOutput.cs = test\shared\MockSocketOutput.cs test\shared\MockSystemClock.cs = test\shared\MockSystemClock.cs test\shared\SocketInputExtensions.cs = test\shared\SocketInputExtensions.cs + test\shared\StringExtensions.cs = test\shared\StringExtensions.cs test\shared\TestApp.cs = test\shared\TestApp.cs test\shared\TestApplicationErrorLogger.cs = test\shared\TestApplicationErrorLogger.cs test\shared\TestConnection.cs = test\shared\TestConnection.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs index e735506074..3c80a8a53a 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs @@ -87,31 +87,34 @@ namespace Microsoft.AspNetCore.Server.Kestrel return ex; } - internal static BadHttpRequestException GetException(RequestRejectionReason reason, string value) + internal static BadHttpRequestException GetException(RequestRejectionReason reason, string detail) { BadHttpRequestException ex; switch (reason) { case RequestRejectionReason.InvalidRequestLine: - ex = new BadHttpRequestException($"Invalid request line: '{value}'", StatusCodes.Status400BadRequest); + ex = new BadHttpRequestException($"Invalid request line: '{detail}'", StatusCodes.Status400BadRequest); + break; + case RequestRejectionReason.InvalidRequestTarget: + ex = new BadHttpRequestException($"Invalid request target: '{detail}'", StatusCodes.Status400BadRequest); break; case RequestRejectionReason.InvalidRequestHeader: - ex = new BadHttpRequestException($"Invalid request header: '{value}'", StatusCodes.Status400BadRequest); + ex = new BadHttpRequestException($"Invalid request header: '{detail}'", StatusCodes.Status400BadRequest); break; case RequestRejectionReason.InvalidContentLength: - ex = new BadHttpRequestException($"Invalid content length: {value}", StatusCodes.Status400BadRequest); + ex = new BadHttpRequestException($"Invalid content length: {detail}", StatusCodes.Status400BadRequest); break; case RequestRejectionReason.UnrecognizedHTTPVersion: - ex = new BadHttpRequestException($"Unrecognized HTTP version: {value}", StatusCodes.Status505HttpVersionNotsupported); + ex = new BadHttpRequestException($"Unrecognized HTTP version: '{detail}'", StatusCodes.Status505HttpVersionNotsupported); break; case RequestRejectionReason.FinalTransferCodingNotChunked: - ex = new BadHttpRequestException($"Final transfer coding is not \"chunked\": \"{value}\"", StatusCodes.Status400BadRequest); + ex = new BadHttpRequestException($"Final transfer coding is not \"chunked\": \"{detail}\"", StatusCodes.Status400BadRequest); break; case RequestRejectionReason.LengthRequired: - ex = new BadHttpRequestException($"{value} request contains no Content-Length or Transfer-Encoding header", StatusCodes.Status411LengthRequired); + ex = new BadHttpRequestException($"{detail} request contains no Content-Length or Transfer-Encoding header", StatusCodes.Status411LengthRequired); break; case RequestRejectionReason.LengthRequiredHttp10: - ex = new BadHttpRequestException($"{value} request contains no Content-Length header", StatusCodes.Status400BadRequest); + ex = new BadHttpRequestException($"{detail} request contains no Content-Length header", StatusCodes.Status400BadRequest); break; default: ex = new BadHttpRequestException("Bad request.", StatusCodes.Status400BadRequest); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs index 921646530c..1f0c0e2fac 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs @@ -1184,23 +1184,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } public void RejectRequest(RequestRejectionReason reason) - { - throw BadHttpRequestException.GetException(reason); - } + => throw BadHttpRequestException.GetException(reason); - public void RejectRequest(RequestRejectionReason reason, string value) - { - throw BadHttpRequestException.GetException(reason, value); - } + public void RejectRequest(RequestRejectionReason reason, string detail) + => throw BadHttpRequestException.GetException(reason, detail); - private void RejectRequestLine(Span requestLine) - { - Debug.Assert(Log.IsEnabled(LogLevel.Information) == true, "Use RejectRequest instead to improve inlining when log is disabled"); + private void RejectRequestTarget(Span target) + => throw GetInvalidRequestTargetException(target); - const int MaxRequestLineError = 32; - var line = requestLine.GetAsciiStringEscaped(MaxRequestLineError); - throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine, line); - } + private BadHttpRequestException GetInvalidRequestTargetException(Span target) + => BadHttpRequestException.GetException( + RequestRejectionReason.InvalidRequestTarget, + Log.IsEnabled(LogLevel.Information) + ? target.GetAsciiStringEscaped(Constants.MaxExceptionDetailSize) + : string.Empty); public void SetBadRequestState(RequestRejectionReason reason) { @@ -1239,7 +1236,7 @@ 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, Span line, bool pathEncoded) + public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod, bool pathEncoded) { Debug.Assert(target.Length != 0, "Request target must be non-zero length"); @@ -1257,15 +1254,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } else if (target.GetKnownHttpScheme(out var scheme)) { - OnAbsoluteFormTarget(target, query, line); + OnAbsoluteFormTarget(target, query); } 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); + // a client mistakenly thinks this server is a proxy server. + OnAuthorityFormTarget(method, target); } Method = method != HttpMethod.Custom @@ -1287,40 +1283,48 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // 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; - if (pathEncoded) - { - // Read raw target before mutating memory. - rawTarget = target.GetAsciiStringNonNullCharacters(); + string requestUrlPath = null; + string rawTarget = null; - // URI was encoded, unescape and then parse as utf8 - int pathLength = UrlEncoder.Decode(path, path); - requestUrlPath = GetUtf8String(path.Slice(0, pathLength)); - } - else + try { - // URI wasn't encoded, parse as ASCII - requestUrlPath = path.GetAsciiStringNonNullCharacters(); - - if (query.Length == 0) + if (pathEncoded) { - // No need to allocate an extra string if the path didn't need - // decoding and there's no query string following it. - rawTarget = requestUrlPath; + // Read raw target before mutating memory. + rawTarget = target.GetAsciiStringNonNullCharacters(); + + // URI was encoded, unescape and then parse as utf8 + int pathLength = UrlEncoder.Decode(path, path); + requestUrlPath = GetUtf8String(path.Slice(0, pathLength)); } else { - rawTarget = target.GetAsciiStringNonNullCharacters(); + // URI wasn't encoded, parse as ASCII + requestUrlPath = path.GetAsciiStringNonNullCharacters(); + + if (query.Length == 0) + { + // No need to allocate an extra string if the path didn't need + // decoding and there's no query string following it. + rawTarget = requestUrlPath; + } + else + { + rawTarget = target.GetAsciiStringNonNullCharacters(); + } } } + catch (InvalidOperationException) + { + RejectRequestTarget(target); + } QueryString = query.GetAsciiStringNonNullCharacters(); RawTarget = rawTarget; SetNormalizedPath(requestUrlPath); } - private void OnAuthorityFormTarget(HttpMethod method, Span target, Span line) + private void OnAuthorityFormTarget(HttpMethod method, Span target) { // TODO Validate that target is a correct host[:port] string. // Reject as 400 if not. This is just a quick scan for invalid characters @@ -1330,12 +1334,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var ch = target[i]; if (!UriUtilities.IsValidAuthorityCharacter(ch)) { - if (Log.IsEnabled(LogLevel.Information)) - { - RejectRequestLine(line); - } - - throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine); + RejectRequestTarget(target); } } @@ -1348,14 +1347,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // 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, + // 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; @@ -1377,7 +1375,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http QueryString = string.Empty; } - private void OnAbsoluteFormTarget(Span target, Span query, Span line) + private void OnAbsoluteFormTarget(Span target, Span query) { // absolute-form // https://tools.ietf.org/html/rfc7230#section-5.3.2 @@ -1396,12 +1394,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http if (!Uri.TryCreate(RawTarget, UriKind.Absolute, out var uri)) { - if (Log.IsEnabled(LogLevel.Information)) - { - RejectRequestLine(line); - } - - throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine); + RejectRequestTarget(target); } SetNormalizedPath(uri.LocalPath); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs index bf9d21e7ab..0065f6b8cf 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs @@ -50,12 +50,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } catch (InvalidOperationException) { - switch (_requestProcessingStatus) + if (_requestProcessingStatus == RequestProcessingStatus.ParsingHeaders) { - case RequestProcessingStatus.ParsingRequestLine: - throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine); - case RequestProcessingStatus.ParsingHeaders: - throw BadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders); + throw BadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders); } throw; } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpRequestLineHandler.cs index ed53d928f3..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, Span line, bool pathEncoded); + 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 2e118c9809..17c0ebbf30 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -117,14 +117,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http if (pathStart == -1) { - // End of path not found + // Start of path not found RejectRequestLine(data, length); } var pathBuffer = new Span(data + pathStart, offset - pathStart); - var queryStart = offset; // Query string + var queryStart = offset; if (ch == ByteQuestionMark) { // We have a query string @@ -138,6 +138,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } } + // End of query string not found + if (offset == length) + { + RejectRequestLine(data, length); + } + var targetBuffer = new Span(data + pathStart, offset - pathStart); var query = new Span(data + queryStart, offset - queryStart); @@ -148,10 +154,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var httpVersion = HttpUtilities.GetKnownVersion(data + offset, length - offset); if (httpVersion == HttpVersion.Unknown) { - RejectUnknownVersion(data, length, offset); + if (data[offset] == ByteCR || data[length - 2] != ByteCR) + { + // If missing delimiter or CR before LF, reject and log entire line + RejectRequestLine(data, length); + } + else + { + // else inform HTTP version is unsupported. + RejectUnknownVersion(data + offset, length - offset - 2); + } } - // After version 8 bytes and cr 1 byte, expect lf + // After version's 8 bytes and CR, expect LF if (data[offset + 8 + 1] != ByteLF) { RejectRequestLine(data, length); @@ -159,7 +174,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var line = new Span(data, length); - handler.OnStartLine(method, httpVersion, targetBuffer, pathBuffer, query, customMethod, line, pathEncoded); + 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 @@ -482,73 +497,24 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http c == '~'; } - public static void RejectRequest(RequestRejectionReason reason) - { - throw BadHttpRequestException.GetException(reason); - } + private void RejectRequest(RequestRejectionReason reason) + => throw BadHttpRequestException.GetException(reason); - private unsafe void RejectUnknownVersion(byte* data, int length, int versionStart) - { - throw GetRejectUnknownVersion(data, length, versionStart); - } - - private unsafe void RejectRequestLine(byte* data, int length) - { - throw GetRejectRequestLineException(new Span(data, length)); - } - - private void RejectRequestLine(Span span) - { - throw GetRejectRequestLineException(span); - } - - private BadHttpRequestException GetRejectRequestLineException(Span span) - { - const int MaxRequestLineError = 32; - return BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine, - 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 RejectRequestLine(byte* requestLine, int length) + => throw GetInvalidRequestException(RequestRejectionReason.InvalidRequestLine, requestLine, length); private unsafe void RejectRequestHeader(byte* headerLine, int length) - { - RejectRequestHeader(new Span(headerLine, length)); - } + => throw GetInvalidRequestException(RequestRejectionReason.InvalidRequestHeader, headerLine, length); - private void RejectRequestHeader(Span span) - { - throw GetRejectRequestHeaderException(span); - } + private unsafe void RejectUnknownVersion(byte* version, int length) + => throw GetInvalidRequestException(RequestRejectionReason.UnrecognizedHTTPVersion, version, length); - private BadHttpRequestException GetRejectRequestHeaderException(Span span) - { - const int MaxRequestHeaderError = 128; - return BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestHeader, - Log.IsEnabled(LogLevel.Information) ? span.GetAsciiStringEscaped(MaxRequestHeaderError) : string.Empty); - } + private unsafe BadHttpRequestException GetInvalidRequestException(RequestRejectionReason reason, byte* detail, int length) + => BadHttpRequestException.GetException( + reason, + Log.IsEnabled(LogLevel.Information) + ? new Span(detail, length).GetAsciiStringEscaped(Constants.MaxExceptionDetailSize) + : string.Empty); public void Reset() { diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs index ce68141b93..33aec0fc67 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs @@ -16,6 +16,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http BadChunkSuffix, BadChunkSizeData, ChunkedRequestIncomplete, + InvalidRequestTarget, InvalidCharactersInHeaderName, RequestLineTooLong, HeadersExceedMaxTotalSize, diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/Constants.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/Constants.cs index 2c201fea7c..0b7a9eaaa2 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/Constants.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/Constants.cs @@ -10,6 +10,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure { public const int ListenBacklog = 128; + public const int MaxExceptionDetailSize = 128; + public const int EOF = -4095; public static readonly int? ECONNRESET = GetECONNRESET(); public static readonly int? EADDRINUSE = GetEADDRINUSE(); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs index 3def317d96..32481ef1b1 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs @@ -126,8 +126,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure { var sb = new StringBuilder(); - int i; - for (i = 0; i < Math.Min(span.Length, maxChars); ++i) + for (var i = 0; i < Math.Min(span.Length, maxChars); i++) { var ch = span[i]; sb.Append(ch < 0x20 || ch >= 0x7F ? $"\\x{ch:X2}" : ((char)ch).ToString()); @@ -137,6 +136,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure { sb.Append("..."); } + return sb.ToString(); } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs index 3d3711cad6..d2f09dd2b9 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs @@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests return TestBadRequest( $"GET / {httpVersion}\r\n", "505 HTTP Version Not Supported", - $"Unrecognized HTTP version: {httpVersion}"); + $"Unrecognized HTTP version: '{httpVersion}'"); } [Theory] @@ -149,27 +149,19 @@ 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: '{Escape(requestLine)}'"); + data.Add(requestLine, $"Invalid request line: '{requestLine.EscapeNonPrintable()}'"); } - foreach (var requestLine in HttpParsingData.RequestLineWithEncodedNullCharInTargetData) + foreach (var target in HttpParsingData.TargetWithEncodedNullCharData) { - data.Add(requestLine, "Invalid request line."); + data.Add($"GET {target} HTTP/1.1\r\n", $"Invalid request target: '{target.EscapeNonPrintable()}'"); } - foreach (var requestLine in HttpParsingData.RequestLineWithNullCharInTargetData) + foreach (var target in HttpParsingData.TargetWithNullCharData) { - data.Add(requestLine, $"Invalid request line."); + data.Add($"GET {target} HTTP/1.1\r\n", $"Invalid request target: '{target.EscapeNonPrintable()}'"); } return data; diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs index 3c4276b619..aa3bdf84eb 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs @@ -126,7 +126,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance new Span(_target), Span.Empty, Span.Empty, - new Span(_startLine), false); consumed = buffer.Start; diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs index e49b613a45..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, Span line, bool pathEncoded) + 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/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index ed28c1d86b..7c8fd7bc62 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -18,6 +18,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Internal; +using Microsoft.Extensions.Logging; using Moq; using Xunit; @@ -302,37 +303,39 @@ namespace Microsoft.AspNetCore.Server.KestrelTests } [Theory] - [MemberData(nameof(RequestLineWithEncodedNullCharInTargetData))] - public async Task TakeStartLineThrowsOnEncodedNullCharInTarget(string requestLine) + [MemberData(nameof(TargetWithEncodedNullCharData))] + public async Task TakeStartLineThrowsOnEncodedNullCharInTarget(string target) { - await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes(requestLine)); + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes($"GET {target} HTTP/1.1\r\n")); var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; - var exception = Assert.Throws(() => + var exception = Assert.Throws(() => _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); _input.Reader.Advance(_consumed, _examined); - Assert.Equal("The path contains null characters.", exception.Message); + Assert.Equal($"Invalid request target: '{target}'", exception.Message); } [Theory] - [MemberData(nameof(RequestLineWithNullCharInTargetData))] - public async Task TakeStartLineThrowsOnNullCharInTarget(string requestLine) + [MemberData(nameof(TargetWithNullCharData))] + public async Task TakeStartLineThrowsOnNullCharInTarget(string target) { - await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes(requestLine)); + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes($"GET {target} HTTP/1.1\r\n")); var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; - var exception = Assert.Throws(() => + var exception = Assert.Throws(() => _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); _input.Reader.Advance(_consumed, _examined); - Assert.Equal(new InvalidOperationException().Message, exception.Message); + Assert.Equal($"Invalid request target: '{target.EscapeNonPrintable()}'", exception.Message); } [Theory] - [MemberData(nameof(RequestLineWithInvalidRequestTargetData))] - public async Task TakeStartLineThrowsWhenRequestTargetIsInvalid(string requestLine) + [MemberData(nameof(MethodWithNullCharData))] + public async Task TakeStartLineThrowsOnNullCharInMethod(string method) { + var requestLine = $"{method} / HTTP/1.1\r\n"; + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes(requestLine)); var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; @@ -340,9 +343,40 @@ namespace Microsoft.AspNetCore.Server.KestrelTests _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); _input.Reader.Advance(_consumed, _examined); - Assert.Equal($"Invalid request line: '{Escape(requestLine)}'", exception.Message); + Assert.Equal($"Invalid request line: '{requestLine.EscapeNonPrintable()}'", exception.Message); } + [Theory] + [MemberData(nameof(QueryStringWithNullCharData))] + public async Task TakeStartLineThrowsOnNullCharInQueryString(string queryString) + { + var target = $"/{queryString}"; + + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes($"GET {target} HTTP/1.1\r\n")); + 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 target: '{target.EscapeNonPrintable()}'", exception.Message); + } + + [Theory] + [MemberData(nameof(TargetInvalidData))] + public async Task TakeStartLineThrowsWhenRequestTargetIsInvalid(string method, string target) + { + var requestLine = $"{method} {target} HTTP/1.1\r\n"; + + 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 target: '{target.EscapeNonPrintable()}'", exception.Message); + } [Theory] [MemberData(nameof(MethodNotAllowedTargetData))] @@ -531,51 +565,98 @@ namespace Microsoft.AspNetCore.Server.KestrelTests Assert.NotSame(original, _frame.RequestAborted.WaitHandle); } + [Fact] + public async Task ExceptionDetailNotIncludedWhenLogLevelInformationNotEnabled() + { + var previousLog = _serviceContext.Log; + + try + { + var mockTrace = new Mock(); + mockTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(false); + + _serviceContext.Log = mockTrace.Object; + + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes($"GET /%00 HTTP/1.1\r\n")); + 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 target: ''", exception.Message); + Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); + } + finally + { + _serviceContext.Log = previousLog; + } + } + public static IEnumerable ValidRequestLineData => HttpParsingData.RequestLineValidData; - public static TheoryData RequestLineWithEncodedNullCharInTargetData + public static TheoryData TargetWithEncodedNullCharData { get { var data = new TheoryData(); - foreach (var requestLine in HttpParsingData.RequestLineWithEncodedNullCharInTargetData) + foreach (var target in HttpParsingData.TargetWithEncodedNullCharData) { - data.Add(requestLine); + data.Add(target); } return data; } } - 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 TargetInvalidData + => HttpParsingData.TargetInvalidData; public static TheoryData MethodNotAllowedTargetData => HttpParsingData.MethodNotAllowedRequestLine; - public static TheoryData RequestLineWithNullCharInTargetData + public static TheoryData TargetWithNullCharData { get { var data = new TheoryData(); - foreach (var requestLine in HttpParsingData.RequestLineWithNullCharInTargetData) + foreach (var target in HttpParsingData.TargetWithNullCharData) { - data.Add(requestLine); + data.Add(target); + } + + return data; + } + } + + public static TheoryData MethodWithNullCharData + { + get + { + var data = new TheoryData(); + + foreach (var target in HttpParsingData.MethodWithNullCharData) + { + data.Add(target); + } + + return data; + } + } + + public static TheoryData QueryStringWithNullCharData + { + get + { + var data = new TheoryData(); + + foreach (var target in HttpParsingData.QueryStringWithNullCharData) + { + data.Add(target); } return data; diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs index bd27800679..41b5af4a55 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs @@ -19,9 +19,6 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { public class HttpParserTests { - // Returns true when all headers parsed - // Return false otherwise - [Theory] [MemberData(nameof(RequestLineValidData))] public void ParsesRequestLine( @@ -50,9 +47,8 @@ namespace Microsoft.AspNetCore.Server.KestrelTests It.IsAny>(), It.IsAny>(), It.IsAny>(), - It.IsAny>(), It.IsAny())) - .Callback, Span, Span, Span, Span, bool>((method, version, target, path, query, customMethod, line, pathEncoded) => + .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); @@ -111,7 +107,28 @@ 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.EscapeNonPrintable()}'", exception.Message); + Assert.Equal(StatusCodes.Status400BadRequest, (exception as BadHttpRequestException).StatusCode); + } + + [Theory] + [MemberData(nameof(MethodWithNonTokenCharData))] + public void ParseRequestLineThrowsOnNonTokenCharsInCustomMethod(string method) + { + var requestLine = $"{method} / HTTP/1.1\r\n"; + + 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(requestLine)); + + var exception = Assert.Throws(() => + parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); + + Assert.Equal($"Invalid request line: '{method.EscapeNonPrintable()} / HTTP/1.1\\x0D\\x0A'", exception.Message); Assert.Equal(StatusCodes.Status400BadRequest, (exception as BadHttpRequestException).StatusCode); } @@ -132,7 +149,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var exception = Assert.Throws(() => parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); - Assert.Equal($"Unrecognized HTTP version: {httpVersion}", exception.Message); + Assert.Equal($"Unrecognized HTTP version: '{httpVersion}'", exception.Message); Assert.Equal(StatusCodes.Status505HttpVersionNotsupported, (exception as BadHttpRequestException).StatusCode); } @@ -324,6 +341,44 @@ namespace Microsoft.AspNetCore.Server.KestrelTests Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); } + [Fact] + public void ExceptionDetailNotIncludedWhenLogLevelInformationNotEnabled() + { + var mockTrace = new Mock(); + mockTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(false); + + var parser = CreateParser(mockTrace.Object); + + // Invalid request line + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes("GET % HTTP/1.1\r\n")); + + var exception = Assert.Throws(() => + parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); + + Assert.Equal("Invalid request line: ''", exception.Message); + Assert.Equal(StatusCodes.Status400BadRequest, (exception as BadHttpRequestException).StatusCode); + + // Unrecognized HTTP version + buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes("GET / HTTP/1.2\r\n")); + + exception = Assert.Throws(() => + parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); + + Assert.Equal("Unrecognized HTTP version: ''", exception.Message); + Assert.Equal(StatusCodes.Status505HttpVersionNotsupported, (exception as BadHttpRequestException).StatusCode); + + // Invalid request header + buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes("Header: value\n\r\n")); + + exception = Assert.Throws(() => + parser.ParseHeaders(Mock.Of(), buffer, out var consumed, out var examined, out var consumedBytes)); + + Assert.Equal("Invalid request header: ''", exception.Message); + Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); + } + private void VerifyHeader( string headerName, string rawHeaderValue, @@ -384,6 +439,8 @@ namespace Microsoft.AspNetCore.Server.KestrelTests public static IEnumerable RequestLineInvalidData => HttpParsingData.RequestLineInvalidData.Select(requestLine => new[] { requestLine }); + public static IEnumerable MethodWithNonTokenCharData => HttpParsingData.MethodWithNonTokenCharData.Select(method => new[] { method }); + public static TheoryData UnrecognizedHttpVersionData => HttpParsingData.UnrecognizedHttpVersionData; public static IEnumerable RequestHeaderInvalidData => HttpParsingData.RequestHeaderInvalidData; diff --git a/test/shared/HttpParsingData.cs b/test/shared/HttpParsingData.cs index 9e5df5e426..f3bd39fcbe 100644 --- a/test/shared/HttpParsingData.cs +++ b/test/shared/HttpParsingData.cs @@ -112,113 +112,144 @@ namespace Microsoft.AspNetCore.Testing "GET / HTTP/1.1\r", }; - public static IEnumerable RequestLineInvalidData => new[] + public static IEnumerable RequestLineInvalidData + { + get + { + return new[] + { + "G\r\n", + "GE\r\n", + "GET\r\n", + "GET \r\n", + "GET /\r\n", + "GET / \r\n", + "GET/HTTP/1.1\r\n", + "GET /HTTP/1.1\r\n", + " \r\n", + " \r\n", + "/ HTTP/1.1\r\n", + " / HTTP/1.1\r\n", + "/ \r\n", + "GET \r\n", + "GET HTTP/1.0\r\n", + "GET HTTP/1.1\r\n", + "GET / \n", + "GET / HTTP/1.0\n", + "GET / HTTP/1.1\n", + "GET / HTTP/1.0\rA\n", + "GET / HTTP/1.1\ra\n", + "GET? / HTTP/1.1\r\n", + "GET ? HTTP/1.1\r\n", + "GET /a?b=cHTTP/1.1\r\n", + "GET /a%20bHTTP/1.1\r\n", + "GET /a%20b?c=dHTTP/1.1\r\n", + "GET %2F HTTP/1.1\r\n", + "GET %00 HTTP/1.1\r\n", + "CUSTOM \r\n", + "CUSTOM /\r\n", + "CUSTOM / \r\n", + "CUSTOM /HTTP/1.1\r\n", + "CUSTOM \r\n", + "CUSTOM HTTP/1.0\r\n", + "CUSTOM HTTP/1.1\r\n", + "CUSTOM / \n", + "CUSTOM / HTTP/1.0\n", + "CUSTOM / HTTP/1.1\n", + "CUSTOM / HTTP/1.0\rA\n", + "CUSTOM / HTTP/1.1\ra\n", + "CUSTOM ? HTTP/1.1\r\n", + "CUSTOM /a?b=cHTTP/1.1\r\n", + "CUSTOM /a%20bHTTP/1.1\r\n", + "CUSTOM /a%20b?c=dHTTP/1.1\r\n", + "CUSTOM %2F HTTP/1.1\r\n", + "CUSTOM %00 HTTP/1.1\r\n", + }.Concat(MethodWithNonTokenCharData.Select(method => $"{method} / HTTP/1.0\r\n")); + } + } + + // Bad HTTP Methods (invalid according to RFC) + public static IEnumerable MethodWithNonTokenCharData + { + get + { + return new[] + { + "(", + ")", + "<", + ">", + "@", + ",", + ";", + ":", + "\\", + "\"", + "/", + "[", + "]", + "?", + "=", + "{", + "}", + "get@", + "post=", + }.Concat(MethodWithNullCharData); + } + } + + public static IEnumerable MethodWithNullCharData => new[] { - "G\r\n", - "GE\r\n", - "GET\r\n", - "GET \r\n", - "GET /\r\n", - "GET / \r\n", - "GET/HTTP/1.1\r\n", - "GET /HTTP/1.1\r\n", - " \r\n", - " \r\n", - "/ HTTP/1.1\r\n", - " / HTTP/1.1\r\n", - "/ \r\n", - "GET \r\n", - "GET HTTP/1.0\r\n", - "GET HTTP/1.1\r\n", - "GET / \n", - "GET / HTTP/1.0\n", - "GET / HTTP/1.1\n", - "GET / HTTP/1.0\rA\n", - "GET / HTTP/1.1\ra\n", - "GET? / HTTP/1.1\r\n", - "GET ? HTTP/1.1\r\n", - "GET /a?b=cHTTP/1.1\r\n", - "GET /a%20bHTTP/1.1\r\n", - "GET /a%20b?c=dHTTP/1.1\r\n", - "GET %2F HTTP/1.1\r\n", - "GET %00 HTTP/1.1\r\n", - "CUSTOM \r\n", - "CUSTOM /\r\n", - "CUSTOM / \r\n", - "CUSTOM /HTTP/1.1\r\n", - "CUSTOM \r\n", - "CUSTOM HTTP/1.0\r\n", - "CUSTOM HTTP/1.1\r\n", - "CUSTOM / \n", - "CUSTOM / HTTP/1.0\n", - "CUSTOM / HTTP/1.1\n", - "CUSTOM / HTTP/1.0\rA\n", - "CUSTOM / HTTP/1.1\ra\n", - "CUSTOM ? HTTP/1.1\r\n", - "CUSTOM /a?b=cHTTP/1.1\r\n", - "CUSTOM /a%20bHTTP/1.1\r\n", - "CUSTOM /a%20b?c=dHTTP/1.1\r\n", - "CUSTOM %2F HTTP/1.1\r\n", - "CUSTOM %00 HTTP/1.1\r\n", // Bad HTTP Methods (invalid according to RFC) - "( / HTTP/1.0\r\n", - ") / HTTP/1.0\r\n", - "< / HTTP/1.0\r\n", - "> / HTTP/1.0\r\n", - "@ / HTTP/1.0\r\n", - ", / HTTP/1.0\r\n", - "; / HTTP/1.0\r\n", - ": / HTTP/1.0\r\n", - "\\ / HTTP/1.0\r\n", - "\" / HTTP/1.0\r\n", - "/ / HTTP/1.0\r\n", - "[ / HTTP/1.0\r\n", - "] / HTTP/1.0\r\n", - "? / HTTP/1.0\r\n", - "= / HTTP/1.0\r\n", - "{ / HTTP/1.0\r\n", - "} / HTTP/1.0\r\n", - "get@ / HTTP/1.0\r\n", - "post= / HTTP/1.0\r\n", + "\0", + "\0GET", + "G\0T", + "GET\0", }; - public static IEnumerable RequestLineWithEncodedNullCharInTargetData => new[] + public static IEnumerable TargetWithEncodedNullCharData => new[] { - "GET /%00 HTTP/1.1\r\n", - "GET /%00%00 HTTP/1.1\r\n", - "GET /%E8%00%84 HTTP/1.1\r\n", - "GET /%E8%85%00 HTTP/1.1\r\n", - "GET /%F3%00%82%86 HTTP/1.1\r\n", - "GET /%F3%85%00%82 HTTP/1.1\r\n", - "GET /%F3%85%82%00 HTTP/1.1\r\n", - "GET /%E8%01%00 HTTP/1.1\r\n", + "/%00", + "/%00%00", + "/%E8%00%84", + "/%E8%85%00", + "/%F3%00%82%86", + "/%F3%85%00%82", + "/%F3%85%82%00", }; - public static TheoryData RequestLineWithInvalidRequestTarget => new TheoryData + public static TheoryData TargetInvalidData { - // 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", - }; + get + { + var data = new TheoryData(); + + // Invalid absolute-form + data.Add("GET", "http://"); + data.Add("GET", "http:/"); + data.Add("GET", "https:/"); + data.Add("GET", "http:///"); + data.Add("GET", "https://"); + data.Add("GET", "http:////"); + data.Add("GET", "http://:80"); + data.Add("GET", "http://:80/abc"); + data.Add("GET", "http://user@"); + data.Add("GET", "http://user@/abc"); + data.Add("GET", "http://abc%20xyz/abc"); + data.Add("GET", "http://%20/abc?query=%0A"); + // Valid absolute-form but with unsupported schemes + data.Add("GET", "otherscheme://host/"); + data.Add("GET", "ws://host/"); + data.Add("GET", "wss://host/"); + // Must only have one asterisk + data.Add("OPTIONS", "**"); + // Relative form + data.Add("GET", "../../"); + data.Add("GET", "..\\."); + + return data; + } + } public static TheoryData MethodNotAllowedRequestLine { @@ -234,36 +265,44 @@ namespace Microsoft.AspNetCore.Testing "TRACE", "PATCH", "CONNECT", - //"OPTIONS", + "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), - })) + var data = new TheoryData(); + + foreach (var method in methods.Except(new[] { "OPTIONS" })) { - theoryData.Add(line.Item1, line.Item2); + data.Add($"{method} * HTTP/1.1\r\n", HttpMethod.Options); } - return theoryData; + foreach (var method in methods.Except(new[] { "CONNECT" })) + { + data.Add($"{method} www.example.com:80 HTTP/1.1\r\n", HttpMethod.Connect); + } + + return data; } } - public static IEnumerable RequestLineWithNullCharInTargetData => new[] + public static IEnumerable TargetWithNullCharData { - // 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", + get + { + return new[] + { + "\0", + "/\0", + "/\0\0", + "/%C8\0", + }.Concat(QueryStringWithNullCharData); + } + } + + public static IEnumerable QueryStringWithNullCharData => new[] + { + "/?\0=a", + "/?a=\0", }; public static TheoryData UnrecognizedHttpVersionData => new TheoryData diff --git a/test/shared/StringExtensions.cs b/test/shared/StringExtensions.cs new file mode 100644 index 0000000000..5d1756b55b --- /dev/null +++ b/test/shared/StringExtensions.cs @@ -0,0 +1,22 @@ +// 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 System; + +namespace Microsoft.AspNetCore.Testing +{ + public static class StringExtensions + { + public static string EscapeNonPrintable(this string s) + { + var ellipsis = s.Length > 128 + ? "..." + : string.Empty; + return s.Substring(0, Math.Min(128, s.Length)) + .Replace("\r", @"\x0D") + .Replace("\n", @"\x0A") + .Replace("\0", @"\x00") + + ellipsis; + } + } +} \ No newline at end of file