Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Commit

Permalink
Use enum for method rather than string compare
Browse files Browse the repository at this point in the history
  • Loading branch information
benaadams committed Feb 3, 2018
1 parent 93b10d9 commit 22fb267
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 19 deletions.
11 changes: 7 additions & 4 deletions src/Kestrel.Core/Internal/Http/Http1Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,13 +182,16 @@ public void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> targe
OnAuthorityFormTarget(method, target);
}

Method = method != HttpMethod.Custom
? HttpUtilities.MethodToString(method) ?? string.Empty
: customMethod.GetAsciiStringNonNullCharacters();
Method = method;
if (method == HttpMethod.Custom)
{
_customMethod = 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");
Expand Down
2 changes: 1 addition & 1 deletion src/Kestrel.Core/Internal/Http/Http1MessageBody.cs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ public static MessageBody For(
{
// 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;
context.ThrowRequestRejected(requestRejectionReason, context.Method);
Expand Down
19 changes: 16 additions & 3 deletions src/Kestrel.Core/Internal/Http/HttpProtocol.FeatureCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Server.Kestrel.Core.Features;
using Microsoft.Extensions.Primitives;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;

namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{
Expand Down Expand Up @@ -90,8 +90,21 @@ string IHttpRequestFeature.Scheme

string IHttpRequestFeature.Method
{
get => Method;
set => Method = value;
get
{
if (_customMethod != null)
{
return _customMethod;
}

_customMethod = HttpUtilities.MethodToString(Method);
return _customMethod;
}
set
{
_customMethod = HttpUtilities.GetKnownMethod(value, out var method) ? null : value;
Method = method;
}
}

string IHttpRequestFeature.PathBase
Expand Down
19 changes: 12 additions & 7 deletions src/Kestrel.Core/Internal/Http/HttpProtocol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public abstract partial class HttpProtocol : IHttpResponseControl

private readonly IHttpProtocolContext _context;

protected string _customMethod = null;
private string _scheme = null;

public HttpProtocol(IHttpProtocolContext context)
Expand Down Expand Up @@ -119,7 +120,7 @@ public string TraceIdentifier
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; }
Expand Down Expand Up @@ -324,7 +325,8 @@ public void Reset()
MaxRequestBodySize = ServerOptions.Limits.MaxRequestBodySize;
AllowSynchronousIO = ServerOptions.AllowSynchronousIO;
TraceIdentifier = null;
Method = null;
Method = HttpMethod.Get;
_customMethod = null;
PathBase = null;
Path = null;
RawTarget = null;
Expand Down Expand Up @@ -874,7 +876,7 @@ protected void VerifyResponseContentLength()
{
var responseHeaders = HttpResponseHeaders;

if (!HttpMethods.IsHead(Method) &&
if (Method != HttpMethod.Head &&
!responseHeaders.HasTransferEncoding &&
responseHeaders.ContentLength.HasValue &&
_responseBytesWritten < responseHeaders.ContentLength.Value)
Expand Down Expand Up @@ -1042,7 +1044,7 @@ private Task WriteSuffix()
Log.ConnectionKeepAlive(ConnectionId);
}

if (HttpMethods.IsHead(Method) && _responseBytesWritten > 0)
if (Method == HttpMethod.Head && _responseBytesWritten > 0)
{
Log.ConnectionHeadResponseBodyWrite(ConnectionId, _responseBytesWritten);
}
Expand All @@ -1062,7 +1064,7 @@ private async Task WriteSuffixAwaited()
Log.ConnectionKeepAlive(ConnectionId);
}

if (HttpMethods.IsHead(Method) && _responseBytesWritten > 0)
if (Method == HttpMethod.Head && _responseBytesWritten > 0)
{
Log.ConnectionHeadResponseBodyWrite(ConnectionId, _responseBytesWritten);
}
Expand Down Expand Up @@ -1092,7 +1094,7 @@ private void CreateResponseHeader(bool appCompleted)
}

// 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.
Expand Down Expand Up @@ -1229,7 +1231,7 @@ private void SetErrorResponseHeaders(int statusCode)
public void HandleNonBodyResponseWrite()
{
// Writes to HEAD response are ignored and logged at the end of the request
if (Method != "HEAD")
if (Method != HttpMethod.Head)
{
// Throw Exception for 204, 205, 304 responses.
throw new InvalidOperationException(CoreStrings.FormatWritingToResponseBodyNotSupported(StatusCode));
Expand All @@ -1247,6 +1249,9 @@ public void ThrowRequestRejected(RequestRejectionReason reason)
public void ThrowRequestRejected(RequestRejectionReason reason, string detail)
=> throw BadHttpRequestException.GetException(reason, detail);

public void ThrowRequestRejected(RequestRejectionReason reason, HttpMethod method)
=> throw BadHttpRequestException.GetException(reason, method.ToString().ToUpperInvariant());

public void ThrowRequestTargetRejected(Span<byte> target)
=> throw GetInvalidRequestTargetException(target);

Expand Down
3 changes: 2 additions & 1 deletion src/Kestrel.Core/Internal/Http2/Http2Stream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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;
Expand Down Expand Up @@ -53,7 +54,7 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio
// 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"];
((IHttpRequestFeature)this).Method = RequestHeaders[":method"];
Scheme = RequestHeaders[":scheme"];
_httpVersion = Http.HttpVersion.Http2;

Expand Down
104 changes: 104 additions & 0 deletions src/Kestrel.Core/Internal/Infrastructure/HttpUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,110 @@ internal static unsafe HttpMethod GetKnownMethod(byte* data, int length, out int
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><c>true</c> if the input matches a known string, <c>false</c> otherwise.</returns>
public static bool GetKnownMethod(string value, out HttpMethod method)
{
// Only called if user code overrides and sets the Method on IHttpRequestFeature
// This should be a rare occurance so not performance sensitive also needs to validate inputs
if (value == null)
{
throw new ArgumentNullException(nameof(value));
}

var length = value.Length;
if (length == 0)
{
throw new ArgumentException(nameof(value));
}

var firstChar = value[0];
if (length == 3)
{
if (firstChar == 'G' && string.Equals(value, "GET", StringComparison.Ordinal))
{
method = HttpMethod.Get;
return true;
}
if (firstChar == 'P' && string.Equals(value, "PUT", StringComparison.Ordinal))
{
method = HttpMethod.Put;
return true;
}

method = HttpMethod.Custom;
return false;
}
if (length == 4)
{
if (firstChar == 'H' && string.Equals(value, "HEAD", StringComparison.Ordinal))
{
method = HttpMethod.Head;
return true;
}
if (firstChar == 'P' && string.Equals(value, "POST", StringComparison.Ordinal))
{
method = HttpMethod.Post;
return true;
}

method = HttpMethod.Custom;
return false;
}
if (length == 5)
{
if (firstChar == 'T' && string.Equals(value, "TRACE", StringComparison.Ordinal))
{
method = HttpMethod.Trace;
return true;
}
if (firstChar == 'P' && string.Equals(value, "PATCH", StringComparison.Ordinal))
{
method = HttpMethod.Patch;
return true;
}

method = HttpMethod.Custom;
return false;
}
if (length == 6)
{
if (firstChar == 'D' && string.Equals(value, "DELETE", StringComparison.Ordinal))
{
method = HttpMethod.Delete;
return true;
}

method = HttpMethod.Custom;
return false;
}
if (length == 7)
{
if (firstChar == 'C' && string.Equals(value, "CONNECT", StringComparison.Ordinal))
{
method = HttpMethod.Connect;
return true;
}
if (firstChar == 'O' && string.Equals(value, "OPTIONS", StringComparison.Ordinal))
{
method = HttpMethod.Options;
return true;
}

method = HttpMethod.Custom;
return false;
}

method = HttpMethod.Custom;
return false;
}

/// <summary>
/// Checks 9 bytes from <paramref name="span"/> correspond to a known HTTP version.
/// </summary>
Expand Down
36 changes: 35 additions & 1 deletion test/Kestrel.Core.Tests/Http1ConnectionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ public async Task TakeStartLineSetsHttpProtocolProperties(
_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);
Expand Down Expand Up @@ -493,6 +493,40 @@ public async Task TakeStartLineThrowsWhenMethodNotAllowed(string requestLine, Ht
Assert.Equal(HttpUtilities.MethodToString(allowedMethod), exception.AllowedHeader);
}

[Theory]
[InlineData("g", HttpMethod.Custom, "g")]
[InlineData("G", HttpMethod.Custom, "G")]
[InlineData("get", HttpMethod.Custom, "get")]
[InlineData("GET", HttpMethod.Get, "GET")]
[InlineData("put", HttpMethod.Custom, "put")]
[InlineData("PUT", HttpMethod.Put, "PUT")]
[InlineData("post", HttpMethod.Custom, "post")]
[InlineData("POST", HttpMethod.Post, "POST")]
[InlineData("head", HttpMethod.Custom, "head")]
[InlineData("HEAD", HttpMethod.Head, "HEAD")]
[InlineData("patch", HttpMethod.Custom, "patch")]
[InlineData("PATCH", HttpMethod.Patch, "PATCH")]
[InlineData("trace", HttpMethod.Custom, "trace")]
[InlineData("TRACE", HttpMethod.Trace, "TRACE")]
[InlineData("delete", HttpMethod.Custom, "delete")]
[InlineData("DELETE", HttpMethod.Delete, "DELETE")]
[InlineData("options", HttpMethod.Custom, "options")]
[InlineData("OPTIONS", HttpMethod.Options, "OPTIONS")]
[InlineData("connect", HttpMethod.Custom, "connect")]
[InlineData("CONNECT", HttpMethod.Connect, "CONNECT")]
[InlineData("unknown", HttpMethod.Custom, "unknown")]
[InlineData("UNKNOWN", HttpMethod.Custom, "UNKNOWN")]
public void RequestFeatureMethodSetsFrameEnum(string method, HttpMethod expectedEnum, string expectedString)
{
using (var input = new TestInput())
{
((IHttpRequestFeature)input.Http1Connection).Method = method;

Assert.Equal(expectedEnum, input.Http1Connection.Method);
Assert.Equal(expectedString, ((IHttpRequestFeature)input.Http1Connection).Method);
}
}

[Fact]
public void ProcessRequestsAsyncEnablesKeepAliveTimeout()
{
Expand Down
4 changes: 2 additions & 2 deletions test/Kestrel.Core.Tests/MessageBodyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public void ForThrowsWhenMethodRequiresLengthButNoContentLengthOrTransferEncodin
{
using (var input = new TestInput())
{
input.Http1Connection.Method = method;
((IHttpRequestFeature)input.Http1Connection).Method = method;
var ex = Assert.Throws<BadHttpRequestException>(() =>
Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders(), input.Http1Connection));

Expand All @@ -355,7 +355,7 @@ public void ForThrowsWhenMethodRequiresLengthButNoContentLengthSetHttp10(string
{
using (var input = new TestInput())
{
input.Http1Connection.Method = method;
((IHttpRequestFeature)input.Http1Connection).Method = method;
var ex = Assert.Throws<BadHttpRequestException>(() =>
Http1MessageBody.For(HttpVersion.Http10, new HttpRequestHeaders(), input.Http1Connection));

Expand Down

0 comments on commit 22fb267

Please sign in to comment.