Use enum for method rather than string compares (#2294)

This commit is contained in:
Ben Adams 2018-02-23 22:29:42 +00:00 committed by Stephen Halter
parent 39951e892e
commit de7e2a2573
10 changed files with 137 additions and 27 deletions

View File

@ -38,6 +38,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
throw GetException(reason);
}
[StackTraceHidden]
public static void Throw(RequestRejectionReason reason, HttpMethod method)
=> throw GetException(reason, method.ToString().ToUpperInvariant());
[MethodImpl(MethodImplOptions.NoInlining)]
internal static BadHttpRequestException GetException(RequestRejectionReason reason)
{

View File

@ -181,13 +181,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
OnAuthorityFormTarget(method, target);
}
Method = method != HttpMethod.Custom
? HttpUtilities.MethodToString(method) ?? string.Empty
: customMethod.GetAsciiStringNonNullCharacters();
Method = method;
if (method == HttpMethod.Custom)
{
_methodText = customMethod.GetAsciiStringNonNullCharacters();
}
_httpVersion = version;
Debug.Assert(RawTarget != null, "RawTarget was not set");
Debug.Assert(Method != null, "Method was not set");
Debug.Assert(((IHttpRequestFeature)this).Method != null, "Method was not set");
Debug.Assert(Path != null, "Path was not set");
Debug.Assert(QueryString != null, "QueryString was not set");
Debug.Assert(HttpVersion != null, "HttpVersion was not set");

View File

@ -275,7 +275,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{
// If we got here, request contains no Content-Length or Transfer-Encoding header.
// Reject with 411 Length Required.
if (HttpMethods.IsPost(context.Method) || HttpMethods.IsPut(context.Method))
if (context.Method == HttpMethod.Post || context.Method == HttpMethod.Put)
{
var requestRejectionReason = httpVersion == HttpVersion.Http11 ? RequestRejectionReason.LengthRequired : RequestRejectionReason.LengthRequiredHttp10;
BadHttpRequestException.Throw(requestRejectionReason, context.Method);

View File

@ -16,5 +16,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
Options,
Custom,
None = byte.MaxValue,
}
}

View File

@ -11,6 +11,7 @@ using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Server.Kestrel.Core.Features;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{
@ -89,8 +90,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
string IHttpRequestFeature.Method
{
get => Method;
set => Method = value;
get
{
if (_methodText != null)
{
return _methodText;
}
_methodText = HttpUtilities.MethodToString(Method) ?? string.Empty;
return _methodText;
}
set
{
_methodText = value;
}
}
string IHttpRequestFeature.PathBase

View File

@ -62,6 +62,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
private readonly IHttpProtocolContext _context;
protected string _methodText = null;
private string _scheme = null;
public HttpProtocol(IHttpProtocolContext context)
@ -119,7 +120,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
public IPAddress LocalIpAddress { get; set; }
public int LocalPort { get; set; }
public string Scheme { get; set; }
public string Method { get; set; }
public HttpMethod Method { get; set; }
public string PathBase { get; set; }
public string Path { get; set; }
public string QueryString { get; set; }
@ -324,7 +325,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
MaxRequestBodySize = ServerOptions.Limits.MaxRequestBodySize;
AllowSynchronousIO = ServerOptions.AllowSynchronousIO;
TraceIdentifier = null;
Method = null;
Method = HttpMethod.None;
_methodText = null;
PathBase = null;
Path = null;
RawTarget = null;
@ -905,7 +907,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{
var responseHeaders = HttpResponseHeaders;
if (!HttpMethods.IsHead(Method) &&
if (Method != HttpMethod.Head &&
StatusCode != StatusCodes.Status304NotModified &&
!responseHeaders.HasTransferEncoding &&
responseHeaders.ContentLength.HasValue &&
@ -1074,7 +1076,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
Log.ConnectionKeepAlive(ConnectionId);
}
if (HttpMethods.IsHead(Method) && _responseBytesWritten > 0)
if (Method == HttpMethod.Head && _responseBytesWritten > 0)
{
Log.ConnectionHeadResponseBodyWrite(ConnectionId, _responseBytesWritten);
}
@ -1094,7 +1096,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
Log.ConnectionKeepAlive(ConnectionId);
}
if (HttpMethods.IsHead(Method) && _responseBytesWritten > 0)
if (Method == HttpMethod.Head && _responseBytesWritten > 0)
{
Log.ConnectionHeadResponseBodyWrite(ConnectionId, _responseBytesWritten);
}
@ -1124,7 +1126,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
}
// Set whether response can have body
_canHaveBody = StatusCanHaveBody(StatusCode) && Method != "HEAD";
_canHaveBody = StatusCanHaveBody(StatusCode) && Method != HttpMethod.Head;
// Don't set the Content-Length or Transfer-Encoding headers
// automatically for HEAD requests or 204, 205, 304 responses.
@ -1261,7 +1263,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
public void HandleNonBodyResponseWrite()
{
// Writes to HEAD response are ignored and logged at the end of the request
if (Method != "HEAD")
if (Method != HttpMethod.Head)
{
ThrowWritingToResponseBodyNotSupported();
}

View File

@ -5,6 +5,7 @@ using System;
using System.Buffers;
using System.IO.Pipelines;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Server.Kestrel.Core.Features;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
@ -53,7 +54,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
// We don't need any of the parameters because we don't implement BeginRead to actually
// do the reading from a pipeline, nor do we use endConnection to report connection-level errors.
Method = RequestHeaders[":method"];
var methodText = RequestHeaders[":method"];
Method = HttpUtilities.GetKnownMethod(methodText);
_methodText = methodText;
Scheme = RequestHeaders[":scheme"];
_httpVersion = Http.HttpVersion.Http2;

View File

@ -6,6 +6,7 @@ using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
@ -177,6 +178,87 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
return HttpMethod.Custom;
}
/// <summary>
/// Parses string <paramref name="value"/> for a known HTTP method.
/// </summary>
/// <remarks>
/// A "known HTTP method" can be an HTTP method name defined in the HTTP/1.1 RFC.
/// The Known Methods (CONNECT, DELETE, GET, HEAD, PATCH, POST, PUT, OPTIONS, TRACE)
/// </remarks>
/// <returns><see cref="HttpMethod"/></returns>
public static HttpMethod GetKnownMethod(string value)
{
// Called by http/2
if (value == null)
{
throw new ArgumentNullException(nameof(value));
}
var length = value.Length;
if (length == 0)
{
throw new ArgumentException(nameof(value));
}
// Start with custom and assign if known method is found
var method = HttpMethod.Custom;
var firstChar = value[0];
if (length == 3)
{
if (firstChar == 'G' && string.Equals(value, HttpMethods.Get, StringComparison.Ordinal))
{
method = HttpMethod.Get;
}
else if (firstChar == 'P' && string.Equals(value, HttpMethods.Put, StringComparison.Ordinal))
{
method = HttpMethod.Put;
}
}
else if (length == 4)
{
if (firstChar == 'H' && string.Equals(value, HttpMethods.Head, StringComparison.Ordinal))
{
method = HttpMethod.Head;
}
else if(firstChar == 'P' && string.Equals(value, HttpMethods.Post, StringComparison.Ordinal))
{
method = HttpMethod.Post;
}
}
else if (length == 5)
{
if (firstChar == 'T' && string.Equals(value, HttpMethods.Trace, StringComparison.Ordinal))
{
method = HttpMethod.Trace;
}
else if(firstChar == 'P' && string.Equals(value, HttpMethods.Patch, StringComparison.Ordinal))
{
method = HttpMethod.Patch;
}
}
else if (length == 6)
{
if (firstChar == 'D' && string.Equals(value, HttpMethods.Delete, StringComparison.Ordinal))
{
method = HttpMethod.Delete;
}
}
else if (length == 7)
{
if (firstChar == 'C' && string.Equals(value, HttpMethods.Connect, StringComparison.Ordinal))
{
method = HttpMethod.Connect;
}
else if (firstChar == 'O' && string.Equals(value, HttpMethods.Options, StringComparison.Ordinal))
{
method = HttpMethod.Options;
}
}
return method;
}
/// <summary>
/// Checks 9 bytes from <paramref name="span"/> correspond to a known HTTP version.
/// </summary>

View File

@ -344,7 +344,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
_transport.Input.AdvanceTo(_consumed, _examined);
Assert.True(returnValue);
Assert.Equal(expectedMethod, _http1Connection.Method);
Assert.Equal(expectedMethod, ((IHttpRequestFeature)_http1Connection).Method);
Assert.Equal(expectedRawTarget, _http1Connection.RawTarget);
Assert.Equal(expectedDecodedPath, _http1Connection.Path);
Assert.Equal(expectedQueryString, _http1Connection.QueryString);
@ -532,7 +532,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
{
// Arrange
_http1Connection.HttpVersion = "HTTP/1.1";
((IHttpRequestFeature)_http1Connection).Method = "HEAD";
_http1Connection.Method = HttpMethod.Head;
// Act/Assert
await _http1Connection.WriteAsync(new ArraySegment<byte>(new byte[1]));
@ -543,7 +543,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
{
// Arrange
_http1Connection.HttpVersion = "HTTP/1.1";
((IHttpRequestFeature)_http1Connection).Method = "HEAD";
_http1Connection.Method = HttpMethod.Head;
// Act/Assert
await _http1Connection.WriteAsync(new ArraySegment<byte>(new byte[1]), default(CancellationToken));
@ -554,7 +554,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
{
// Arrange
_http1Connection.HttpVersion = "HTTP/1.1";
((IHttpRequestFeature)_http1Connection).Method = "HEAD";
_http1Connection.Method = HttpMethod.Head;
// Act
_http1Connection.ResponseHeaders.Add("Transfer-Encoding", "chunked");

View File

@ -333,9 +333,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
}
[Theory]
[InlineData("POST")]
[InlineData("PUT")]
public void ForThrowsWhenMethodRequiresLengthButNoContentLengthOrTransferEncodingIsSet(string method)
[InlineData(HttpMethod.Post)]
[InlineData(HttpMethod.Put)]
public void ForThrowsWhenMethodRequiresLengthButNoContentLengthOrTransferEncodingIsSet(HttpMethod method)
{
using (var input = new TestInput())
{
@ -344,14 +344,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders(), input.Http1Connection));
Assert.Equal(StatusCodes.Status411LengthRequired, ex.StatusCode);
Assert.Equal(CoreStrings.FormatBadRequest_LengthRequired(method), ex.Message);
Assert.Equal(CoreStrings.FormatBadRequest_LengthRequired(((IHttpRequestFeature)input.Http1Connection).Method), ex.Message);
}
}
[Theory]
[InlineData("POST")]
[InlineData("PUT")]
public void ForThrowsWhenMethodRequiresLengthButNoContentLengthSetHttp10(string method)
[InlineData(HttpMethod.Post)]
[InlineData(HttpMethod.Put)]
public void ForThrowsWhenMethodRequiresLengthButNoContentLengthSetHttp10(HttpMethod method)
{
using (var input = new TestInput())
{
@ -360,7 +360,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
Http1MessageBody.For(HttpVersion.Http10, new HttpRequestHeaders(), input.Http1Connection));
Assert.Equal(StatusCodes.Status400BadRequest, ex.StatusCode);
Assert.Equal(CoreStrings.FormatBadRequest_LengthRequiredHttp10(method), ex.Message);
Assert.Equal(CoreStrings.FormatBadRequest_LengthRequiredHttp10(((IHttpRequestFeature)input.Http1Connection).Method), ex.Message);
}
}