Skip to content

Use one SequenceReader+Rewind rather than 2 SequenceReaders #8076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 7, 2019
Merged

Use one SequenceReader+Rewind rather than 2 SequenceReaders #8076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/Servers/Kestrel/Core/ref/Microsoft.AspNetCore.Server.Kestrel.Core.netcoreapp3.0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,8 @@ public partial class HttpParser<TRequestHandler> : Microsoft.AspNetCore.Server.K
{
public HttpParser() { }
public HttpParser(bool showErrorDetails) { }
bool Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.IHttpParser<TRequestHandler>.ParseHeaders(TRequestHandler handler, in System.Buffers.ReadOnlySequence<byte> buffer, out System.SequencePosition consumed, out System.SequencePosition examined, out int consumedBytes) { throw null; }
bool Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.IHttpParser<TRequestHandler>.ParseRequestLine(TRequestHandler handler, in System.Buffers.ReadOnlySequence<byte> buffer, out System.SequencePosition consumed, out System.SequencePosition examined) { throw null; }
public bool ParseHeaders(TRequestHandler handler, in System.Buffers.ReadOnlySequence<byte> buffer, out System.SequencePosition consumed, out System.SequencePosition examined, out int consumedBytes) { throw null; }
public bool ParseHeaders(TRequestHandler handler, ref System.Buffers.SequenceReader<byte> reader) { throw null; }
public bool ParseRequestLine(TRequestHandler handler, in System.Buffers.ReadOnlySequence<byte> buffer, out System.SequencePosition consumed, out System.SequencePosition examined) { throw null; }
}
public enum HttpScheme
Expand All @@ -253,7 +252,7 @@ public partial interface IHttpHeadersHandler
}
public partial interface IHttpParser<TRequestHandler> where TRequestHandler : Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.IHttpHeadersHandler, Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.IHttpRequestLineHandler
{
bool ParseHeaders(TRequestHandler handler, in System.Buffers.ReadOnlySequence<byte> buffer, out System.SequencePosition consumed, out System.SequencePosition examined, out int consumedBytes);
bool ParseHeaders(TRequestHandler handler, ref System.Buffers.SequenceReader<byte> reader);
bool ParseRequestLine(TRequestHandler handler, in System.Buffers.ReadOnlySequence<byte> buffer, out System.SequencePosition consumed, out System.SequencePosition examined);
}
public partial interface IHttpRequestLineHandler
Expand Down
77 changes: 61 additions & 16 deletions src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,32 +189,77 @@ public bool TakeStartLine(ReadOnlySequence<byte> buffer, out SequencePosition co
return result;
}

public bool TakeMessageHeaders(ReadOnlySequence<byte> buffer, bool trailers, out SequencePosition consumed, out SequencePosition examined)
public bool TakeMessageHeaders(in ReadOnlySequence<byte> buffer, bool trailers, out SequencePosition consumed, out SequencePosition examined)
{
// Make sure the buffer is limited
bool overLength = false;
if (buffer.Length >= _remainingRequestHeadersBytesAllowed)
if (buffer.Length > _remainingRequestHeadersBytesAllowed)
{
buffer = buffer.Slice(buffer.Start, _remainingRequestHeadersBytesAllowed);

// If we sliced it means the current buffer bigger than what we're
// allowed to look at
overLength = true;
return TrimAndTakeMessageHeaders(buffer, trailers, out consumed, out examined);
}

var result = _parser.ParseHeaders(new Http1ParsingHandler(this, trailers), buffer, out consumed, out examined, out var consumedBytes);
_remainingRequestHeadersBytesAllowed -= consumedBytes;

if (!result && overLength)
var reader = new SequenceReader<byte>(buffer);
var result = false;
try
{
BadHttpRequestException.Throw(RequestRejectionReason.HeadersExceedMaxTotalSize);
result = _parser.ParseHeaders(new Http1ParsingHandler(this, trailers), ref reader);

if (result)
{
TimeoutControl.CancelTimeout();
}

return result;
}
if (result)
finally
{
TimeoutControl.CancelTimeout();
consumed = reader.Position;
_remainingRequestHeadersBytesAllowed -= (int)reader.Consumed;

if (result)
{
examined = consumed;
}
else
{
examined = buffer.End;
}
}

return result;
bool TrimAndTakeMessageHeaders(in ReadOnlySequence<byte> buffer, bool trailers, out SequencePosition consumed, out SequencePosition examined)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The redundancy of TakeMessageHeaders and TrimAndTakeMessageHeaders is one thing I don't love, but I understand doing this if necessary to optimize the fast path.

{
var trimmedBuffer = buffer.Slice(buffer.Start, _remainingRequestHeadersBytesAllowed);

var reader = new SequenceReader<byte>(trimmedBuffer);
var result = false;
try
{
result = _parser.ParseHeaders(new Http1ParsingHandler(this, trailers), ref reader);

if (!result)
{
// We read the maximum allowed but didn't complete the headers.
BadHttpRequestException.Throw(RequestRejectionReason.HeadersExceedMaxTotalSize);
}

TimeoutControl.CancelTimeout();

return result;
}
finally
{
consumed = reader.Position;
_remainingRequestHeadersBytesAllowed -= (int)reader.Consumed;

if (result)
{
examined = consumed;
}
else
{
examined = trimmedBuffer.End;
}
}
}
}

public void OnStartLine(HttpMethod method, HttpVersion version, Span<byte> target, Span<byte> path, Span<byte> query, Span<byte> customMethod, bool pathEncoded)
Expand Down
Loading