Skip to content

Add linefeed terminator support #41269

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.

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 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
149 changes: 110 additions & 39 deletions src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,30 @@ 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 _allowLineFeedTerminator;

/// <summary>
/// This API supports framework infrastructure and is not intended to be used
/// directly from application code.
/// </summary>
public HttpParser() : this(showErrorDetails: true)
public HttpParser() : this(showErrorDetails: true, allowLineFeedTerminator: false)
{
}

/// <summary>
/// This API supports framework infrastructure and is not intended to be used
/// directly from application code.
/// </summary>
public HttpParser(bool showErrorDetails)
public HttpParser(bool showErrorDetails) : this(showErrorDetails, allowLineFeedTerminator: false)
{
_showErrorDetails = showErrorDetails;
}

internal HttpParser(bool showErrorDetails, bool allowLineFeedTerminator)
{
_showErrorDetails = showErrorDetails;
_allowLineFeedTerminator = allowLineFeedTerminator;
}

// byte types don't have a data type annotation so we pre-cast them; to avoid in-place casts
private const byte ByteCR = (byte)'\r';
private const byte ByteLF = (byte)'\n';
Expand Down Expand Up @@ -141,11 +147,35 @@ private void ParseRequestLine(TRequestHandler handler, ReadOnlySpan<byte> reques
// Consume space
offset++;

// 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 (!_allowLineFeedTerminator)
{
RejectRequestLine(requestLine);
// 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 + 8] != ByteCR)
{
RejectRequestLine(requestLine);
}
}
else
{
// LF should have been dropped prior to method call
// If offset + 8 is .Length then requestLine is valid since it mean LF was the next char
if ((uint)offset + 8 != (uint)requestLine.Length)
{
// Version + CR is 9 bytes which should take us to .Length
if ((uint)offset + 9 != (uint)requestLine.Length || requestLine[offset + 8] != ByteCR)
{
RejectRequestLine(requestLine);
}
}
else
{
// e.g., GET / HTTP1.\r\n
if (requestLine[offset + 7] == ByteCR)
{
RejectRequestLine(requestLine);
}
}
}

// Version
Expand Down Expand Up @@ -188,7 +218,7 @@ public bool ParseHeaders(TRequestHandler handler, ref SequenceReader<byte> reade
else if (reader.TryRead(out ch1)) // Possibly split across spans
{
// Note if we read ahead by 1 or 2 bytes
readAhead = (reader.TryRead(out ch2)) ? 2 : 1;
readAhead = reader.TryRead(out ch2) ? 2 : 1;
}

if (ch1 == ByteCR)
Expand Down Expand Up @@ -231,44 +261,60 @@ public bool ParseHeaders(TRequestHandler handler, ref SequenceReader<byte> reade
// 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++;
var headerSpan = span[..length];

if (expectedCR != ByteCR)
if (length < 4)
{
// Sequence needs to be CRLF not LF first.
RejectRequestHeader(span[..length]);
// Less than min possible headerSpan of 3 bytes a:b
RejectRequestHeader(headerSpan);
}

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

if (lineTerminator == ByteLF)
{
// 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)]))
if (!_allowLineFeedTerminator)
{
// 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.
// Sequence needs to be CRLF not LF first.
RejectRequestHeader(span[..length]);
}

// 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 ((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)
{
// Sequence needs to be CRLF not LF first.
RejectRequestHeader(span[..length]);
}
}
else
{
// No enough data, set length to 0.
length = 0;
}
}

if (length != 0 && !TryTakeSingleHeader(handler, headerSpan))
{
// 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[..length]);
}

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

Expand Down Expand Up @@ -317,14 +363,17 @@ private int ParseMultiSpanHeader(TRequestHandler handler, ref SequenceReader<byt
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)
{
RejectRequestHeader(headerSpan);
if (!_allowLineFeedTerminator || headerSpan[^1] != ByteLF)
{
RejectRequestHeader(headerSpan);
}
}
return -1;
}
Expand All @@ -335,14 +384,36 @@ private int ParseMultiSpanHeader(TRequestHandler handler, ref SequenceReader<byt

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

if (headerSpan[^2] != ByteCR)
{
// Sequence needs to be CRLF not LF first.
RejectRequestHeader(headerSpan[..^1]);
if (!_allowLineFeedTerminator)
{
// Sequence needs to be CRLF.
RejectRequestHeader(headerSpan[..^1]);
}
else if (headerSpan[^2] != ByteLF)
{
RejectRequestHeader(headerSpan[..^1]);
}
else
{
// Line ends with LF and quirk mode is enabled, try to parse the header
if (!TryTakeSingleHeader(handler, headerSpan[..^1]))
{
// Not parsable as a valid name:value header pair.
RejectRequestHeader(headerSpan);
}

// Omit last byte since the header is a valid LF terminated line
return headerSpan.Length - 1;
}
}

if (headerSpan[^1] != ByteLF ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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.AllowLineFeedTerminator),
SystemClock = heartbeatManager,
DateHeaderValueManager = dateHeaderValueManager,
ConnectionManager = connectionManager,
Expand Down
9 changes: 9 additions & 0 deletions src/Servers/Kestrel/Core/src/KestrelServerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ public class KestrelServerOptions
/// </remarks>
public bool DisableStringReuse { get; set; }

/// <summary>
/// Gets or sets a value that controls whether the request lines
/// can end with LF only instead of CR/LF.
/// </summary>
/// <remarks>
/// Defaults to false.
/// </remarks>
public bool AllowLineFeedTerminator { get; set; }

/// <summary>
/// Controls whether to return the "Alt-Svc" header from an HTTP/2 or lower response for HTTP/3.
/// </summary>
Expand Down
2 changes: 2 additions & 0 deletions src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#nullable enable
*REMOVED*~Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer.KestrelServer(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions!>! options, Microsoft.AspNetCore.Connections.IConnectionListenerFactory! transportFactory, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void
Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer.KestrelServer(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions!>! options, Microsoft.AspNetCore.Connections.IConnectionListenerFactory! transportFactory, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void
Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.AllowLineFeedTerminator.get -> bool
Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.AllowLineFeedTerminator.set -> void
Loading