Skip to content

Make header parsing "safe" #20562

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 6 commits into from
Apr 13, 2020
Merged

Make header parsing "safe" #20562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b2ac2b4
Make header parsing "safe"
benaadams Apr 5, 2020
d3b1b03
OnHeadersComplete improve speculation and memory ordering (VTune)
benaadams Apr 9, 2020
6892f4a
Remove gotos
benaadams Apr 9, 2020
58d2231
Remove result variable
benaadams Apr 9, 2020
108529f
Remove gotos
benaadams Apr 9, 2020
0ba3a52
Unsafe call through to safe exception
benaadams Apr 9, 2020
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
282 changes: 177 additions & 105 deletions src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ private async Task ProcessRequests<TContext>(IHttpApplication<TContext> applicat
BeginRequestProcessing();

var result = default(ReadResult);
var endConnection = false;
bool endConnection;
do
{
if (BeginRead(out var awaitable))
Expand Down Expand Up @@ -1274,7 +1274,7 @@ public void ThrowRequestTargetRejected(Span<byte> target)
=> throw GetInvalidRequestTargetException(target);

[MethodImpl(MethodImplOptions.NoInlining)]
private BadHttpRequestException GetInvalidRequestTargetException(Span<byte> target)
private BadHttpRequestException GetInvalidRequestTargetException(ReadOnlySpan<byte> target)
=> KestrelBadHttpRequestException.GetException(
RequestRejectionReason.InvalidRequestTarget,
Log.IsEnabled(LogLevel.Information)
Expand Down
20 changes: 12 additions & 8 deletions src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,22 @@ public HttpRequestHeaders(bool reuseHeaderValues = true, bool useLatin1 = false)

public void OnHeadersComplete()
{
var bitsToClear = _previousBits & ~_bits;
var newHeaderFlags = _bits;
var previousHeaderFlags = _previousBits;
_previousBits = 0;

if (bitsToClear != 0)
var headersToClear = (~newHeaderFlags) & previousHeaderFlags;
if (headersToClear == 0)
{
// Some previous headers were not reused or overwritten.

// While they cannot be accessed by the current request (as they were not supplied by it)
// there is no point in holding on to them, so clear them now,
// to allow them to get collected by the GC.
Clear(bitsToClear);
// All headers were resued.
return;
}

// Some previous headers were not reused or overwritten.
// While they cannot be accessed by the current request (as they were not supplied by it)
// there is no point in holding on to them, so clear them now,
// to allow them to get collected by the GC.
Clear(headersToClear);
}

protected override void ClearFast()
Expand Down
2 changes: 1 addition & 1 deletion src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ private static unsafe string GetLatin1StringNonNullCharacters(this ReadOnlySpan<
public static string GetRequestHeaderStringNonNullCharacters(this ReadOnlySpan<byte> span, bool useLatin1) =>
useLatin1 ? GetLatin1StringNonNullCharacters(span) : GetAsciiOrUTF8StringNonNullCharacters(span);

public static string GetAsciiStringEscaped(this Span<byte> span, int maxChars)
public static string GetAsciiStringEscaped(this ReadOnlySpan<byte> span, int maxChars)
{
var sb = new StringBuilder();

Expand Down
25 changes: 25 additions & 0 deletions src/Servers/Kestrel/Core/test/HttpParserTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,31 @@ public void ParseRequestLineTlsOverHttp()
Assert.Equal(RequestRejectionReason.TlsOverHttpError, badHttpRequestException.Reason);
}

[Theory]
[MemberData(nameof(RequestHeaderInvalidData))]
public void ParseHeadersThrowsOnInvalidRequestHeadersWithGratuitouslySplitBuffers(string rawHeaders, string expectedExceptionMessage)
{
var mockTrace = new Mock<IKestrelTrace>();
mockTrace
.Setup(trace => trace.IsEnabled(LogLevel.Information))
.Returns(true);

var parser = CreateParser(mockTrace.Object);
var buffer = BytePerSegmentTestSequenceFactory.Instance.CreateWithContent(rawHeaders);
var requestHandler = new RequestHandler();

#pragma warning disable CS0618 // Type or member is obsolete
var exception = Assert.Throws<BadHttpRequestException>(() =>
#pragma warning restore CS0618 // Type or member is obsolete
{
var reader = new SequenceReader<byte>(buffer);
parser.ParseHeaders(requestHandler, ref reader);
});

Assert.Equal(expectedExceptionMessage, exception.Message);
Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode);
}

[Fact]
public void ParseHeadersWithGratuitouslySplitBuffers()
{
Expand Down
48 changes: 24 additions & 24 deletions src/Servers/Kestrel/shared/test/HttpParsingData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,19 +377,19 @@ public static IEnumerable<string> TargetWithNullCharData
new[] { "Header-1: value1\r\n\tHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"\x09Header-2: value2\x0D\x0A") },

// CR in value
new[] { "Header-1: value1\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0D\x0D\x0A") },
new[] { "Header-1: val\rue1\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: val\x0Due1\x0D\x0A") },
new[] { "Header-1: value1\rHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0DHeader-2: value2\x0D\x0A") },
new[] { "Header-1: value1\r\nHeader-2: value2\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: value2\x0D\x0D\x0A") },
new[] { "Header-1: value1\r\nHeader-2: v\ralue2\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: v\x0Dalue2\x0D\x0A") },
new[] { "Header-1: Value__\rVector16________Vector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value__\x0DVector16________Vector32\x0D\x0A") },
new[] { "Header-1: Value___Vector16\r________Vector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16\x0D________Vector32\x0D\x0A") },
new[] { "Header-1: Value___Vector16_______\rVector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16_______\x0DVector32\x0D\x0A") },
new[] { "Header-1: Value___Vector16________Vector32\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32\x0D\x0D\x0A") },
new[] { "Header-1: Value___Vector16________Vector32_\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32_\x0D\x0D\x0A") },
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16_______\rVector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16_______\x0DVector32\x0D\x0A") },
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\x0D\x0D\x0A") },
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\x0D\x0D\x0A") },
new[] { "Header-1: value1\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0D\x0D") },
new[] { "Header-1: val\rue1\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: val\x0Du") },
new[] { "Header-1: value1\rHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0DH") },
new[] { "Header-1: value1\r\nHeader-2: value2\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: value2\x0D\x0D") },
new[] { "Header-1: value1\r\nHeader-2: v\ralue2\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: v\x0Da") },
new[] { "Header-1: Value__\rVector16________Vector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value__\x0DV") },
new[] { "Header-1: Value___Vector16\r________Vector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16\x0D_") },
new[] { "Header-1: Value___Vector16_______\rVector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16_______\x0DV") },
new[] { "Header-1: Value___Vector16________Vector32\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32\x0D\x0D") },
new[] { "Header-1: Value___Vector16________Vector32_\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32_\x0D\x0D") },
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16_______\rVector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16_______\x0DV") },
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\x0D\x0D") },
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\x0D\x0D") },

// Missing colon
new[] { "Header-1 value1\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1 value1\x0D\x0A") },
Expand All @@ -406,20 +406,20 @@ public static IEnumerable<string> TargetWithNullCharData
// Whitespace in header name
new[] { "Header : value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header : value\x0D\x0A") },
new[] { "Header\t: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header\x09: value\x0D\x0A") },
new[] { "Header\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header\x0D: value\x0D\x0A") },
new[] { "Header_\rVector16: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header_\x0DVector16: value\x0D\x0A") },
new[] { "Header__Vector16\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16\x0D: value\x0D\x0A") },
new[] { "Header__Vector16_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16_\x0D: value\x0D\x0A") },
new[] { "Header_\rVector16________Vector32: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header_\x0DVector16________Vector32: value\x0D\x0A") },
new[] { "Header__Vector16________Vector32\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32\x0D: value\x0D\x0A") },
new[] { "Header__Vector16________Vector32_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32_\x0D: value\x0D\x0A") },
new[] { "Header__Vector16________Vector32Header_\rVector16________Vector32: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header_\x0DVector16________Vector32: value\x0D\x0A") },
new[] { "Header__Vector16________Vector32Header__Vector16________Vector32\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header__Vector16________Vector32\x0D: value\x0D\x0A") },
new[] { "Header__Vector16________Vector32Header__Vector16________Vector32_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header__Vector16________Vector32_\x0D: value\x0D\x0A") },
new[] { "Header\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header\x0D:") },
new[] { "Header_\rVector16: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header_\x0DV") },
new[] { "Header__Vector16\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16\x0D:") },
new[] { "Header__Vector16_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16_\x0D:") },
new[] { "Header_\rVector16________Vector32: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header_\x0DV") },
new[] { "Header__Vector16________Vector32\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32\x0D:") },
new[] { "Header__Vector16________Vector32_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32_\x0D:") },
new[] { "Header__Vector16________Vector32Header_\rVector16________Vector32: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header_\x0DV") },
new[] { "Header__Vector16________Vector32Header__Vector16________Vector32\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header__Vector16________Vector32\x0D:") },
new[] { "Header__Vector16________Vector32Header__Vector16________Vector32_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header__Vector16________Vector32_\x0D:") },
new[] { "Header 1: value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1: value1\x0D\x0A") },
new[] { "Header 1 : value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1 : value1\x0D\x0A") },
new[] { "Header 1\t: value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1\x09: value1\x0D\x0A") },
new[] { "Header 1\r: value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1\x0D: value1\x0D\x0A") },
new[] { "Header 1\r: value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1\x0D:") },
new[] { "Header-1: value1\r\nHeader 2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 2: value2\x0D\x0A") },
new[] { "Header-1: value1\r\nHeader-2 : value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2 : value2\x0D\x0A") },
new[] { "Header-1: value1\r\nHeader-2\t: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2\x09: value2\x0D\x0A") },
Expand Down
44 changes: 44 additions & 0 deletions src/Shared/ServerInfrastructure/BufferExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,50 @@ public static ArraySegment<byte> GetArray(this ReadOnlyMemory<byte> memory)
return result;
}

/// <summary>
/// Returns position of first occurrence of item in the <see cref="ReadOnlySequence{T}"/>
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static SequencePosition? PositionOfAny<T>(in this ReadOnlySequence<T> source, T value0, T value1) where T : IEquatable<T>
{
if (source.IsSingleSegment)
{
int index = source.First.Span.IndexOfAny(value0, value1);
if (index != -1)
{
return source.GetPosition(index);
}

return null;
}
else
{
return PositionOfAnyMultiSegment(source, value0, value1);
}
}

private static SequencePosition? PositionOfAnyMultiSegment<T>(in ReadOnlySequence<T> source, T value0, T value1) where T : IEquatable<T>
{
SequencePosition position = source.Start;
SequencePosition result = position;
while (source.TryGet(ref position, out ReadOnlyMemory<T> memory))
{
int index = memory.Span.IndexOfAny(value0, value1);
if (index != -1)
{
return source.GetPosition(index, result);
}
else if (position.GetObject() == null)
{
break;
}

result = position;
}

return null;
}

internal static void WriteAscii(ref this BufferWriter<PipeWriter> buffer, string data)
{
if (string.IsNullOrEmpty(data))
Expand Down