Cache _absoluteRequestTarget when reusing strings (#18547)

Author: halter73

src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs

@@ -34,6 +34,13 @@ internal partial class Http1Connection : HttpProtocol, IRequestProcessor
         private HttpRequestTarget _requestTargetForm = HttpRequestTarget.Unknown;
         private Uri _absoluteRequestTarget;
 
+        // The _parsed fields cache the Path, QueryString, RawTarget, and/or _absoluteRequestTarget
+        // from the previous request when DisableStringReuse is false.
+        private string _parsedPath = null;
+        private string _parsedQueryString = null;
+        private string _parsedRawTarget = null;
+        private Uri _parsedAbsoluteRequestTarget;
+
         private int _remainingRequestHeadersBytesAllowed;
 
         public Http1Connection(HttpConnectionContext context)
@@ -337,6 +344,7 @@ private void OnOriginFormTarget(bool pathEncoded, Span<byte> target, Span<byte>
                 // Clear parsedData as we won't check it if we come via this path again,
                 // an setting to null is fast as it doesn't need to use a GC write barrier.
                 _parsedRawTarget = _parsedPath = _parsedQueryString = null;
+                _parsedAbsoluteRequestTarget = null;
                 return;
             }
 
@@ -389,6 +397,10 @@ private void OnOriginFormTarget(bool pathEncoded, Span<byte> target, Span<byte>
                     Path = _parsedPath;
                     QueryString = _parsedQueryString;
                 }
+
+                // Clear parsedData for absolute target as we won't check it if we come via this path again,
+                // an setting to null is fast as it doesn't need to use a GC write barrier.
+                _parsedAbsoluteRequestTarget = null;
             }
             catch (InvalidOperationException)
             {
@@ -441,9 +453,10 @@ private void OnAuthorityFormTarget(HttpMethod method, Span<byte> target)
 
             Path = string.Empty;
             QueryString = string.Empty;
-            // Clear parsedData for path and queryString as we won't check it if we come via this path again,
+            // Clear parsedData for path, queryString and absolute target as we won't check it if we come via this path again,
             // an setting to null is fast as it doesn't need to use a GC write barrier.
             _parsedPath = _parsedQueryString = null;
+            _parsedAbsoluteRequestTarget = null;
         }
 
         private void OnAsteriskFormTarget(HttpMethod method)
@@ -463,6 +476,7 @@ private void OnAsteriskFormTarget(HttpMethod method)
             // Clear parsedData as we won't check it if we come via this path again,
             // an setting to null is fast as it doesn't need to use a GC write barrier.
             _parsedRawTarget = _parsedPath = _parsedQueryString = null;
+            _parsedAbsoluteRequestTarget = null;
         }
 
         private void OnAbsoluteFormTarget(Span<byte> target, Span<byte> query)
@@ -497,7 +511,7 @@ private void OnAbsoluteFormTarget(Span<byte> target, Span<byte> query)
                     ThrowRequestTargetRejected(target);
                 }
 
-                _absoluteRequestTarget = uri;
+                _absoluteRequestTarget = _parsedAbsoluteRequestTarget = uri;
                 Path = _parsedPath = uri.LocalPath;
                 // don't use uri.Query because we need the unescaped version
                 previousValue = _parsedQueryString;
@@ -520,6 +534,7 @@ private void OnAbsoluteFormTarget(Span<byte> target, Span<byte> query)
                 RawTarget = _parsedRawTarget;
                 Path = _parsedPath;
                 QueryString = _parsedQueryString;
+                _absoluteRequestTarget = _parsedAbsoluteRequestTarget;
             }
         }
 

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

@@ -134,13 +134,8 @@ public string TraceIdentifier
         public HttpMethod Method { get; set; }
         public string PathBase { get; set; }
 
-        protected string _parsedPath = null;
         public string Path { get; set; }
-
-        protected string _parsedQueryString = null;
         public string QueryString { get; set; }
-
-        protected string _parsedRawTarget = null;
         public string RawTarget { get; set; }
 
         public string HttpVersion

src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs

@@ -253,6 +253,38 @@ await connection.Receive($"HTTP/1.1 200 OK",
             }
         }
 
+        [Fact]
+        public async Task CanHandleTwoAbsoluteFormRequestsInARow()
+        {
+            // Regression test for https://github.com/dotnet/aspnetcore/issues/18438
+            var testContext = new TestServiceContext(LoggerFactory);
+
+            await using (var server = new TestServer(TestApp.EchoAppChunked, testContext))
+            {
+                using (var connection = server.CreateConnection())
+                {
+                    await connection.Send(
+                        "GET http://localhost/ HTTP/1.1",
+                        "Host: localhost",
+                        "",
+                        "GET http://localhost/ HTTP/1.1",
+                        "Host: localhost",
+                        "",
+                        "");
+                    await connection.Receive(
+                        "HTTP/1.1 200 OK",
+                        $"Date: {testContext.DateHeaderValue}",
+                        "Content-Length: 0",
+                        "",
+                        "HTTP/1.1 200 OK",
+                        $"Date: {testContext.DateHeaderValue}",
+                        "Content-Length: 0",
+                        "",
+                        "");
+                }
+            }
+        }
+
         [Fact]
         public async Task AppCanSetTraceIdentifier()
         {
@@ -358,7 +390,6 @@ await connection.ReceiveEnd(
             }
         }
 
-
         [Fact]
         public async Task Http10NotKeptAliveByDefault()
         {