Parser cleanup and remove line continuation header error (#1431)

- Cleaned up some parsing logic (removed locals etc)
- Removing line continuation errors cleaned up code duplication
a little bit

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs

@@ -61,17 +61,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                 end = buffer.Move(end, 1);
                 var startLineBuffer = buffer.Slice(start, end);
 
-                if (startLineBuffer.IsSingleSpan)
+                span = startLineBuffer.ToSpan();
-                {
-                    // No copies, directly use the one and only span
-                    span = startLineBuffer.First.Span;
-                }
-                else
-                {
-                    // We're not a single span here but we can use pooled arrays to avoid allocations in the rare case
-                    span = new Span<byte>(new byte[startLineBuffer.Length]);
-                    startLineBuffer.CopyTo(span);
-                }
             }
 
             var pathStart = -1;
@@ -80,10 +70,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
             var pathEnd = -1;
             var versionStart = -1;
 
-            HttpVersion httpVersion = HttpVersion.Unknown;
+            var httpVersion = HttpVersion.Unknown;
             HttpMethod method;
             Span<byte> customMethod;
-            int i = 0;
+            var i = 0;
             var length = span.Length;
             var done = false;
 
@@ -320,25 +310,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                     return false;
                 }
 
-                if (lineEnd != bufferEnd)
+                // Make sure LF is included in lineEnd
-                {
+                lineEnd = buffer.Move(lineEnd, 1);
-                    lineEnd = buffer.Move(lineEnd, 1);
-                }
 
                 var headerBuffer = buffer.Slice(consumed, lineEnd);
-
+                var span = headerBuffer.ToSpan();
-                Span<byte> span;
-                if (headerBuffer.IsSingleSpan)
-                {
-                    // No copies, directly use the one and only span
-                    span = headerBuffer.First.Span;
-                }
-                else
-                {
-                    // We're not a single span here but we can use pooled arrays to avoid allocations in the rare case
-                    span = new Span<byte>(new byte[headerBuffer.Length]);
-                    headerBuffer.CopyTo(span);
-                }
 
                 if (!TakeSingleHeader(span, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd))
                 {
@@ -347,73 +323,41 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
 
                 // Skip the reader forward past the header line
                 reader.Skip(span.Length);
-
+                consumed = reader.Cursor;
-                // Before accepting the header line, we need to see at least one character
+                consumedBytes += span.Length;
-                // > so we can make sure there's no space or tab
-                var next = reader.Peek();
-
-                // TODO: We don't need to reject the line here, we can use the state machine
-                // to store the fact that we're reading a header value
-                if (next == -1)
-                {
-                    // If we can't see the next char then reject the entire line
-                    return false;
-                }
-
-                if (next == ByteSpace || next == ByteTab)
-                {
-                    // From https://tools.ietf.org/html/rfc7230#section-3.2.4:
-                    //
-                    // Historically, HTTP header field values could be extended over
-                    // multiple lines by preceding each extra line with at least one space
-                    // or horizontal tab (obs-fold).  This specification deprecates such
-                    // line folding except within the message/http media type
-                    // (Section 8.3.1).  A sender MUST NOT generate a message that includes
-                    // line folding (i.e., that has any field-value that contains a match to
-                    // the obs-fold rule) unless the message is intended for packaging
-                    // within the message/http media type.
-                    //
-                    // A server that receives an obs-fold in a request message that is not
-                    // within a message/http container MUST either reject the message by
-                    // sending a 400 (Bad Request), preferably with a representation
-                    // explaining that obsolete line folding is unacceptable, or replace
-                    // each received obs-fold with one or more SP octets prior to
-                    // interpreting the field value or forwarding the message downstream.
-                    RejectRequest(RequestRejectionReason.HeaderValueLineFoldingNotSupported);
-                }
 
                 var nameBuffer = span.Slice(nameStart, nameEnd - nameStart);
                 var valueBuffer = span.Slice(valueStart, valueEnd - valueStart);
-                consumedBytes += span.Length;
 
                 handler.OnHeader(nameBuffer, valueBuffer);
-
-                consumed = reader.Cursor;
             }
         }
 
         private unsafe bool TakeMessageHeadersSingleSpan<T>(T handler, Span<byte> headersSpan, out int consumedBytes) where T : IHttpHeadersHandler
         {
             consumedBytes = 0;
+            var length = headersSpan.Length;
 
-            var remaining = headersSpan.Length;
-            var index = 0;
             fixed (byte* data = &headersSpan.DangerousGetPinnableReference())
             {
-                while (true)
+                while (consumedBytes < length)
                 {
-                    if (remaining < 2)
+                    var ch1 = data[consumedBytes];
-                    {
+                    var ch2 = -1;
-                        return false;
-                    }
 
-                    var ch1 = data[index];
+                    if (consumedBytes + 1 < length)
-                    var ch2 = data[index + 1];
+                    {
+                        ch2 = data[consumedBytes + 1];
+                    }
 
                     if (ch1 == ByteCR)
                     {
                         // Check for final CRLF.
-                        if (ch2 == ByteLF)
+                        if (ch2 == -1)
+                        {
+                            return false;
+                        }
+                        else if (ch2 == ByteLF)
                         {
                             consumedBytes += 2;
                             return true;
@@ -427,61 +371,30 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                         RejectRequest(RequestRejectionReason.HeaderLineMustNotStartWithWhitespace);
                     }
 
-                    var endOfLineIndex = IndexOf(data, index, headersSpan.Length, ByteLF);
+                    var lineEnd = IndexOf(data, consumedBytes, headersSpan.Length, ByteLF);
 
-                    if (endOfLineIndex == -1)
+                    if (lineEnd == -1)
                     {
                         return false;
                     }
 
-                    var span = new Span<byte>(data + index, (endOfLineIndex - index + 1));
+                    var span = new Span<byte>(data + consumedBytes, (lineEnd - consumedBytes + 1));
-                    index += span.Length;
 
                     if (!TakeSingleHeader(span, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd))
                     {
                         return false;
                     }
 
-                    if ((endOfLineIndex + 1) >= headersSpan.Length)
+                    consumedBytes += span.Length;
-                    {
-                        return false;
-                    }
-
-                    // Before accepting the header line, we need to see at least one character
-                    // > so we can make sure there's no space or tab
-                    var next = data[index];
-
-                    if (next == ByteSpace || next == ByteTab)
-                    {
-                        // From https://tools.ietf.org/html/rfc7230#section-3.2.4:
-                        //
-                        // Historically, HTTP header field values could be extended over
-                        // multiple lines by preceding each extra line with at least one space
-                        // or horizontal tab (obs-fold).  This specification deprecates such
-                        // line folding except within the message/http media type
-                        // (Section 8.3.1).  A sender MUST NOT generate a message that includes
-                        // line folding (i.e., that has any field-value that contains a match to
-                        // the obs-fold rule) unless the message is intended for packaging
-                        // within the message/http media type.
-                        //
-                        // A server that receives an obs-fold in a request message that is not
-                        // within a message/http container MUST either reject the message by
-                        // sending a 400 (Bad Request), preferably with a representation
-                        // explaining that obsolete line folding is unacceptable, or replace
-                        // each received obs-fold with one or more SP octets prior to
-                        // interpreting the field value or forwarding the message downstream.
-                        RejectRequest(RequestRejectionReason.HeaderValueLineFoldingNotSupported);
-                    }
 
                     var nameBuffer = span.Slice(nameStart, nameEnd - nameStart);
                     var valueBuffer = span.Slice(valueStart, valueEnd - valueStart);
 
                     handler.OnHeader(nameBuffer, valueBuffer);
-
-                    consumedBytes += span.Length;
-                    remaining -= span.Length;
                 }
             }
+
+            return false;
         }
 
         private unsafe int IndexOf(byte* data, int index, int length, byte value)

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/PipelineExtensions.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.IO.Pipelines;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
@@ -77,6 +78,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
             return result;
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static Span<byte> ToSpan(this ReadableBuffer buffer)
         {
             if (buffer.IsSingleSpan)

test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs

@@ -199,7 +199,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
             var exception = Assert.Throws<BadHttpRequestException>(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined));
             _socketInput.Reader.Advance(_consumed, _examined);
 
-            Assert.Equal("Header value line folding not supported.", exception.Message);
+            Assert.Equal("Header line must not start with whitespace.", exception.Message);
             Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode);
         }
 

test/shared/HttpParsingData.cs

@@ -287,7 +287,7 @@ namespace Microsoft.AspNetCore.Testing
 
                 return new[]
                 {
-                    Tuple.Create(headersWithLineFolding,"Header value line folding not supported."),
+                    Tuple.Create(headersWithLineFolding,"Header line must not start with whitespace."),
                     Tuple.Create(headersWithCRInValue,"Header value must not contain CR characters."),
                     Tuple.Create(headersWithMissingColon,"No ':' character found in header line."),
                     Tuple.Create(headersStartingWithWhitespace, "Header line must not start with whitespace."),