Skip to content
Merged
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
32 changes: 32 additions & 0 deletions src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,28 @@ 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 _allowSpaceAfterRequestLine;

public HttpParser() : this(showErrorDetails: true)
{
}

public HttpParser(bool showErrorDetails)
: this (showErrorDetails, CheckAllowSpaceAfterRequestLine())
{
}

internal HttpParser(bool showErrorDetails, bool allowSpaceAfterRequestLine)
{
_showErrorDetails = showErrorDetails;
_allowSpaceAfterRequestLine = allowSpaceAfterRequestLine;
}

private static bool CheckAllowSpaceAfterRequestLine()
{
// This mitigation is temporary and 6.0 specific, we do not anticipate porting this feature to later versions.
AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.AllowSpaceAfterRequestLine", out var allowSpaceAfterRequestLine);
return allowSpaceAfterRequestLine;
}

// byte types don't have a data type annotation so we pre-cast them; to avoid in-place casts
Expand All @@ -48,7 +62,25 @@ public bool ParseRequestLine(TRequestHandler handler, ref SequenceReader<byte> r

if (reader.TryReadTo(out ReadOnlySpan<byte> requestLine, ByteLF, advancePastDelimiter: true))
{
if (_allowSpaceAfterRequestLine)
{
// Skip a space after the request line
if (reader.TryPeek(out byte s) && s == ByteSpace)
Copy link
Member

Choose a reason for hiding this comment

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

One space?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, from the reports we think that's all we need.

{
reader.Advance(1);
}
// Don't parse the request line until we've started receiving headers.
// Need to make sure we skipped an extra space if present.
else if (reader.End)
{
// Reset state so we can try again.
reader.Rewind(requestLine.Length + 1);
return false;
}
}

ParseRequestLine(handler, requestLine);

return true;
}

Expand Down
25 changes: 24 additions & 1 deletion src/Servers/Kestrel/Core/test/HttpParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,28 @@ public void ParsesRequestLine(
Assert.True(buffer.Slice(examined).IsEmpty);
}

[Fact]
public void ParsesRequestLineWithTrailingSpace()
{
var parser = CreateParser(_nullTrace, allowSpaceAfterRequestLine: true);
var buffer = new ReadOnlySequence<byte>(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n "));
var requestHandler = new RequestHandler();

Assert.False(ParseRequestLine(parser, requestHandler, buffer.Slice(0, buffer.Length - 1), out var consumed, out var examined));
Assert.True(buffer.Slice(0, consumed).IsEmpty);
Assert.Equal(buffer.Length - 1, buffer.Slice(0, examined).Length);

Assert.True(ParseRequestLine(parser, requestHandler, buffer, out consumed, out examined));
Assert.True(buffer.Slice(consumed).IsEmpty);
Assert.True(buffer.Slice(examined).IsEmpty);

Assert.Equal(HttpMethods.Get, requestHandler.Method);
Assert.Equal("HTTP/1.1", requestHandler.Version);
Assert.Equal("/", requestHandler.RawTarget);
Assert.Equal("/", requestHandler.RawPath);
Assert.Equal("HTTP/1.1", requestHandler.Version);
}

[Theory]
[MemberData(nameof(RequestLineIncompleteData))]
public void ParseRequestLineReturnsFalseWhenGivenIncompleteRequestLines(string requestLine)
Expand Down Expand Up @@ -536,7 +558,8 @@ private void VerifyRawHeaders(string rawHeaders, IEnumerable<string> expectedHea
Assert.True(buffer.Slice(reader.Position).IsEmpty);
}

private IHttpParser<RequestHandler> CreateParser(IKestrelTrace log) => new HttpParser<RequestHandler>(log.IsEnabled(LogLevel.Information));
private IHttpParser<RequestHandler> CreateParser(IKestrelTrace log, bool allowSpaceAfterRequestLine = false)
=> new HttpParser<RequestHandler>(log.IsEnabled(LogLevel.Information), allowSpaceAfterRequestLine);

public static IEnumerable<object[]> RequestLineValidData => HttpParsingData.RequestLineValidData;

Expand Down