From 547ad80a272c94861453ae50daaa5f9addb0cc8f Mon Sep 17 00:00:00 2001
From: David Fowler <davidfowl@gmail.com>
Date: Sun, 2 Apr 2017 00:03:06 -0700
Subject: [PATCH] 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
---
 .../Internal/Http/KestrelHttpParser.cs        | 20 +++++++++++++------
 .../HttpParserTests.cs                        | 15 ++++++++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/KestrelHttpParser.cs
index 581281ca43..ef8d48951c 100644
--- a/src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Http/KestrelHttpParser.cs
+++ b/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);
-                span = buffer.Slice(start, end).ToSpan();
+                found = buffer.Move(found, 1);
                 return true;
             }
             return false;
diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs
index 0f269514ba..99daa1a53d 100644
--- a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs
+++ b/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,