Skip to content

Add linefeed line terminator switch #41101

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

Closed
wants to merge 2 commits into from
Closed

Add linefeed line terminator switch #41101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ba9084
Add LF request line terminator option
sebastienros May 18, 2022
92888ea
Improve performance
sebastienros May 18, 2022
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
2 changes: 1 addition & 1 deletion src/Servers/Kestrel/Core/src/CoreStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,4 +668,4 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
<data name="Http3ControlStreamErrorInitializingOutbound" xml:space="preserve">
<value>Error initializing outbound control stream.</value>
</data>
</root>
</root>
241 changes: 114 additions & 127 deletions src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
public class HttpParser<TRequestHandler> : IHttpParser<TRequestHandler> where TRequestHandler : IHttpHeadersHandler, IHttpRequestLineHandler
{
private readonly bool _showErrorDetails;
private readonly bool _enableLineFeedTerminator;

public HttpParser() : this(showErrorDetails: true)
public HttpParser() : this(showErrorDetails: true, enableLineFeedTerminator: false)
{
}

public HttpParser(bool showErrorDetails)
public HttpParser(bool showErrorDetails) : this(showErrorDetails, enableLineFeedTerminator: false)
{
}

internal HttpParser(bool showErrorDetails, bool enableLineFeedTerminator)
{
_showErrorDetails = showErrorDetails;
_enableLineFeedTerminator = enableLineFeedTerminator;
}

// byte types don't have a data type annotation so we pre-cast them; to avoid in-place casts
Expand Down Expand Up @@ -126,9 +132,15 @@ private void ParseRequestLine(TRequestHandler handler, ReadOnlySpan<byte> reques

// Version + CR is 9 bytes which should take us to .Length
// LF should have been dropped prior to method call
if ((uint)offset + 9 != (uint)requestLine.Length || requestLine[offset + sizeof(ulong)] != ByteCR)
if ((uint)offset + 9 != (uint)requestLine.Length || requestLine[offset + 8] != ByteCR)
{
RejectRequestLine(requestLine);
// LF should have been dropped prior to method call
// If _enableLineFeedTerminator and offset + 8 is .Length,
// then requestLine is valid since it mean LF was the next char
if (!_enableLineFeedTerminator || (uint)offset + 8 != (uint)requestLine.Length)
{
RejectRequestLine(requestLine);
}
}

// Version
Expand All @@ -152,188 +164,163 @@ public bool ParseHeaders(TRequestHandler handler, ref SequenceReader<byte> reade
while (!reader.End)
{
var span = reader.UnreadSpan;

// Size of header in the current span, if known
var length = -1;

while (span.Length > 0)
{
var ch1 = (byte)0;
var ch2 = (byte)0;
var readAhead = 0;
// The size of the EOL terminator. Always -1 (no valid EOL), 1 (LF) or 2 (CRLF)
var eolSize = -1;

// Fast path, we're still looking at the same span
if (span.Length >= 2)
{
ch1 = span[0];
ch2 = span[1];
}
else if (reader.TryRead(out ch1)) // Possibly split across spans
// length can be set when the span is returned by ParseMultiSpanHeader
if (length == -1)
{
// Note if we read ahead by 1 or 2 bytes
readAhead = (reader.TryRead(out ch2)) ? 2 : 1;
length = span.IndexOfAny(ByteCR, ByteLF);
}

if (ch1 == ByteCR)
if (length != -1)
{
// Check for final CRLF.
if (ch2 == ByteLF)
{
// If we got 2 bytes from the span directly so skip ahead 2 so that
// the reader's state matches what we expect
if (readAhead == 0)
{
reader.Advance(2);
}
// Validate the EOL terminator
eolSize = ParseHeaderLineEnd(span, length);

// Double CRLF found, so end of headers.
handler.OnHeadersComplete(endStream: false);
return true;
}
else if (readAhead == 1)
// Not valid
if (eolSize == -1)
{
// Didn't read 2 bytes, reset the reader so we don't consume anything
reader.Rewind(1);
return false;
length = -1;
}

Debug.Assert(readAhead == 0 || readAhead == 2);
// Headers don't end in CRLF line.

KestrelBadHttpRequestException.Throw(RequestRejectionReason.InvalidRequestHeadersNoCRLF);
}

var length = 0;
// We only need to look for the end if we didn't read ahead; otherwise there isn't enough in
// in the span to contain a header.
if (readAhead == 0)
// Empty header (EOL only)?
if (length == 0)
{
length = span.IndexOfAny(ByteCR, ByteLF);
// If not found length with be -1; casting to uint will turn it to uint.MaxValue
// which will be larger than any possible span.Length. This also serves to eliminate
// the bounds check for the next lookup of span[length]
if ((uint)length < (uint)span.Length)
{
// Early memory read to hide latency
var expectedCR = span[length];
// Correctly has a CR, move to next
length++;

if (expectedCR != ByteCR)
{
// Sequence needs to be CRLF not LF first.
RejectRequestHeader(span[..length]);
}
handler.OnHeadersComplete(endStream: false);
reader.Advance(eolSize);
return true;
}

if ((uint)length < (uint)span.Length)
{
// Early memory read to hide latency
var expectedLF = span[length];
// Correctly has a LF, move to next
length++;

if (expectedLF != ByteLF ||
length < 5 ||
// Exclude the CRLF from the headerLine and parse the header name:value pair
!TryTakeSingleHeader(handler, span[..(length - 2)]))
{
// Sequence needs to be CRLF and not contain an inner CR not part of terminator.
// Less than min possible headerSpan of 5 bytes a:b\r\n
// Not parsable as a valid name:value header pair.
RejectRequestHeader(span[..length]);
}
// If not found length will be -1; casting to uint will turn it to uint.MaxValue
// which will be larger than any possible span.Length. This also serves to eliminate
// the bounds check for the next lookup of span[length]
if ((uint)length < (uint)span.Length)
{
var lineLength = length + eolSize;

// Read the header successfully, skip the reader forward past the headerSpan.
span = span.Slice(length);
reader.Advance(length);
}
else
{
// No enough data, set length to 0.
length = 0;
}
if (length != 0 && !TryTakeSingleHeader(handler, span[..length]))
{
// Sequence needs to be CRLF and not contain an inner CR not part of terminator.
// Not parsable as a valid name:value header pair.
RejectRequestHeader(span[..lineLength]);
}

// Read the header successfully, skip the reader forward past the headerSpan.
span = span[lineLength..];
reader.Advance(lineLength);
}

// End found in current span
if (length > 0)
{
length = -1;
continue;
}

// We moved the reader to look ahead 2 bytes so rewind the reader
if (readAhead > 0)
{
reader.Rewind(readAhead);
}
// Load next header line to parse as a span
span = ParseMultiSpanHeader(ref reader, out length);

length = ParseMultiSpanHeader(handler, ref reader);
if (length < 0)
// If there any remaining line?
if (length == -1 && span.Length == 0)
{
// Not there
return false;
}

reader.Advance(length);
// As we crossed spans set the current span to default
// so we move to the next span on the next iteration
span = default;
}
}

return false;
}

private int ParseMultiSpanHeader(TRequestHandler handler, ref SequenceReader<byte> reader)
// Returns the length of the line terminator (CRLF = 2, LF = 1)
// If no valid EOL is detected then -1
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private int ParseHeaderLineEnd(ReadOnlySpan<byte> headerSpan, int headerLineLength)
{
// This method needs to be called with a positive value representing the index of either CR or LF
Debug.Assert(headerLineLength >= 0);

if (headerSpan[headerLineLength] == ByteCR)
{
// No more chars after CR? Don't consume an incomplete header
if (headerSpan.Length == headerLineLength + 1)
{
return -1;
}

// CR must be followed by LF in all cases
if (headerSpan[headerLineLength + 1] != ByteLF)
{
if (headerLineLength == 0)
{
KestrelBadHttpRequestException.Throw(RequestRejectionReason.InvalidRequestHeadersNoCRLF);
}
else
{
RejectRequestHeader(headerSpan[..(headerLineLength + 2)]);
}
}

return 2;
}

if (_enableLineFeedTerminator)
{
return 1;
}

// LF but not allowed
RejectRequestHeader(headerSpan[..(headerLineLength + 1)]);

return 0;
}

// Returns a span from the remaining sequence until the next valid EOL
private ReadOnlySpan<byte> ParseMultiSpanHeader(ref SequenceReader<byte> reader, out int length)
{
length = -1;

var currentSlice = reader.UnreadSequence;
var lineEndPosition = currentSlice.PositionOfAny(ByteCR, ByteLF);

if (lineEndPosition == null)
{
// Not there.
return -1;
return ReadOnlySpan<byte>.Empty;
}

SequencePosition lineEnd;
ReadOnlySpan<byte> headerSpan;
if (currentSlice.Slice(reader.Position, lineEndPosition.Value).Length == currentSlice.Length - 1)
{
// No enough data, so CRLF can't currently be there.
// However, we need to check the found char is CR and not LF
// However, we need to check the found char is CR and not LF (unless quirk mode)

// Advance 1 to include CR/LF in lineEnd
lineEnd = currentSlice.GetPosition(1, lineEndPosition.Value);
headerSpan = currentSlice.Slice(reader.Position, lineEnd).ToSpan();
if (headerSpan[^1] != ByteCR)

if (headerSpan[^1] == ByteLF)
{
RejectRequestHeader(headerSpan);
length = headerSpan.Length - 1;
return headerSpan;
}
return -1;

return ReadOnlySpan<byte>.Empty;
}

// Advance 2 to include CR{LF?} in lineEnd
lineEnd = currentSlice.GetPosition(2, lineEndPosition.Value);
headerSpan = currentSlice.Slice(reader.Position, lineEnd).ToSpan();

if (headerSpan.Length < 5)
{
// Less than min possible headerSpan is 5 bytes a:b\r\n
RejectRequestHeader(headerSpan);
}

if (headerSpan[^2] != ByteCR)
{
// Sequence needs to be CRLF not LF first.
RejectRequestHeader(headerSpan[..^1]);
}

if (headerSpan[^1] != ByteLF ||
// Exclude the CRLF from the headerLine and parse the header name:value pair
!TryTakeSingleHeader(handler, headerSpan[..^2]))
{
// Sequence needs to be CRLF and not contain an inner CR not part of terminator.
// Not parsable as a valid name:value header pair.
RejectRequestHeader(headerSpan);
}

return headerSpan.Length;
length = headerSpan.Length - 2;
return headerSpan;
}

private static bool TryTakeSingleHeader(TRequestHandler handler, ReadOnlySpan<byte> headerLine)
Expand Down
2 changes: 1 addition & 1 deletion src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private static ServiceContext CreateServiceContext(IOptions<KestrelServerOptions
{
Log = trace,
Scheduler = PipeScheduler.ThreadPool,
HttpParser = new HttpParser<Http1ParsingHandler>(trace.IsEnabled(LogLevel.Information)),
HttpParser = new HttpParser<Http1ParsingHandler>(trace.IsEnabled(LogLevel.Information), serverOptions.EnableLineFeedTerminator),
SystemClock = heartbeatManager,
DateHeaderValueManager = dateHeaderValueManager,
ConnectionManager = connectionManager,
Expand Down
15 changes: 15 additions & 0 deletions src/Servers/Kestrel/Core/src/KestrelServerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,21 @@ internal bool EnableInsecureAbsoluteFormHostOverride
/// </summary>
internal bool IsDevCertLoaded { get; set; }

private bool? _enableLineFeedTerminator;
internal bool EnableLineFeedTerminator
{
get
{
if (!_enableLineFeedTerminator.HasValue)
{
_enableLineFeedTerminator = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.EnableLineFeedTerminator", out var enabled) && enabled;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
_enableLineFeedTerminator = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.EnableLineFeedTerminator", out var enabled) && enabled;
_allowHttp1LineFeedTerminators = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.AllowHttp1LineFeedTerminators", out var enabled) && enabled;

}

return _enableLineFeedTerminator.Value;
}
set => _enableLineFeedTerminator = value;
}

/// <summary>
/// Specifies a configuration Action to run for each newly created endpoint. Calling this again will replace
/// the prior action.
Expand Down
Loading