Don't update consumed if we didn't see a new line (#1592)

* Don't update consumed if we didn't see a new line
- The start line parser incorrectly updated the consumed cursor
when it attempted to find new lines across buffers. This change fixes
that and also removes passing the span by reference (which will be illegal).
- Added a unit test

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

@@ -41,7 +41,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                 consumed = buffer.Move(consumed, lineIndex + 1);
                 span = span.Slice(0, lineIndex + 1);
             }
-            else if (buffer.IsSingleSpan || !TryGetNewLineSpan(ref buffer, ref span, out consumed))
+            else if (buffer.IsSingleSpan)
+            {
+                // No request line end
+                return false;
+            }
+            else if (TryGetNewLine(ref buffer, out var found))
+            {
+                span = buffer.Slice(consumed, found).ToSpan();
+                consumed = found;
+            }
+            else
             {
                 // No request line end
                 return false;
@@ -429,16 +439,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
             return true;
         }
 
-
         [MethodImpl(MethodImplOptions.NoInlining)]
-        private static bool TryGetNewLineSpan(ref ReadableBuffer buffer, ref Span<byte> span, out ReadCursor end)
+        private static bool TryGetNewLine(ref ReadableBuffer buffer, out ReadCursor found)
         {
             var start = buffer.Start;
-            if (ReadCursorOperations.Seek(start, buffer.End, out end, ByteLF) != -1)
+            if (ReadCursorOperations.Seek(start, buffer.End, out found, ByteLF) != -1)
             {
                 // Move 1 byte past the \n
-                end = buffer.Move(end, 1);
+                found = buffer.Move(found, 1);
-                span = buffer.Slice(start, end).ToSpan();
                 return true;
             }
             return false;

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

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO.Pipelines;
+using System.IO.Pipelines.Testing;
 using System.Linq;
 using System.Text;
 using Microsoft.AspNetCore.Http;
@@ -365,6 +366,20 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
             Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode);
         }
 
+        [Fact]
+        public void ParseRequestLineSplitBufferWithoutNewLineDoesNotUpdateConsumed()
+        {
+            var parser = CreateParser(Mock.Of<IKestrelTrace>());
+            var buffer = BufferUtilities.CreateBuffer("GET ", "/");
+
+            var requestHandler = new RequestHandler();
+            var result = parser.ParseRequestLine(requestHandler, buffer, out var consumed, out var examined);
+
+            Assert.False(result);
+            Assert.Equal(buffer.Start, consumed);
+            Assert.Equal(buffer.End, examined);
+        }
+
         private void VerifyHeader(
             string headerName,
             string rawHeaderValue,