Handle absolute, asterisk, and authority-form request targets

src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs

@@ -4,19 +4,32 @@
 using System.IO;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Internal.Http;
+using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
+using Microsoft.Extensions.Primitives;
 
 namespace Microsoft.AspNetCore.Server.Kestrel
 {
     public sealed class BadHttpRequestException : IOException
     {
         private BadHttpRequestException(string message, int statusCode)
+            : this(message, statusCode, null)
+        { }
+
+        private BadHttpRequestException(string message, int statusCode, HttpMethod? requiredMethod)
             : base(message)
         {
             StatusCode = statusCode;
+
+            if (requiredMethod.HasValue)
+            {
+                AllowedHeader = HttpUtilities.MethodToString(requiredMethod.Value);
+            }
         }
 
         internal int StatusCode { get; }
 
+        internal StringValues AllowedHeader { get; }
+
         internal static BadHttpRequestException GetException(RequestRejectionReason reason)
         {
             BadHttpRequestException ex;
@@ -61,6 +74,12 @@ internal static BadHttpRequestException GetException(RequestRejectionReason reas
                 case RequestRejectionReason.RequestTimeout:
                     ex = new BadHttpRequestException("Request timed out.", StatusCodes.Status408RequestTimeout);
                     break;
+                case RequestRejectionReason.OptionsMethodRequired:
+                    ex = new BadHttpRequestException("Method not allowed.", StatusCodes.Status405MethodNotAllowed, HttpMethod.Options);
+                    break;
+                case RequestRejectionReason.ConnectMethodRequired:
+                    ex = new BadHttpRequestException("Method not allowed.", StatusCodes.Status405MethodNotAllowed, HttpMethod.Connect);
+                    break;
                 default:
                     ex = new BadHttpRequestException("Bad request.", StatusCodes.Status400BadRequest);
                     break;

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

@@ -11,7 +11,6 @@
 using System.Runtime.CompilerServices;
 using System.Text;
 using System.Text.Encodings.Web.Utf8;
-using System.Text.Utf8;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
@@ -27,6 +26,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
 {
     public abstract partial class Frame : IFrameControl, IHttpRequestLineHandler, IHttpHeadersHandler
     {
+        private const byte ByteAsterisk = (byte)'*';
+        private const byte ByteForwardSlash = (byte)'/';
         private const byte BytePercentage = (byte)'%';
 
         private static readonly ArraySegment<byte> _endChunkedResponseBytes = CreateAsciiByteArraySegment("0\r\n\r\n");
@@ -39,6 +40,9 @@ public abstract partial class Frame : IFrameControl, IHttpRequestLineHandler, IH
         private static readonly byte[] _bytesEndHeaders = Encoding.ASCII.GetBytes("\r\n\r\n");
         private static readonly byte[] _bytesServer = Encoding.ASCII.GetBytes("\r\nServer: Kestrel");
 
+        private const string EmptyPath = "/";
+        private const string Asterisk = "*";
+
         private readonly object _onStartingSync = new Object();
         private readonly object _onCompletedSync = new Object();
 
@@ -818,7 +822,7 @@ protected Task ProduceEnd()
                 // that should take precedence.
                 if (_requestRejectedException != null)
                 {
-                    SetErrorResponseHeaders(statusCode: _requestRejectedException.StatusCode);
+                    SetErrorResponseException(_requestRejectedException);
                 }
                 else
                 {
@@ -1077,7 +1081,7 @@ public bool TakeMessageHeaders(ReadableBuffer buffer, out ReadCursor consumed, o
             {
                 buffer = buffer.Slice(buffer.Start, _remainingRequestHeadersBytesAllowed);
 
-                // If we sliced it means the current buffer bigger than what we're 
+                // If we sliced it means the current buffer bigger than what we're
                 // allowed to look at
                 overLength = true;
             }
@@ -1127,6 +1131,16 @@ private void RejectNonBodyTransferEncodingResponse(bool appCompleted)
             }
         }
 
+        private void SetErrorResponseException(BadHttpRequestException ex)
+        {
+            SetErrorResponseHeaders(ex.StatusCode);
+
+            if (!StringValues.IsNullOrEmpty(ex.AllowedHeader))
+            {
+                FrameResponseHeaders.HeaderAllow = ex.AllowedHeader;
+            }
+        }
+
         private void SetErrorResponseHeaders(int statusCode)
         {
             Debug.Assert(!HasResponseStarted, $"{nameof(SetErrorResponseHeaders)} called after response had already started.");
@@ -1179,6 +1193,15 @@ public void RejectRequest(RequestRejectionReason reason, string value)
             throw BadHttpRequestException.GetException(reason, value);
         }
 
+        private void RejectRequestLine(Span<byte> requestLine)
+        {
+            Debug.Assert(Log.IsEnabled(LogLevel.Information) == true, "Use RejectRequest instead to improve inlining when log is disabled");
+
+            const int MaxRequestLineError = 32;
+            var line = requestLine.GetAsciiStringEscaped(MaxRequestLineError);
+            throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine, line);
+        }
+
         public void SetBadRequestState(RequestRejectionReason reason)
         {
             SetBadRequestState(BadHttpRequestException.GetException(reason));
@@ -1190,7 +1213,7 @@ public void SetBadRequestState(BadHttpRequestException ex)
 
             if (!HasResponseStarted)
             {
-                SetErrorResponseHeaders(ex.StatusCode);
+                SetErrorResponseException(ex);
             }
 
             _keepAlive = false;
@@ -1216,8 +1239,51 @@ protected void ReportApplicationError(Exception ex)
             Log.ApplicationError(ConnectionId, ex);
         }
 
-        public void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, bool pathEncoded)
+        public void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, Span<byte> line, bool pathEncoded)
+        {
+            Debug.Assert(target.Length != 0, "Request target must be non-zero length");
+
+            var ch = target[0];
+            if (ch == ByteForwardSlash)
+            {
+                // origin-form.
+                // The most common form of request-target.
+                // https://tools.ietf.org/html/rfc7230#section-5.3.1
+                OnOriginFormTarget(method, version, target, path, query, customMethod, pathEncoded);
+            }
+            else if (ch == ByteAsterisk && target.Length == 1)
+            {
+                OnAsteriskFormTarget(method);
+            }
+            else if (target.GetKnownHttpScheme(out var scheme))
+            {
+                OnAbsoluteFormTarget(target, query, line);
+            }
+            else
+            {
+                // Assume anything else is considered authority form.
+                // FYI: this should be an edge case. This should only happen when
+                // a client mistakenly things this server is a proxy server.
+
+                OnAuthorityFormTarget(method, target, line);
+            }
+
+            Method = method != HttpMethod.Custom
+                ? HttpUtilities.MethodToString(method) ?? string.Empty
+                : customMethod.GetAsciiStringNonNullCharacters();
+            HttpVersion = HttpUtilities.VersionToString(version);
+
+            Debug.Assert(RawTarget != null, "RawTarget was not set");
+            Debug.Assert(Method != null, "Method was not set");
+            Debug.Assert(Path != null, "Path was not set");
+            Debug.Assert(QueryString != "QueryString was not set");
+            Debug.Assert(HttpVersion != "HttpVersion was not set");
+        }
+
+        private void OnOriginFormTarget(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, bool pathEncoded)
         {
+            Debug.Assert(target[0] == ByteForwardSlash, "Should only be called when path starts with /");
+
             // 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;
             // then encoded/escaped to ASCII  https://www.ietf.org/rfc/rfc3987.txt "Mapping of IRIs to URIs"
@@ -1249,34 +1315,111 @@ public void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> targe
                 }
             }
 
-            var normalizedTarget = PathNormalizer.RemoveDotSegments(requestUrlPath);
-            if (method != HttpMethod.Custom)
+            QueryString = query.GetAsciiStringNonNullCharacters();
+            RawTarget = rawTarget;
+            SetNormalizedPath(requestUrlPath);
+        }
+
+        private void OnAuthorityFormTarget(HttpMethod method, Span<byte> target, Span<byte> line)
+        {
+            // TODO Validate that target is a correct host[:port] string.
+            // Reject as 400 if not. This is just a quick scan for invalid characters
+            // but doesn't check that the target fully matches the URI spec.
+            for (var i = 0; i < target.Length; i++)
+            {
+                var ch = target[i];
+                if (!UriUtilities.IsValidAuthorityCharacter(ch))
+                {
+                    if (Log.IsEnabled(LogLevel.Information))
+                    {
+                        RejectRequestLine(line);
+                    }
+
+                    throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine);
+                }
+            }
+
+            // The authority-form of request-target is only used for CONNECT
+            // requests (https://tools.ietf.org/html/rfc7231#section-4.3.6).
+            if (method != HttpMethod.Connect)
             {
-                Method = HttpUtilities.MethodToString(method) ?? string.Empty;
+                RejectRequest(RequestRejectionReason.ConnectMethodRequired);
             }
-            else
+
+            // When making a CONNECT request to establish a tunnel through one or
+            // more proxies, a client MUST send only the target URI's authority
+            // component(excluding any userinfo and its "@" delimiter) as the
+            // request - target.For example,
+            //
+            //  CONNECT www.example.com:80 HTTP/1.1
+            //
+            // Allowed characters in the 'host + port' section of authority.
+            // See https://tools.ietf.org/html/rfc3986#section-3.2
+
+            RawTarget = target.GetAsciiStringNonNullCharacters();
+            Path = string.Empty;
+            PathBase = string.Empty;
+            QueryString = string.Empty;
+        }
+
+        private void OnAsteriskFormTarget(HttpMethod method)
+        {
+            // The asterisk-form of request-target is only used for a server-wide
+            // OPTIONS request (https://tools.ietf.org/html/rfc7231#section-4.3.7).
+            if (method != HttpMethod.Options)
+            {
+                RejectRequest(RequestRejectionReason.OptionsMethodRequired);
+            }
+
+            RawTarget = Asterisk;
+            Path = string.Empty;
+            PathBase = string.Empty;
+            QueryString = string.Empty;
+        }
+
+        private void OnAbsoluteFormTarget(Span<byte> target, Span<byte> query, Span<byte> line)
+        {
+            // absolute-form
+            // https://tools.ietf.org/html/rfc7230#section-5.3.2
+
+            // This code should be the edge-case.
+
+            // From the spec:
+            //    a server MUST accept the absolute-form in requests, even though
+            //    HTTP/1.1 clients will only send them in requests to proxies.
+
+            RawTarget = target.GetAsciiStringNonNullCharacters();
+
+            // Validation of absolute URIs is slow, but clients
+            // should not be sending this form anyways, so perf optimization
+            // not high priority
+
+            if (!Uri.TryCreate(RawTarget, UriKind.Absolute, out var uri))
             {
-                Method = customMethod.GetAsciiStringNonNullCharacters();
+                if (Log.IsEnabled(LogLevel.Information))
+                {
+                    RejectRequestLine(line);
+                }
+
+                throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine);
             }
 
+            SetNormalizedPath(uri.LocalPath);
+            // don't use uri.Query because we need the unescaped version
             QueryString = query.GetAsciiStringNonNullCharacters();
-            RawTarget = rawTarget;
-            HttpVersion = HttpUtilities.VersionToString(version);
+        }
 
+        private void SetNormalizedPath(string requestPath)
+        {
+            var normalizedTarget = PathNormalizer.RemoveDotSegments(requestPath);
             if (RequestUrlStartsWithPathBase(normalizedTarget, out bool caseMatches))
             {
                 PathBase = caseMatches ? _pathBase : normalizedTarget.Substring(0, _pathBase.Length);
                 Path = normalizedTarget.Substring(_pathBase.Length);
             }
-            else if (rawTarget[0] == '/') // check rawTarget since normalizedTarget can be "" or "/" after dot segment removal
-            {
-                Path = normalizedTarget;
-            }
             else
             {
-                Path = string.Empty;
-                PathBase = string.Empty;
-                QueryString = string.Empty;
+                Path = normalizedTarget;
             }
         }
 

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

@@ -0,0 +1,12 @@
+// 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.
+
+namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
+{
+    public enum HttpScheme
+    {
+        Unknown = -1,
+        Http = 0,
+        Https = 1
+    }
+}

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

@@ -7,6 +7,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
 {
     public interface IHttpRequestLineHandler
     {
-        void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, bool pathEncoded);
+        void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, Span<byte> line, bool pathEncoded);
     }
 }

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

@@ -157,7 +157,9 @@ private unsafe void ParseRequestLine<T>(T handler, byte* data, int length) where
                 RejectRequestLine(data, length);
             }
 
-            handler.OnStartLine(method, httpVersion, targetBuffer, pathBuffer, query, customMethod, pathEncoded);
+            var line = new Span<byte>(data, length);
+
+            handler.OnStartLine(method, httpVersion, targetBuffer, pathBuffer, query, customMethod, line, pathEncoded);
         }
 
         public unsafe bool ParseHeaders<T>(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined, out int consumedBytes) where T : IHttpHeadersHandler
@@ -341,7 +343,7 @@ private unsafe void TakeSingleHeader<T>(byte* headerLine, int length, T handler)
             // Skip colon from value start
             var valueStart = nameEnd + 1;
             // Ignore start whitespace
-            for(; valueStart < valueEnd; valueStart++)
+            for (; valueStart < valueEnd; valueStart++)
             {
                 var ch = headerLine[valueStart];
                 if (ch != ByteTab && ch != ByteSpace && ch != ByteCR)
@@ -409,7 +411,7 @@ private static unsafe bool Contains(byte* searchSpace, int length, byte value)
                 }
             }
             return false;
-        found:
+            found:
             return true;
         }
 

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

@@ -23,6 +23,8 @@ public enum RequestRejectionReason
         RequestTimeout,
         FinalTransferCodingNotChunked,
         LengthRequired,
-        LengthRequiredHttp10
+        LengthRequiredHttp10,
+        OptionsMethodRequired,
+        ConnectMethodRequired,
     }
 }