Log rejected request targets.

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

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);

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)
+        public void RejectRequest(RequestRejectionReason reason, string detail)
-        {
+            => throw BadHttpRequestException.GetException(reason, detail);
-            throw BadHttpRequestException.GetException(reason, value);
-        }
 
-        private void RejectRequestLine(Span<byte> requestLine)
+        private void RejectRequestTarget(Span<byte> target)
-        {
+            => throw GetInvalidRequestTargetException(target);
-            Debug.Assert(Log.IsEnabled(LogLevel.Information) == true, "Use RejectRequest instead to improve inlining when log is disabled");
 
-            const int MaxRequestLineError = 32;
+        private BadHttpRequestException GetInvalidRequestTargetException(Span<byte> target)
-            var line = requestLine.GetAsciiStringEscaped(MaxRequestLineError);
+            => BadHttpRequestException.GetException(
-            throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine, line);
+                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<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, Span<byte> line, bool pathEncoded)
+        public void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> 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.
+                // a client mistakenly thinks this server is a proxy server.
-
+                OnAuthorityFormTarget(method, target);
-                OnAuthorityFormTarget(method, target, line);
             }
 
             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 requestUrlPath = null;
-            string rawTarget;
+            string rawTarget = null;
-            if (pathEncoded)
-            {
-                // Read raw target before mutating memory.
-                rawTarget = target.GetAsciiStringNonNullCharacters();
 
-                // URI was encoded, unescape and then parse as utf8
+            try
-                int pathLength = UrlEncoder.Decode(path, path);
-                requestUrlPath = GetUtf8String(path.Slice(0, pathLength));
-            }
-            else
             {
-                // URI wasn't encoded, parse as ASCII
+                if (pathEncoded)
-                requestUrlPath = path.GetAsciiStringNonNullCharacters();
-
-                if (query.Length == 0)
                 {
-                    // No need to allocate an extra string if the path didn't need
+                    // Read raw target before mutating memory.
-                    // decoding and there's no query string following it.
+                    rawTarget = target.GetAsciiStringNonNullCharacters();
-                    rawTarget = requestUrlPath;
+
+                    // 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<byte> target, Span<byte> line)
+        private void OnAuthorityFormTarget(HttpMethod method, Span<byte> 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))
+                    RejectRequestTarget(target);
-                    {
-                        RejectRequestLine(line);
-                    }
-
-                    throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine);
                 }
             }
 
@@ -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
+            // component (excluding any userinfo and its "@" delimiter) as the
-            // request - target.For example,
+            // 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<byte> target, Span<byte> query, Span<byte> line)
+        private void OnAbsoluteFormTarget(Span<byte> target, Span<byte> 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))
+                RejectRequestTarget(target);
-                {
-                    RejectRequestLine(line);
-                }
-
-                throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine);
             }
 
             SetNormalizedPath(uri.LocalPath);

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.MalformedRequestInvalidHeaders);
-                                    throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine);
-                                case RequestProcessingStatus.ParsingHeaders:
-                                    throw BadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders);
                             }
                             throw;
                         }

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<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, Span<byte> line, bool pathEncoded);
+        void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, bool pathEncoded);
     }
 }

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<byte>(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<byte>(data + pathStart, offset - pathStart);
             var query = new Span<byte>(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<byte>(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>(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)
+        private void RejectRequest(RequestRejectionReason reason)
-        {
+            => throw BadHttpRequestException.GetException(reason);
-            throw BadHttpRequestException.GetException(reason);
-        }
 
-        private unsafe void RejectUnknownVersion(byte* data, int length, int versionStart)
+        private unsafe void RejectRequestLine(byte* requestLine, int length)
-        {
+            => throw GetInvalidRequestException(RequestRejectionReason.InvalidRequestLine, requestLine, length);
-            throw GetRejectUnknownVersion(data, length, versionStart);
-        }
-
-        private unsafe void RejectRequestLine(byte* data, int length)
-        {
-            throw GetRejectRequestLineException(new Span<byte>(data, length));
-        }
-
-        private void RejectRequestLine(Span<byte> span)
-        {
-            throw GetRejectRequestLineException(span);
-        }
-
-        private BadHttpRequestException GetRejectRequestLineException(Span<byte> 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<byte>(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)
-        {
+            => throw GetInvalidRequestException(RequestRejectionReason.InvalidRequestHeader, headerLine, length);
-            RejectRequestHeader(new Span<byte>(headerLine, length));
-        }
 
-        private void RejectRequestHeader(Span<byte> span)
+        private unsafe void RejectUnknownVersion(byte* version, int length)
-        {
+            => throw GetInvalidRequestException(RequestRejectionReason.UnrecognizedHTTPVersion, version, length);
-            throw GetRejectRequestHeaderException(span);
-        }
 
-        private BadHttpRequestException GetRejectRequestHeaderException(Span<byte> span)
+        private unsafe BadHttpRequestException GetInvalidRequestException(RequestRejectionReason reason, byte* detail, int length)
-        {
+            => BadHttpRequestException.GetException(
-            const int MaxRequestHeaderError = 128;
+                reason,
-            return BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestHeader,
+                Log.IsEnabled(LogLevel.Information)
-                Log.IsEnabled(LogLevel.Information) ? span.GetAsciiStringEscaped(MaxRequestHeaderError) : string.Empty);
+                    ? new Span<byte>(detail, length).GetAsciiStringEscaped(Constants.MaxExceptionDetailSize)
-        }
+                    : string.Empty);
 
         public void Reset()
         {

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,

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();

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 (var i = 0; i < Math.Min(span.Length, maxChars); i++)
-            for (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();
         }
 

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, string>();
 
-                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;

test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs

@@ -126,7 +126,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance
                     new Span<byte>(_target),
                     Span<byte>.Empty,
                     Span<byte>.Empty,
-                    new Span<byte>(_startLine),
                     false);
 
                 consumed = buffer.Start;

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<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, Span<byte> line, bool pathEncoded)
+        public void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, bool pathEncoded)
         {
         }
 

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))]
+        [MemberData(nameof(TargetWithEncodedNullCharData))]
-        public async Task TakeStartLineThrowsOnEncodedNullCharInTarget(string requestLine)
+        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<InvalidOperationException>(() =>
+            var exception = Assert.Throws<BadHttpRequestException>(() =>
                 _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))]
+        [MemberData(nameof(TargetWithNullCharData))]
-        public async Task TakeStartLineThrowsOnNullCharInTarget(string requestLine)
+        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<InvalidOperationException>(() =>
+            var exception = Assert.Throws<BadHttpRequestException>(() =>
                 _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))]
+        [MemberData(nameof(MethodWithNullCharData))]
-        public async Task TakeStartLineThrowsWhenRequestTargetIsInvalid(string requestLine)
+        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<BadHttpRequestException>(() =>
+                _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<BadHttpRequestException>(() =>
+                _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<IKestrelTrace>();
+                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<BadHttpRequestException>(() =>
+                    _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<object> ValidRequestLineData => HttpParsingData.RequestLineValidData;
 
-        public static TheoryData<string> RequestLineWithEncodedNullCharInTargetData
+        public static TheoryData<string> TargetWithEncodedNullCharData
         {
             get
             {
                 var data = new TheoryData<string>();
 
-                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)
+        public static TheoryData<string, string> TargetInvalidData 
-        {
+            => HttpParsingData.TargetInvalidData;
-            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<string> RequestLineWithInvalidRequestTargetData 
-            => HttpParsingData.RequestLineWithInvalidRequestTarget;
 
         public static TheoryData<string, HttpMethod> MethodNotAllowedTargetData
             => HttpParsingData.MethodNotAllowedRequestLine;
 
-        public static TheoryData<string> RequestLineWithNullCharInTargetData
+        public static TheoryData<string> TargetWithNullCharData
         {
             get
             {
                 var data = new TheoryData<string>();
 
-                foreach (var requestLine in HttpParsingData.RequestLineWithNullCharInTargetData)
+                foreach (var target in HttpParsingData.TargetWithNullCharData)
                 {
-                    data.Add(requestLine);
+                    data.Add(target);
+                }
+
+                return data;
+            }
+        }
+
+        public static TheoryData<string> MethodWithNullCharData
+        {
+            get
+            {
+                var data = new TheoryData<string>();
+
+                foreach (var target in HttpParsingData.MethodWithNullCharData)
+                {
+                    data.Add(target);
+                }
+
+                return data;
+            }
+        }
+
+        public static TheoryData<string> QueryStringWithNullCharData
+        {
+            get
+            {
+                var data = new TheoryData<string>();
+
+                foreach (var target in HttpParsingData.QueryStringWithNullCharData)
+                {
+                    data.Add(target);
                 }
 
                 return data;

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<Span<byte>>(),
                     It.IsAny<Span<byte>>(),
                     It.IsAny<Span<byte>>(),
-                    It.IsAny<Span<byte>>(),
                     It.IsAny<bool>()))
-                .Callback<HttpMethod, HttpVersion, Span<byte>, Span<byte>, Span<byte>, Span<byte>, Span<byte>, bool>((method, version, target, path, query, customMethod, line, pathEncoded) =>
+                .Callback<HttpMethod, HttpVersion, Span<byte>, Span<byte>, Span<byte>, Span<byte>, 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<BadHttpRequestException>(() =>
                 parser.ParseRequestLine(Mock.Of<IHttpRequestLineHandler>(), 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<IKestrelTrace>();
+            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<BadHttpRequestException>(() =>
+                parser.ParseRequestLine(Mock.Of<IHttpRequestLineHandler>(), 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<BadHttpRequestException>(() =>
                 parser.ParseRequestLine(Mock.Of<IHttpRequestLineHandler>(), 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<IKestrelTrace>();
+            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<BadHttpRequestException>(() =>
+                parser.ParseRequestLine(Mock.Of<IHttpRequestLineHandler>(), 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<BadHttpRequestException>(() =>
+                parser.ParseRequestLine(Mock.Of<IHttpRequestLineHandler>(), 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<BadHttpRequestException>(() =>
+                parser.ParseHeaders(Mock.Of<IHttpHeadersHandler>(), 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<object[]> RequestLineInvalidData => HttpParsingData.RequestLineInvalidData.Select(requestLine => new[] { requestLine });
 
+        public static IEnumerable<object[]> MethodWithNonTokenCharData => HttpParsingData.MethodWithNonTokenCharData.Select(method => new[] { method });
+
         public static TheoryData<string> UnrecognizedHttpVersionData => HttpParsingData.UnrecognizedHttpVersionData;
 
         public static IEnumerable<object[]> RequestHeaderInvalidData => HttpParsingData.RequestHeaderInvalidData;

test/shared/HttpParsingData.cs

@@ -112,113 +112,144 @@ namespace Microsoft.AspNetCore.Testing
             "GET / HTTP/1.1\r",
         };
 
-        public static IEnumerable<string> RequestLineInvalidData => new[]
+        public static IEnumerable<string> 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<string> MethodWithNonTokenCharData
+        {
+            get
+            {
+                return new[]
+                {
+                    "(",
+                    ")",
+                    "<",
+                    ">",
+                    "@",
+                    ",",
+                    ";",
+                    ":",
+                    "\\",
+                    "\"",
+                    "/",
+                    "[",
+                    "]",
+                    "?",
+                    "=",
+                    "{",
+                    "}",
+                    "get@",
+                    "post=",
+                }.Concat(MethodWithNullCharData);
+            }
+        }
+
+        public static IEnumerable<string> 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",
+            "\0",
-            ") / HTTP/1.0\r\n",
+            "\0GET",
-            "< / HTTP/1.0\r\n",
+            "G\0T",
-            "> / HTTP/1.0\r\n",
+            "GET\0",
-            "@ / 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",
         };
 
-        public static IEnumerable<string> RequestLineWithEncodedNullCharInTargetData => new[]
+        public static IEnumerable<string> TargetWithEncodedNullCharData => new[]
         {
-            "GET /%00 HTTP/1.1\r\n",
+            "/%00",
-            "GET /%00%00 HTTP/1.1\r\n",
+            "/%00%00",
-            "GET /%E8%00%84 HTTP/1.1\r\n",
+            "/%E8%00%84",
-            "GET /%E8%85%00 HTTP/1.1\r\n",
+            "/%E8%85%00",
-            "GET /%F3%00%82%86 HTTP/1.1\r\n",
+            "/%F3%00%82%86",
-            "GET /%F3%85%00%82 HTTP/1.1\r\n",
+            "/%F3%85%00%82",
-            "GET /%F3%85%82%00 HTTP/1.1\r\n",
+            "/%F3%85%82%00",
-            "GET /%E8%01%00 HTTP/1.1\r\n",
         };
 
-        public static TheoryData<string> RequestLineWithInvalidRequestTarget => new TheoryData<string>
+        public static TheoryData<string, string> TargetInvalidData
         {
-            // Invalid absolute-form requests
+            get
-            "GET http:// HTTP/1.1\r\n",
+            {
-            "GET http:/ HTTP/1.1\r\n",
+                var data = new TheoryData<string, string>();
-            "GET https:/ HTTP/1.1\r\n",
+
-            "GET http:/// HTTP/1.1\r\n",
+                // Invalid absolute-form
-            "GET https:// HTTP/1.1\r\n",
+                data.Add("GET", "http://");
-            "GET http://// HTTP/1.1\r\n",
+                data.Add("GET", "http:/");
-            "GET http://:80 HTTP/1.1\r\n",
+                data.Add("GET", "https:/");
-            "GET http://:80/abc HTTP/1.1\r\n",
+                data.Add("GET", "http:///");
-            "GET http://user@ HTTP/1.1\r\n",
+                data.Add("GET", "https://");
-            "GET http://user@/abc HTTP/1.1\r\n",
+                data.Add("GET", "http:////");
-            "GET http://abc%20xyz/abc HTTP/1.1\r\n",
+                data.Add("GET", "http://:80");
-            "GET http://%20/abc?query=%0A HTTP/1.1\r\n",
+                data.Add("GET", "http://:80/abc");
-            // Valid absolute-form but with unsupported schemes
+                data.Add("GET", "http://user@");
-            "GET otherscheme://host/ HTTP/1.1\r\n",
+                data.Add("GET", "http://user@/abc");
-            "GET ws://host/ HTTP/1.1\r\n",
+                data.Add("GET", "http://abc%20xyz/abc");
-            "GET wss://host/ HTTP/1.1\r\n",
+                data.Add("GET", "http://%20/abc?query=%0A");
-            // Must only have one asterisk
+                // Valid absolute-form but with unsupported schemes
-            "OPTIONS ** HTTP/1.1\r\n",
+                data.Add("GET", "otherscheme://host/");
-            // Relative form
+                data.Add("GET", "ws://host/");
-            "GET ../../ HTTP/1.1\r\n",
+                data.Add("GET", "wss://host/");
-            "GET ..\\. HTTP/1.1\r\n",
+                // Must only have one asterisk
-        };
+                data.Add("OPTIONS", "**");
+                // Relative form
+                data.Add("GET", "../../");
+                data.Add("GET", "..\\.");
+
+                return data;
+            }
+        }
 
         public static TheoryData<string, HttpMethod> MethodNotAllowedRequestLine
         {
@@ -234,36 +265,44 @@ namespace Microsoft.AspNetCore.Testing
                     "TRACE",
                     "PATCH",
                     "CONNECT",
-                    //"OPTIONS",
+                    "OPTIONS",
                     "CUSTOM",
                 };
 
-                var theoryData = new TheoryData<string, HttpMethod>();
+                var data = new TheoryData<string, HttpMethod>();
-                foreach (var line in methods
+
-                    .Select(m => Tuple.Create($"{m} * HTTP/1.1\r\n", HttpMethod.Options))
+                foreach (var method in methods.Except(new[] { "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);
+                    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<string> RequestLineWithNullCharInTargetData => new[]
+        public static IEnumerable<string> TargetWithNullCharData
         {
-            // TODO re-enable after we get both #1469 and #1470 merged
+            get
-            // "GET \0 HTTP/1.1\r\n",
+            {
-            "GET /\0 HTTP/1.1\r\n",
+                return new[]
-            "GET /\0\0 HTTP/1.1\r\n",
+                {
-            "GET /%C8\0 HTTP/1.1\r\n",
+                    "\0",
+                    "/\0",
+                    "/\0\0",
+                    "/%C8\0",
+                }.Concat(QueryStringWithNullCharData);
+            }
+        }
+
+        public static IEnumerable<string> QueryStringWithNullCharData => new[]
+        {
+            "/?\0=a",
+            "/?a=\0",
         };
 
         public static TheoryData<string> UnrecognizedHttpVersionData => new TheoryData<string>

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;
+        }
+    }
+}