huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Modify RequestProcessingAsync to call single parse method (#1427) 

* Modify RequestProcessingAsync to call single parse method
* Fix bad request logging
This commit is contained in:
Stephen Halter 2017-03-03 14:43:32 -08:00 committed by GitHub
parent 1d685e195e
commit ac60f13312
8 changed files with 111 additions and 123 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs
View File

@ -40,6 +40,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel
case RequestRejectionReason.HeaderValueLineFoldingNotSupported:
ex = new BadHttpRequestException("Header value line folding not supported.", StatusCodes.Status400BadRequest);
break;
case RequestRejectionReason.InvalidRequestLine:
ex = new BadHttpRequestException("Invalid request line.", StatusCodes.Status400BadRequest);
break;
case RequestRejectionReason.MalformedRequestInvalidHeaders:
ex = new BadHttpRequestException("Malformed request: invalid headers.", StatusCodes.Status400BadRequest);
break;

77
src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs
View File

@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
private CancellationTokenSource _abortedCts;
private CancellationToken? _manuallySetRequestAbortToken;
private RequestProcessingStatus _requestProcessingStatus;
protected RequestProcessingStatus _requestProcessingStatus;
protected bool _keepAlive;
protected bool _upgrade;
private bool _canHaveBody;
@ -982,15 +982,46 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
Output.ProducingComplete(end);
}
public void ParseRequest(ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined)
{
consumed = buffer.Start;
examined = buffer.End;
switch (_requestProcessingStatus)
{
case RequestProcessingStatus.RequestPending:
if (buffer.IsEmpty)
{
break;
}
ConnectionControl.ResetTimeout(_requestHeadersTimeoutMilliseconds, TimeoutAction.SendTimeoutResponse);
_requestProcessingStatus = RequestProcessingStatus.ParsingRequestLine;
goto case RequestProcessingStatus.ParsingRequestLine;
case RequestProcessingStatus.ParsingRequestLine:
if (TakeStartLine(buffer, out consumed, out examined))
{
buffer = buffer.Slice(consumed, buffer.End);
_requestProcessingStatus = RequestProcessingStatus.ParsingHeaders;
goto case RequestProcessingStatus.ParsingHeaders;
}
else
{
break;
}
case RequestProcessingStatus.ParsingHeaders:
if (TakeMessageHeaders(buffer, out consumed, out examined))
{
_requestProcessingStatus = RequestProcessingStatus.AppStarted;
}
break;
}
}
public bool TakeStartLine(ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined)
{
if (_requestProcessingStatus == RequestProcessingStatus.RequestPending)
{
ConnectionControl.ResetTimeout(_requestHeadersTimeoutMilliseconds, TimeoutAction.SendTimeoutResponse);
}
_requestProcessingStatus = RequestProcessingStatus.RequestStarted;
var overLength = false;
if (buffer.Length >= ServerOptions.Limits.MaxRequestLineSize)
{
@ -1039,7 +1070,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
return true;
}
public bool TakeMessageHeaders(ReadableBuffer buffer, FrameRequestHeaders requestHeaders, out ReadCursor consumed, out ReadCursor examined)
public bool TakeMessageHeaders(ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined)
{
// Make sure the buffer is limited
bool overLength = false;
@ -1085,7 +1116,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
if (!appCompleted)
{
// Back out of header creation surface exeception in user code
_requestProcessingStatus = RequestProcessingStatus.RequestStarted;
_requestProcessingStatus = RequestProcessingStatus.AppStarted;
throw ex;
}
else
@ -1141,18 +1172,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
public void RejectRequest(RequestRejectionReason reason)
{
RejectRequest(BadHttpRequestException.GetException(reason));
throw BadHttpRequestException.GetException(reason);
}
public void RejectRequest(RequestRejectionReason reason, string value)
{
RejectRequest(BadHttpRequestException.GetException(reason, value));
}
private void RejectRequest(BadHttpRequestException ex)
{
Log.ConnectionBadRequest(ConnectionId, ex);
throw ex;
throw BadHttpRequestException.GetException(reason, value);
}
public void SetBadRequestState(RequestRejectionReason reason)
@ -1162,6 +1187,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
public void SetBadRequestState(BadHttpRequestException ex)
{
Log.ConnectionBadRequest(ConnectionId, ex);
if (!HasResponseStarted)
{
SetErrorResponseHeaders(ex.StatusCode);
@ -1190,20 +1217,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
Log.ApplicationError(ConnectionId, ex);
}
public enum RequestLineStatus
{
Empty,
Incomplete,
Done
}
private enum RequestProcessingStatus
{
RequestPending,
RequestStarted,
ResponseStarted
}
public void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod)
{
// URIs are always encoded/escaped to ASCII https://tools.ietf.org/html/rfc3986#page-11

79
src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs
View File

@ -3,7 +3,6 @@
using System;
using System.IO;
using System.IO.Pipelines;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Hosting.Server;
@ -31,16 +30,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
/// </summary>
public override async Task RequestProcessingAsync()
{
var requestLineStatus = default(RequestLineStatus);
try
{
while (!_requestProcessingStopping)
{
// If writer completes with an error Input.ReadAsyncDispatched would throw and
// this would not be reset to empty. But it's required by ECONNRESET check lower in the method.
requestLineStatus = RequestLineStatus.Empty;
ConnectionControl.SetTimeout(_keepAliveMilliseconds, TimeoutAction.CloseConnection);
while (!_requestProcessingStopping)
@ -49,76 +42,44 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
var examined = result.Buffer.End;
var consumed = result.Buffer.End;
InitializeHeaders();
try
{
if (!result.Buffer.IsEmpty)
{
requestLineStatus = TakeStartLine(result.Buffer, out consumed, out examined)
? RequestLineStatus.Done : RequestLineStatus.Incomplete;
}
else
{
requestLineStatus = RequestLineStatus.Empty;
}
ParseRequest(result.Buffer, out consumed, out examined);
}
catch (InvalidOperationException)
{
throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine);
switch (_requestProcessingStatus)
{
case RequestProcessingStatus.ParsingRequestLine:
throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine);
case RequestProcessingStatus.ParsingHeaders:
throw BadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders);
}
throw;
}
finally
{
Input.Reader.Advance(consumed, examined);
}
if (requestLineStatus == RequestLineStatus.Done)
if (_requestProcessingStatus == RequestProcessingStatus.AppStarted)
{
break;
}
if (result.IsCompleted)
{
if (requestLineStatus == RequestLineStatus.Empty)
switch (_requestProcessingStatus)
{
return;
case RequestProcessingStatus.RequestPending:
return;
case RequestProcessingStatus.ParsingRequestLine:
throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine);
case RequestProcessingStatus.ParsingHeaders:
throw BadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders);
}
RejectRequest(RequestRejectionReason.InvalidRequestLine, requestLineStatus.ToString());
}
}
InitializeHeaders();
while (!_requestProcessingStopping)
{
var result = await Input.Reader.ReadAsync();
var examined = result.Buffer.End;
var consumed = result.Buffer.End;
bool headersDone;
try
{
headersDone = TakeMessageHeaders(result.Buffer, FrameRequestHeaders, out consumed,
out examined);
}
catch (InvalidOperationException)
{
throw BadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders);
}
finally
{
Input.Reader.Advance(consumed, examined);
}
if (headersDone)
{
break;
}
if (result.IsCompleted)
{
RejectRequest(RequestRejectionReason.MalformedRequestInvalidHeaders);
}
}
@ -231,7 +192,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
catch (IOException ex) when (ex.InnerException is UvException)
{
// Don't log ECONNRESET errors made between requests. Browsers like IE will reset connections regularly.
if (requestLineStatus != RequestLineStatus.Empty ||
if (_requestProcessingStatus != RequestProcessingStatus.RequestPending ||
((UvException)ex.InnerException).StatusCode != Constants.ECONNRESET)
{
Log.RequestProcessingError(ConnectionId, ex);

21
src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs
View File

@ -289,7 +289,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
}
else if (ch2 == ByteLF)
{
consumed = reader.Cursor;
// REVIEW: Removed usage of ReadableBufferReader.Cursor, because it's broken when the buffer is
// sliced and doesn't start at the start of a segment. We should probably fix this.
//consumed = reader.Cursor;
consumed = buffer.Move(consumed, 2);
examined = consumed;
return true;
}
@ -323,7 +326,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
// Skip the reader forward past the header line
reader.Skip(span.Length);
consumed = reader.Cursor;
// REVIEW: Removed usage of ReadableBufferReader.Cursor, because it's broken when the buffer is
// sliced and doesn't start at the start of a segment. We should probably fix this.
//consumed = reader.Cursor;
consumed = buffer.Move(consumed, span.Length);
consumedBytes += span.Length;
var nameBuffer = span.Slice(nameStart, nameEnd - nameStart);
@ -562,17 +568,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
public void RejectRequest(RequestRejectionReason reason)
{
RejectRequest(BadHttpRequestException.GetException(reason));
throw BadHttpRequestException.GetException(reason);
}
public void RejectRequest(RequestRejectionReason reason, string value)
{
RejectRequest(BadHttpRequestException.GetException(reason, value));
}
private void RejectRequest(BadHttpRequestException ex)
{
throw ex;
throw BadHttpRequestException.GetException(reason, value);
}
private void RejectRequestLine(Span<byte> span)
@ -608,4 +609,4 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
Complete
}
}
}
}

2
src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs
View File

@ -595,7 +595,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
try
{
if (_context.TakeMessageHeaders(buffer, _requestHeaders, out consumed, out examined))
if (_context.TakeMessageHeaders(buffer, out consumed, out examined))
{
break;
}

14
src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestProcessingStatus.cs Normal file
View File

@ -0,0 +1,14 @@
// 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 RequestProcessingStatus
{
RequestPending,
ParsingRequestLine,
ParsingHeaders,
AppStarted,
ResponseStarted
}
}

6
test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs
View File

@ -6,12 +6,8 @@ using System.IO.Pipelines;
using System.Linq;
using System.Text;
using BenchmarkDotNet.Attributes;
using Microsoft.AspNetCore.Server.Kestrel.Internal;
using Microsoft.AspNetCore.Server.Kestrel.Internal.Http;
using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
using Microsoft.AspNetCore.Testing;
using MemoryPool = Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure.MemoryPool;
using RequestLineStatus = Microsoft.AspNetCore.Server.Kestrel.Internal.Http.Frame.RequestLineStatus;
namespace Microsoft.AspNetCore.Server.Kestrel.Performance
{
@ -154,7 +150,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance
Frame.InitializeHeaders();
if (!Frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)Frame.RequestHeaders, out consumed, out examined))
if (!Frame.TakeMessageHeaders(readableBuffer, out consumed, out examined))
{
ThrowInvalidMessageHeaders();
}

32
test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs
View File

@ -89,7 +89,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header:value\r\n\r\n"));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined);
var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined);
_socketInput.Reader.Advance(_consumed, _examined);
Assert.True(success);
@ -114,7 +114,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined);
var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined);
_socketInput.Reader.Advance(_consumed, _examined);
Assert.True(success);
@ -138,7 +138,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined);
var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined);
_socketInput.Reader.Advance(_consumed, _examined);
Assert.True(success);
@ -161,7 +161,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined);
var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined);
_socketInput.Reader.Advance(_consumed, _examined);
Assert.True(success);
@ -177,7 +177,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var exception = Assert.Throws<BadHttpRequestException>(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined));
var exception = Assert.Throws<BadHttpRequestException>(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined));
_socketInput.Reader.Advance(_consumed, _examined);
Assert.Equal(expectedExceptionMessage, exception.Message);
@ -190,13 +190,13 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header-1: value1\r\n"));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
Assert.False(_frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined));
Assert.False(_frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined));
_socketInput.Reader.Advance(_consumed, _examined);
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(" "));
readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var exception = Assert.Throws<BadHttpRequestException>(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined));
var exception = Assert.Throws<BadHttpRequestException>(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined));
_socketInput.Reader.Advance(_consumed, _examined);
Assert.Equal("Header line must not start with whitespace.", exception.Message);
@ -213,7 +213,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine}\r\n"));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var exception = Assert.Throws<BadHttpRequestException>(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined));
var exception = Assert.Throws<BadHttpRequestException>(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined));
_socketInput.Reader.Advance(_consumed, _examined);
Assert.Equal("Request headers too long.", exception.Message);
@ -229,7 +229,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLines}\r\n"));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var exception = Assert.Throws<BadHttpRequestException>(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined));
var exception = Assert.Throws<BadHttpRequestException>(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined));
_socketInput.Reader.Advance(_consumed, _examined);
Assert.Equal("Request contains too many headers.", exception.Message);
@ -248,7 +248,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined);
var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined);
_socketInput.Reader.Advance(_consumed, _examined);
Assert.True(success);
@ -282,7 +282,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine1}\r\n"));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
var takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined);
var takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined);
_socketInput.Reader.Advance(_consumed, _examined);
Assert.True(takeMessageHeaders);
@ -294,7 +294,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine2}\r\n"));
readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined);
takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined);
_socketInput.Reader.Advance(_consumed, _examined);
Assert.True(takeMessageHeaders);
@ -461,14 +461,14 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
}
[Fact]
public async Task TakeStartLineStartsRequestHeadersTimeoutOnFirstByteAvailable()
public async Task ParseRequestStartsRequestHeadersTimeoutOnFirstByteAvailable()
{
var connectionControl = new Mock<IConnectionControl>();
_connectionContext.ConnectionControl = connectionControl.Object;
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("G"));
_frame.TakeStartLine((await _socketInput.Reader.ReadAsync()).Buffer, out _consumed, out _examined);
_frame.ParseRequest((await _socketInput.Reader.ReadAsync()).Buffer, out _consumed, out _examined);
_socketInput.Reader.Advance(_consumed, _examined);
var expectedRequestHeadersTimeout = (long)_serviceContext.ServerOptions.Limits.RequestHeadersTimeout.TotalMilliseconds;
@ -533,7 +533,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeader));
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
_frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined);
_frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined);
_socketInput.Reader.Advance(_consumed, _examined);
Assert.Equal(readableBuffer.End, _examined);
}
@ -566,7 +566,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
ReadCursor examined;
var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer;
Assert.Equal(false, _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out consumed, out examined));
Assert.Equal(false, _frame.TakeMessageHeaders(readableBuffer, out consumed, out examined));
_socketInput.Reader.Advance(consumed, examined);
}