Always call ConsumingComplete() with furthest examined position (#1095).

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

@@ -789,10 +789,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
             SocketOutput.ProducingComplete(end);
         }
 
-        protected RequestLineStatus TakeStartLine(SocketInput input)
+        public RequestLineStatus TakeStartLine(SocketInput input)
         {
             var scan = input.ConsumingStart();
             var consumed = scan;
+            var end = scan;
 
             try
             {
@@ -806,7 +807,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
 
                 _requestProcessingStatus = RequestProcessingStatus.RequestStarted;
 
-                var end = scan;
                 int bytesScanned;
                 if (end.Seek(ref _vectorLFs, out bytesScanned, ServerOptions.Limits.MaxRequestLineSize) == -1)
                 {
@@ -923,6 +923,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                     RejectRequest(RequestRejectionReason.MissingLFInRequestLine);
                 }
                 scan.Take(); // consume LF
+                end = scan;
 
                 // 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;
@@ -984,7 +985,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
             }
             finally
             {
-                input.ConsumingComplete(consumed, scan);
+                input.ConsumingComplete(consumed, end);
             }
         }
 
@@ -1049,11 +1050,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
         {
             var scan = input.ConsumingStart();
             var consumed = scan;
+            var end = scan;
             try
             {
-                while (!scan.IsEnd)
+                while (!end.IsEnd)
                 {
-                    var ch = scan.Peek();
+                    var ch = end.Peek();
                     if (ch == -1)
                     {
                         return false;
@@ -1061,8 +1063,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                     else if (ch == '\r')
                     {
                         // Check for final CRLF.
-                        scan.Take();
+                        end.Take();
-                        ch = scan.Take();
+                        ch = end.Take();
 
                         if (ch == -1)
                         {
@@ -1070,7 +1072,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                         }
                         else if (ch == '\n')
                         {
-                            consumed = scan;
+                            consumed = end;
                             return true;
                         }
 
@@ -1089,7 +1091,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                         RejectRequest(RequestRejectionReason.TooManyHeaders);
                     }
 
-                    var end = scan;
                     int bytesScanned;
                     if (end.Seek(ref _vectorLFs, out bytesScanned, _remainingRequestHeadersBytesAllowed) == -1)
                     {
@@ -1135,6 +1136,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
 
                     scan.Take(); // we know this is '\r'
                     ch = scan.Take(); // expecting '\n'
+                    end = scan;
 
                     if (ch != '\n')
                     {
@@ -1203,7 +1205,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
             }
             finally
             {
-                input.ConsumingComplete(consumed, scan);
+                input.ConsumingComplete(consumed, end);
             }
         }
 
@@ -1286,12 +1288,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
             _secondsSinceLastRequest = 0;
         }
 
-        protected enum RequestLineStatus
+        public enum RequestLineStatus
         {
             Empty,
-            MethodIncomplete,
-            TargetIncomplete,
-            VersionIncomplete,
             Incomplete,
             Done
         }

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

@@ -713,5 +713,156 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
             Assert.Same(originalResponseBody, frame.ResponseBody);
             Assert.Same(originalDuplexStream, frame.DuplexStream);
         }
+
+        [Fact]
+        public void TakeStartLineCallsConsumingCompleteWithFurthestExamined()
+        {
+            var trace = new KestrelTrace(new TestKestrelTrace());
+            var ltp = new LoggingThreadPool(trace);
+            using (var pool = new MemoryPool())
+            using (var socketInput = new SocketInput(pool, ltp))
+            {
+                var connectionContext = new ConnectionContext()
+                {
+                    DateHeaderValueManager = new DateHeaderValueManager(),
+                    ServerAddress = ServerAddress.FromUrl("http://localhost:5000"),
+                    ServerOptions = new KestrelServerOptions(),
+                    Log = trace
+                };
+                var frame = new Frame<object>(application: null, context: connectionContext);
+                frame.Reset();
+
+                var requestLineBytes = Encoding.ASCII.GetBytes("GET / ");
+                socketInput.IncomingData(requestLineBytes, 0, requestLineBytes.Length);
+                frame.TakeStartLine(socketInput);
+                Assert.False(socketInput.IsCompleted);
+
+                requestLineBytes = Encoding.ASCII.GetBytes("HTTP/1.1\r\n");
+                socketInput.IncomingData(requestLineBytes, 0, requestLineBytes.Length);
+                frame.TakeStartLine(socketInput);
+                Assert.False(socketInput.IsCompleted);
+            }
+        }
+
+        [Theory]
+        [InlineData("", Frame.RequestLineStatus.Empty)]
+        [InlineData("G", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GE", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET ", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET /", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / ", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / H", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / HT", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / HTT", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / HTTP", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / HTTP/", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / HTTP/1", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / HTTP/1.", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / HTTP/1.1", Frame.RequestLineStatus.Incomplete)]
+        [InlineData("GET / HTTP/1.1\r", Frame.RequestLineStatus.Incomplete)]
+        public void TakeStartLineReturnsWhenGivenIncompleteRequestLines(string requestLine, Frame.RequestLineStatus expectedReturnValue)
+        {
+            var trace = new KestrelTrace(new TestKestrelTrace());
+            var ltp = new LoggingThreadPool(trace);
+            using (var pool = new MemoryPool())
+            using (var socketInput = new SocketInput(pool, ltp))
+            {
+                var connectionContext = new ConnectionContext()
+                {
+                    DateHeaderValueManager = new DateHeaderValueManager(),
+                    ServerAddress = ServerAddress.FromUrl("http://localhost:5000"),
+                    ServerOptions = new KestrelServerOptions(),
+                    Log = trace
+                };
+                var frame = new Frame<object>(application: null, context: connectionContext);
+                frame.Reset();
+
+                var requestLineBytes = Encoding.ASCII.GetBytes(requestLine);
+                socketInput.IncomingData(requestLineBytes, 0, requestLineBytes.Length);
+
+                var returnValue = frame.TakeStartLine(socketInput);
+                Assert.Equal(expectedReturnValue, returnValue);
+            }
+        }
+
+        [Fact]
+        public void TakeMessageHeadersCallsConsumingCompleteWithFurthestExamined()
+        {
+            var trace = new KestrelTrace(new TestKestrelTrace());
+            var ltp = new LoggingThreadPool(trace);
+            using (var pool = new MemoryPool())
+            using (var socketInput = new SocketInput(pool, ltp))
+            {
+                var connectionContext = new ConnectionContext()
+                {
+                    DateHeaderValueManager = new DateHeaderValueManager(),
+                    ServerAddress = ServerAddress.FromUrl("http://localhost:5000"),
+                    ServerOptions = new KestrelServerOptions(),
+                    Log = trace
+                };
+                var frame = new Frame<object>(application: null, context: connectionContext);
+                frame.Reset();
+                frame.InitializeHeaders();
+
+                var headersBytes = Encoding.ASCII.GetBytes("Header: ");
+                socketInput.IncomingData(headersBytes, 0, headersBytes.Length);
+                frame.TakeMessageHeaders(socketInput, (FrameRequestHeaders)frame.RequestHeaders);
+                Assert.False(socketInput.IsCompleted);
+
+                headersBytes = Encoding.ASCII.GetBytes("value\r\n");
+                socketInput.IncomingData(headersBytes, 0, headersBytes.Length);
+                frame.TakeMessageHeaders(socketInput, (FrameRequestHeaders)frame.RequestHeaders);
+                Assert.False(socketInput.IsCompleted);
+
+                headersBytes = Encoding.ASCII.GetBytes("\r\n");
+                socketInput.IncomingData(headersBytes, 0, headersBytes.Length);
+                frame.TakeMessageHeaders(socketInput, (FrameRequestHeaders)frame.RequestHeaders);
+                Assert.False(socketInput.IsCompleted);
+            }
+        }
+
+        [Theory]
+        [InlineData("\r")]
+        [InlineData("H")]
+        [InlineData("He")]
+        [InlineData("Hea")]
+        [InlineData("Head")]
+        [InlineData("Heade")]
+        [InlineData("Header")]
+        [InlineData("Header:")]
+        [InlineData("Header: ")]
+        [InlineData("Header: v")]
+        [InlineData("Header: va")]
+        [InlineData("Header: val")]
+        [InlineData("Header: valu")]
+        [InlineData("Header: value")]
+        [InlineData("Header: value\r")]
+        [InlineData("Header: value\r\n")]
+        [InlineData("Header: value\r\n\r")]
+        public void TakeMessageHeadersReturnsWhenGivenIncompleteHeaders(string headers)
+        {
+            var trace = new KestrelTrace(new TestKestrelTrace());
+            var ltp = new LoggingThreadPool(trace);
+            using (var pool = new MemoryPool())
+            using (var socketInput = new SocketInput(pool, ltp))
+            {
+                var connectionContext = new ConnectionContext()
+                {
+                    DateHeaderValueManager = new DateHeaderValueManager(),
+                    ServerAddress = ServerAddress.FromUrl("http://localhost:5000"),
+                    ServerOptions = new KestrelServerOptions(),
+                    Log = trace
+                };
+                var frame = new Frame<object>(application: null, context: connectionContext);
+                frame.Reset();
+                frame.InitializeHeaders();
+
+                var headerBytes = Encoding.ASCII.GetBytes(headers);
+                socketInput.IncomingData(headerBytes, 0, headerBytes.Length);
+
+                Assert.Equal(false, frame.TakeMessageHeaders(socketInput, (FrameRequestHeaders)frame.RequestHeaders));
+            }
+        }
     }
 }