Add non-throwing parser path for bad request handling

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

@@ -10,6 +10,7 @@
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Internal;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
 
@@ -164,51 +165,6 @@ public void HandleReadDataRateTimeout()
         SendTimeoutResponse();
     }
 
-    public bool ParseRequest(ref SequenceReader<byte> reader)
-    {
-        switch (_requestProcessingStatus)
-        {
-            case RequestProcessingStatus.RequestPending:
-                // Skip any empty lines (\r or \n) between requests.
-                // Peek first as a minor performance optimization; it's a quick inlined check.
-                if (reader.TryPeek(out byte b) && (b == ByteCR || b == ByteLF))
-                {
-                    reader.AdvancePastAny(ByteCR, ByteLF);
-                }
-
-                if (reader.End)
-                {
-                    break;
-                }
-
-                TimeoutControl.ResetTimeout(ServerOptions.Limits.RequestHeadersTimeout, TimeoutReason.RequestHeaders);
-
-                _requestProcessingStatus = RequestProcessingStatus.ParsingRequestLine;
-                goto case RequestProcessingStatus.ParsingRequestLine;
-            case RequestProcessingStatus.ParsingRequestLine:
-                if (TakeStartLine(ref reader))
-                {
-                    _requestProcessingStatus = RequestProcessingStatus.ParsingHeaders;
-                    goto case RequestProcessingStatus.ParsingHeaders;
-                }
-                else
-                {
-                    break;
-                }
-            case RequestProcessingStatus.ParsingHeaders:
-                if (TakeMessageHeaders(ref reader, trailers: false))
-                {
-                    _requestProcessingStatus = RequestProcessingStatus.AppStarted;
-                    // Consumed preamble
-                    return true;
-                }
-                break;
-        }
-
-        // Haven't completed consuming preamble
-        return false;
-    }
-
     public bool TakeStartLine(ref SequenceReader<byte> reader)
     {
         // Make sure the buffer is limited
@@ -287,6 +243,128 @@ bool TrimAndTakeMessageHeaders(ref SequenceReader<byte> reader, bool trailers)
         }
     }
 
+    /// <summary>
+    /// Non-throwing version of ParseRequest. Returns HttpParseResult instead of throwing.
+    /// </summary>
+    private HttpParseResult TryParseRequestCore(ref SequenceReader<byte> reader)
+    {
+        switch (_requestProcessingStatus)
+        {
+            case RequestProcessingStatus.RequestPending:
+                if (reader.TryPeek(out byte b) && (b == ByteCR || b == ByteLF))
+                {
+                    reader.AdvancePastAny(ByteCR, ByteLF);
+                }
+
+                if (reader.End)
+                {
+                    return HttpParseResult.Incomplete;
+                }
+
+                TimeoutControl.ResetTimeout(ServerOptions.Limits.RequestHeadersTimeout, TimeoutReason.RequestHeaders);
+                _requestProcessingStatus = RequestProcessingStatus.ParsingRequestLine;
+                goto case RequestProcessingStatus.ParsingRequestLine;
+
+            case RequestProcessingStatus.ParsingRequestLine:
+                var startLineResult = TryTakeStartLineCore(ref reader);
+                if (startLineResult.HasError)
+                {
+                    return startLineResult;
+                }
+                if (startLineResult.IsComplete)
+                {
+                    _requestProcessingStatus = RequestProcessingStatus.ParsingHeaders;
+                    goto case RequestProcessingStatus.ParsingHeaders;
+                }
+                return HttpParseResult.Incomplete;
+
+            case RequestProcessingStatus.ParsingHeaders:
+                var headersResult = TryTakeMessageHeadersCore(ref reader, trailers: false);
+                if (headersResult.HasError)
+                {
+                    return headersResult;
+                }
+                if (headersResult.IsComplete)
+                {
+                    _requestProcessingStatus = RequestProcessingStatus.AppStarted;
+                    return HttpParseResult.Complete;
+                }
+                return HttpParseResult.Incomplete;
+        }
+
+        return HttpParseResult.Incomplete;
+    }
+
+    private HttpParseResult TryTakeStartLineCore(ref SequenceReader<byte> reader)
+    {
+        if (reader.Remaining >= ServerOptions.Limits.MaxRequestLineSize)
+        {
+            return TryTrimAndTakeStartLineCore(ref reader);
+        }
+
+        return ((HttpParser<Http1ParsingHandler>)_parser).TryParseRequestLine(new Http1ParsingHandler(this), ref reader);
+
+        HttpParseResult TryTrimAndTakeStartLineCore(ref SequenceReader<byte> reader)
+        {
+            var trimmedBuffer = reader.Sequence.Slice(reader.Position, ServerOptions.Limits.MaxRequestLineSize);
+            var trimmedReader = new SequenceReader<byte>(trimmedBuffer);
+
+            var result = ((HttpParser<Http1ParsingHandler>)_parser).TryParseRequestLine(new Http1ParsingHandler(this), ref trimmedReader);
+            if (result.HasError)
+            {
+                return result;
+            }
+            if (!result.IsComplete)
+            {
+                return HttpParseResult.Error(RequestRejectionReason.RequestLineTooLong);
+            }
+
+            reader.Advance(trimmedReader.Consumed);
+            return HttpParseResult.Complete;
+        }
+    }
+
+    private HttpParseResult TryTakeMessageHeadersCore(ref SequenceReader<byte> reader, bool trailers)
+    {
+        if (reader.Remaining > _remainingRequestHeadersBytesAllowed)
+        {
+            return TryTrimAndTakeMessageHeadersCore(ref reader, trailers);
+        }
+
+        var alreadyConsumed = reader.Consumed;
+        var result = ((HttpParser<Http1ParsingHandler>)_parser).TryParseHeaders(new Http1ParsingHandler(this, trailers), ref reader);
+        _remainingRequestHeadersBytesAllowed -= reader.Consumed - alreadyConsumed;
+
+        if (result.IsComplete)
+        {
+            TimeoutControl.CancelTimeout();
+        }
+
+        return result;
+
+        HttpParseResult TryTrimAndTakeMessageHeadersCore(ref SequenceReader<byte> reader, bool trailers)
+        {
+            var trimmedBuffer = reader.Sequence.Slice(reader.Position, _remainingRequestHeadersBytesAllowed);
+            var trimmedReader = new SequenceReader<byte>(trimmedBuffer);
+
+            var result = ((HttpParser<Http1ParsingHandler>)_parser).TryParseHeaders(new Http1ParsingHandler(this, trailers), ref trimmedReader);
+            _remainingRequestHeadersBytesAllowed -= trimmedReader.Consumed;
+
+            if (result.HasError)
+            {
+                return result;
+            }
+            if (!result.IsComplete)
+            {
+                return HttpParseResult.Error(RequestRejectionReason.HeadersExceedMaxTotalSize);
+            }
+
+            TimeoutControl.CancelTimeout();
+            reader.Advance(trimmedReader.Consumed);
+            return HttpParseResult.Complete;
+        }
+    }
+
     public void OnStartLine(HttpVersionAndMethod versionAndMethod, TargetOffsetPathLength targetPath, Span<byte> startLine)
     {
         // Null characters are not allowed and should have been checked by HttpParser before calling this method
@@ -722,29 +800,45 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio
     {
         var reader = new SequenceReader<byte>(result.Buffer);
         var isConsumed = false;
+
+        // Use non-throwing parser path for performance.
+        // Note: The parser itself doesn't throw, but handler callbacks (OnStartLine, OnHeader)
+        // can still throw BadHttpRequestException for semantic errors (e.g., invalid request target).
+        // We catch those here to ensure metrics are recorded via OnBadRequest.
+        HttpParseResult parseResult;
         try
         {
-            isConsumed = ParseRequest(ref reader);
-        }
-        catch (InvalidOperationException) when (_requestProcessingStatus == RequestProcessingStatus.ParsingHeaders)
-        {
-            KestrelBadHttpRequestException.Throw(RequestRejectionReason.MalformedRequestInvalidHeaders);
-            throw;
+            parseResult = TryParseRequestCore(ref reader);
+
+            // Handle parse errors without exceptions
+            if (parseResult.HasError)
+            {
+                // Create exception and call OnBadRequest BEFORE finally runs AdvanceTo,
+                // as that may invalidate the buffer
+                var ex = CreateBadRequestException(parseResult, result.Buffer);
+                OnBadRequest(result.Buffer, ex);
+
+                SetBadRequestState(ex);
+                endConnection = true;
+                return true;
+            }
+
+            isConsumed = parseResult.IsComplete;
         }
 #pragma warning disable CS0618 // Type or member is obsolete
         catch (BadHttpRequestException ex)
         {
+            // OnBadRequest must be called before finally runs AdvanceTo
             OnBadRequest(result.Buffer, ex);
 
-            // Avoid re-throwing - handle the bad request state directly here
-            // to eliminate exception propagation overhead through async state machines
             SetBadRequestState(ex);
             endConnection = true;
             return true;
         }
 #pragma warning restore CS0618 // Type or member is obsolete
         catch (Exception)
         {
+            // Record OtherError for unexpected exceptions that escape the parser
             KestrelMetrics.AddConnectionEndReason(MetricsContext, ConnectionEndReason.OtherError);
             throw;
         }
@@ -761,25 +855,35 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio
                     endConnection = true;
                     return true;
                 case RequestProcessingStatus.ParsingRequestLine:
-                    KestrelBadHttpRequestException.Throw(RequestRejectionReason.InvalidRequestLine);
-                    break;
+                    {
+                        var ex = KestrelBadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine);
+                        OnBadRequest(result.Buffer, ex);
+                        SetBadRequestState(ex);
+                        endConnection = true;
+                        return true;
+                    }
                 case RequestProcessingStatus.ParsingHeaders:
-                    KestrelBadHttpRequestException.Throw(RequestRejectionReason.MalformedRequestInvalidHeaders);
-                    break;
+                    {
+                        var ex = KestrelBadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders);
+                        OnBadRequest(result.Buffer, ex);
+                        SetBadRequestState(ex);
+                        endConnection = true;
+                        return true;
+                    }
             }
         }
         else if (!_keepAlive && _requestProcessingStatus == RequestProcessingStatus.RequestPending)
         {
-            // Stop the request processing loop if the server is shutting down or there was a keep-alive timeout
-            // and there is no ongoing request.
             endConnection = true;
             return true;
         }
         else if (RequestTimedOut)
         {
-            // In this case, there is an ongoing request but the start line/header parsing has timed out, so send
-            // a 408 response.
-            KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestHeadersTimeout);
+            var ex = KestrelBadHttpRequestException.GetException(RequestRejectionReason.RequestHeadersTimeout);
+            OnBadRequest(result.Buffer, ex);
+            SetBadRequestState(ex);
+            endConnection = true;
+            return true;
         }
 
         endConnection = false;
@@ -836,6 +940,34 @@ internal static ConnectionEndReason GetConnectionEndReason(Microsoft.AspNetCore.
         }
     }
 
+#pragma warning disable CS0618 // Type or member is obsolete
+    /// <summary>
+    /// Creates a BadHttpRequestException with error detail extracted from the buffer.
+    /// </summary>
+    private BadHttpRequestException CreateBadRequestException(HttpParseResult parseResult, ReadOnlySequence<byte> buffer)
+    {
+        // Some error reasons don't use detail
+        if (parseResult.ErrorReason == RequestRejectionReason.InvalidRequestHeadersNoCRLF)
+        {
+            return KestrelBadHttpRequestException.GetException(parseResult.ErrorReason);
+        }
+
+        // Extract error detail from buffer if available, but only when logging is enabled
+        // to avoid leaking internal details in production.
+        if (Log.IsEnabled(LogLevel.Information) &&
+            parseResult.ErrorLength > 0 &&
+            parseResult.ErrorOffset + parseResult.ErrorLength <= buffer.Length)
+        {
+            var errorSlice = buffer.Slice(parseResult.ErrorOffset, parseResult.ErrorLength);
+            var errorBytes = errorSlice.IsSingleSegment ? errorSlice.FirstSpan : errorSlice.ToArray();
+            var detail = errorBytes.GetAsciiStringEscaped(Constants.MaxExceptionDetailSize);
+            return KestrelBadHttpRequestException.GetException(parseResult.ErrorReason, detail);
+        }
+
+        return KestrelBadHttpRequestException.GetException(parseResult.ErrorReason);
+    }
+#pragma warning restore CS0618 // Type or member is obsolete
+
 #pragma warning disable CS0618 // Type or member is obsolete
     private void OnBadRequest(ReadOnlySequence<byte> requestData, BadHttpRequestException ex)
 #pragma warning restore CS0618 // Type or member is obsolete