Sanitize and centralize exception throws (#2293)

* Sanitize and centralize exception throws

src/Kestrel.Core/BadHttpRequestException.cs

@@ -1,7 +1,9 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.Diagnostics;
 using System.IO;
+using System.Runtime.CompilerServices;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
@@ -30,6 +32,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
 
         internal StringValues AllowedHeader { get; }
 
+        [StackTraceHidden]
+        internal static void Throw(RequestRejectionReason reason)
+        {
+            throw GetException(reason);
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
         internal static BadHttpRequestException GetException(RequestRejectionReason reason)
         {
             BadHttpRequestException ex;
@@ -102,6 +111,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
             return ex;
         }
 
+        [StackTraceHidden]
+        internal static void Throw(RequestRejectionReason reason, string detail)
+        {
+            throw GetException(reason, detail);
+        }
+
+        [StackTraceHidden]
+        internal static void Throw(RequestRejectionReason reason, in StringValues detail)
+        {
+            throw GetException(reason, detail.ToString());
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
         internal static BadHttpRequestException GetException(RequestRejectionReason reason, string detail)
         {
             BadHttpRequestException ex;
@@ -141,4 +163,4 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
             return ex;
         }
     }
-}
+}

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

@@ -119,7 +119,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             var result = _parser.ParseRequestLine(new Http1ParsingHandler(this), buffer, out consumed, out examined);
             if (!result && overLength)
             {
-                ThrowRequestRejected(RequestRejectionReason.RequestLineTooLong);
+                BadHttpRequestException.Throw(RequestRejectionReason.RequestLineTooLong);
             }
 
             return result;
@@ -143,7 +143,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
             if (!result && overLength)
             {
-                ThrowRequestRejected(RequestRejectionReason.HeadersExceedMaxTotalSize);
+                BadHttpRequestException.Throw(RequestRejectionReason.HeadersExceedMaxTotalSize);
             }
             if (result)
             {
@@ -278,7 +278,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             // requests (https://tools.ietf.org/html/rfc7231#section-4.3.6).
             if (method != HttpMethod.Connect)
             {
-                ThrowRequestRejected(RequestRejectionReason.ConnectMethodRequired);
+                BadHttpRequestException.Throw(RequestRejectionReason.ConnectMethodRequired);
             }
 
             // When making a CONNECT request to establish a tunnel through one or
@@ -303,7 +303,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             // OPTIONS request (https://tools.ietf.org/html/rfc7231#section-4.3.7).
             if (method != HttpMethod.Options)
             {
-                ThrowRequestRejected(RequestRejectionReason.OptionsMethodRequired);
+                BadHttpRequestException.Throw(RequestRejectionReason.OptionsMethodRequired);
             }
 
             RawTarget = Asterisk;
@@ -367,17 +367,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             var host = HttpRequestHeaders.HeaderHost;
             if (host.Count <= 0)
             {
-                ThrowRequestRejected(RequestRejectionReason.MissingHostHeader);
+                BadHttpRequestException.Throw(RequestRejectionReason.MissingHostHeader);
             }
             else if (host.Count > 1)
             {
-                ThrowRequestRejected(RequestRejectionReason.MultipleHostHeaders);
+                BadHttpRequestException.Throw(RequestRejectionReason.MultipleHostHeaders);
             }
             else if (_requestTargetForm == HttpRequestTarget.AuthorityForm)
             {
                 if (!host.Equals(RawTarget))
                 {
-                    ThrowRequestRejected(RequestRejectionReason.InvalidHostHeader, host.ToString());
+                    BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, in host);
                 }
             }
             else if (_requestTargetForm == HttpRequestTarget.AbsoluteForm)
@@ -393,7 +393,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 if ((host != _absoluteRequestTarget.Authority || !_absoluteRequestTarget.IsDefaultPort)
                     && host != authorityAndPort)
                 {
-                    ThrowRequestRejected(RequestRejectionReason.InvalidHostHeader, host.ToString());
+                    BadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, in host);
                 }
             }
         }
@@ -446,7 +446,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             {
                 if (_requestProcessingStatus == RequestProcessingStatus.ParsingHeaders)
                 {
-                    throw BadHttpRequestException.GetException(RequestRejectionReason
+                    BadHttpRequestException.Throw(RequestRejectionReason
                         .MalformedRequestInvalidHeaders);
                 }
                 throw;
@@ -464,11 +464,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                         endConnection = true;
                         return true;
                     case RequestProcessingStatus.ParsingRequestLine:
-                        throw BadHttpRequestException.GetException(
+                        BadHttpRequestException.Throw(RequestRejectionReason.InvalidRequestLine);
-                            RequestRejectionReason.InvalidRequestLine);
+                        break;
                     case RequestProcessingStatus.ParsingHeaders:
-                        throw BadHttpRequestException.GetException(
+                        BadHttpRequestException.Throw(RequestRejectionReason.MalformedRequestInvalidHeaders);
-                            RequestRejectionReason.MalformedRequestInvalidHeaders);
+                        break;
                 }
             }
             else if (!_keepAlive && _requestProcessingStatus == RequestProcessingStatus.RequestPending)
@@ -482,7 +482,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             {
                 // In this case, there is an ongoing request but the start line/header parsing has timed out, so send
                 // a 408 response.
-                throw BadHttpRequestException.GetException(RequestRejectionReason.RequestTimeout);
+                BadHttpRequestException.Throw(RequestRejectionReason.RequestTimeout);
             }
 
             endConnection = false;

src/Kestrel.Core/Internal/Http/Http1MessageBody.cs

@@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
                     if (_context.RequestTimedOut)
                     {
-                        _context.ThrowRequestRejected(RequestRejectionReason.RequestTimeout);
+                        BadHttpRequestException.Throw(RequestRejectionReason.RequestTimeout);
                     }
 
                     var readableBuffer = result.Buffer;
@@ -97,7 +97,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                         }
                         else if (result.IsCompleted)
                         {
-                            _context.ThrowRequestRejected(RequestRejectionReason.UnexpectedEndOfRequestContent);
+                            BadHttpRequestException.Throw(RequestRejectionReason.UnexpectedEndOfRequestContent);
                         }
 
                         awaitable = _context.Input.ReadAsync();
@@ -233,7 +233,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             {
                 if (headers.HeaderTransferEncoding.Count > 0 || (headers.ContentLength.HasValue && headers.ContentLength.Value != 0))
                 {
-                    context.ThrowRequestRejected(RequestRejectionReason.UpgradeRequestCannotHavePayload);
+                    BadHttpRequestException.Throw(RequestRejectionReason.UpgradeRequestCannotHavePayload);
                 }
 
                 return new ForUpgrade(context);
@@ -252,7 +252,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 // status code and then close the connection.
                 if (transferCoding != TransferCoding.Chunked)
                 {
-                    context.ThrowRequestRejected(RequestRejectionReason.FinalTransferCodingNotChunked, transferEncoding.ToString());
+                    BadHttpRequestException.Throw(RequestRejectionReason.FinalTransferCodingNotChunked, in transferEncoding);
                 }
 
                 return new ForChunkedEncoding(keepAlive, context);
@@ -278,7 +278,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 if (HttpMethods.IsPost(context.Method) || HttpMethods.IsPut(context.Method))
                 {
                     var requestRejectionReason = httpVersion == HttpVersion.Http11 ? RequestRejectionReason.LengthRequired : RequestRejectionReason.LengthRequiredHttp10;
-                    context.ThrowRequestRejected(requestRejectionReason, context.Method);
+                    BadHttpRequestException.Throw(requestRejectionReason, context.Method);
                 }
             }
 
@@ -339,7 +339,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             {
                 if (_contentLength > _context.MaxRequestBodySize)
                 {
-                    _context.ThrowRequestRejected(RequestRejectionReason.RequestBodyTooLarge);
+                    BadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTooLarge);
                 }
             }
         }
@@ -452,7 +452,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
                 if (_consumedBytes > _context.MaxRequestBodySize)
                 {
-                    _context.ThrowRequestRejected(RequestRejectionReason.RequestBodyTooLarge);
+                    BadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTooLarge);
                 }
             }
 
@@ -509,7 +509,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 }
 
                 // At this point, 10 bytes have been consumed which is enough to parse the max value "7FFFFFFF\r\n".
-                _context.ThrowRequestRejected(RequestRejectionReason.BadChunkSizeData);
+                BadHttpRequestException.Throw(RequestRejectionReason.BadChunkSizeData);
             }
 
             private void ParseExtension(ReadOnlyBuffer<byte> buffer, out SequencePosition consumed, out SequencePosition examined)
@@ -604,7 +604,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 }
                 else
                 {
-                    _context.ThrowRequestRejected(RequestRejectionReason.BadChunkSuffix);
+                    BadHttpRequestException.Throw(RequestRejectionReason.BadChunkSuffix);
                 }
             }
 
@@ -660,7 +660,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                     throw new IOException(CoreStrings.BadRequest_BadChunkSizeData, ex);
                 }
 
-                _context.ThrowRequestRejected(RequestRejectionReason.BadChunkSizeData);
+                BadHttpRequestException.Throw(RequestRejectionReason.BadChunkSizeData);
                 return -1; // can't happen, but compiler complains
             }
 

src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs

@@ -4049,7 +4049,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                             {
                                 if (_contentLength.HasValue)
                                 {
-                                    ThrowMultipleContentLengthsException();
+                                    BadHttpRequestException.Throw(RequestRejectionReason.MultipleContentLengths);
                                 }
                                 else
                                 {

src/Kestrel.Core/Internal/Http/HttpParser.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Buffers;
 using System.Collections;
+using System.Diagnostics;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
@@ -254,7 +255,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                                 }
 
                                 // Headers don't end in CRLF line.
-                                RejectRequest(RequestRejectionReason.InvalidRequestHeadersNoCRLF);
+                                BadHttpRequestException.Throw(RequestRejectionReason.InvalidRequestHeadersNoCRLF);
                             }
 
                             // We moved the reader so look ahead 2 bytes so reset both the reader
@@ -483,18 +484,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 c == '~';
         }
 
-        private void RejectRequest(RequestRejectionReason reason)
+        [StackTraceHidden]
-            => throw BadHttpRequestException.GetException(reason);
-
         private unsafe void RejectRequestLine(byte* requestLine, int length)
             => throw GetInvalidRequestException(RequestRejectionReason.InvalidRequestLine, requestLine, length);
 
+        [StackTraceHidden]
         private unsafe void RejectRequestHeader(byte* headerLine, int length)
             => throw GetInvalidRequestException(RequestRejectionReason.InvalidRequestHeader, headerLine, length);
 
+        [StackTraceHidden]
         private unsafe void RejectUnknownVersion(byte* version, int length)
             => throw GetInvalidRequestException(RequestRejectionReason.UnrecognizedHTTPVersion, version, length);
 
+        [MethodImpl(MethodImplOptions.NoInlining)]
         private unsafe BadHttpRequestException GetInvalidRequestException(RequestRejectionReason reason, byte* detail, int length)
             => BadHttpRequestException.GetException(
                 reason,

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

@@ -429,7 +429,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             _requestHeadersParsed++;
             if (_requestHeadersParsed > ServerOptions.Limits.MaxRequestHeaderCount)
             {
-                ThrowRequestRejected(RequestRejectionReason.TooManyHeaders);
+                BadHttpRequestException.Throw(RequestRejectionReason.TooManyHeaders);
             }
             var valueString = value.GetAsciiStringNonNullCharacters();
 
@@ -863,13 +863,26 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 _responseBytesWritten + count > responseHeaders.ContentLength.Value)
             {
                 _keepAlive = false;
-                throw new InvalidOperationException(
+                ThrowTooManyBytesWritten(count);
-                    CoreStrings.FormatTooManyBytesWritten(_responseBytesWritten + count, responseHeaders.ContentLength.Value));
             }
 
             _responseBytesWritten += count;
         }
 
+        [StackTraceHidden]
+        private void ThrowTooManyBytesWritten(int count)
+        {
+            throw GetTooManyBytesWrittenException(count);
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private InvalidOperationException GetTooManyBytesWrittenException(int count)
+        {
+            var responseHeaders = HttpResponseHeaders;
+            return new InvalidOperationException(
+                CoreStrings.FormatTooManyBytesWritten(_responseBytesWritten + count, responseHeaders.ContentLength.Value));
+        }
+
         private void CheckLastWrite()
         {
             var responseHeaders = HttpResponseHeaders;
@@ -1250,25 +1263,28 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             // Writes to HEAD response are ignored and logged at the end of the request
             if (Method != "HEAD")
             {
-                // Throw Exception for 204, 205, 304 responses.
+                ThrowWritingToResponseBodyNotSupported();
-                throw new InvalidOperationException(CoreStrings.FormatWritingToResponseBodyNotSupported(StatusCode));
             }
         }
 
+        [StackTraceHidden]
+        private void ThrowWritingToResponseBodyNotSupported()
+        {
+            // Throw Exception for 204, 205, 304 responses.
+            throw new InvalidOperationException(CoreStrings.FormatWritingToResponseBodyNotSupported(StatusCode));
+        }
+
+        [StackTraceHidden]
         private void ThrowResponseAbortedException()
         {
             throw new ObjectDisposedException(CoreStrings.UnhandledApplicationException, _applicationException);
         }
 
-        public void ThrowRequestRejected(RequestRejectionReason reason)
+        [StackTraceHidden]
-            => throw BadHttpRequestException.GetException(reason);
-
-        public void ThrowRequestRejected(RequestRejectionReason reason, string detail)
-            => throw BadHttpRequestException.GetException(reason, detail);
-
         public void ThrowRequestTargetRejected(Span<byte> target)
             => throw GetInvalidRequestTargetException(target);
 
+        [MethodImpl(MethodImplOptions.NoInlining)]
         private BadHttpRequestException GetInvalidRequestTargetException(Span<byte> target)
             => BadHttpRequestException.GetException(
                 RequestRejectionReason.InvalidRequestTarget,

src/Kestrel.Core/Internal/Http/HttpRequestHeaders.cs

@@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             long parsed;
             if (!HeaderUtilities.TryParseNonNegativeInt64(value, out parsed))
             {
-                ThrowInvalidContentLengthException(value);
+                BadHttpRequestException.Throw(RequestRejectionReason.InvalidContentLength, value);
             }
 
             return parsed;
@@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             {
                 if (!StringUtilities.TryGetAsciiString(pKeyBytes, keyBuffer, keyLength))
                 {
-                    throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidCharactersInHeaderName);
+                    BadHttpRequestException.Throw(RequestRejectionReason.InvalidCharactersInHeaderName);
                 }
             }
 
@@ -56,16 +56,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             Unknown[key] = AppendValue(existing, value);
         }
 
-        private static void ThrowInvalidContentLengthException(string value)
-        {
-            throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidContentLength, value);
-        }
-
-        private static void ThrowMultipleContentLengthsException()
-        {
-            throw BadHttpRequestException.GetException(RequestRejectionReason.MultipleContentLengths);
-        }
-
         public Enumerator GetEnumerator()
         {
             return new Enumerator(this);

src/Kestrel.Core/Internal/Infrastructure/StackTraceHiddenAttribute.cs

@@ -0,0 +1,17 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace System.Diagnostics
+{
+    /// <summary>
+    /// Attribute to add to non-returning throw only methods, 
+    /// to restore the stack trace back to what it would be if the throw was in-place
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Struct, Inherited = false)]
+    internal sealed class StackTraceHiddenAttribute : Attribute
+    {
+        // https://github.com/dotnet/coreclr/blob/eb54e48b13fdfb7233b7bcd32b93792ba3e89f0c/src/mscorlib/shared/System/Diagnostics/StackTraceHiddenAttribute.cs
+        public StackTraceHiddenAttribute() { }
+    }
+}

tools/CodeGenerator/KnownHeaders.cs

@@ -36,7 +36,7 @@ namespace CodeGenerator
                             {{{(header.Identifier == "ContentLength" ? $@"
                                 if (_contentLength.HasValue)
                                 {{
-                                    ThrowMultipleContentLengthsException();
+                                    BadHttpRequestException.Throw(RequestRejectionReason.MultipleContentLengths);
                                 }}
                                 else
                                 {{