Skip to content

Commit 6016e70

Browse files
benaadamsbenaadams
authored
Make header parsing "safe" (#20562)
* OnHeadersComplete improve speculation and memory ordering (VTune)
1 parent e3d3da3 commit 6016e70

File tree

7 files changed

+285
-140
lines changed

7 files changed

+285
-140
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs

Lines changed: 177 additions & 105 deletions
Large diffs are not rendered by default.

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ private async Task ProcessRequests<TContext>(IHttpApplication<TContext> applicat
603603
BeginRequestProcessing();
604604

605605
var result = default(ReadResult);
606-
var endConnection = false;
606+
bool endConnection;
607607
do
608608
{
609609
if (BeginRead(out var awaitable))
@@ -1274,7 +1274,7 @@ public void ThrowRequestTargetRejected(Span<byte> target)
12741274
=> throw GetInvalidRequestTargetException(target);
12751275

12761276
[MethodImpl(MethodImplOptions.NoInlining)]
1277-
private BadHttpRequestException GetInvalidRequestTargetException(Span<byte> target)
1277+
private BadHttpRequestException GetInvalidRequestTargetException(ReadOnlySpan<byte> target)
12781278
=> KestrelBadHttpRequestException.GetException(
12791279
RequestRejectionReason.InvalidRequestTarget,
12801280
Log.IsEnabled(LogLevel.Information)

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,22 @@ public HttpRequestHeaders(bool reuseHeaderValues = true, bool useLatin1 = false)
2727

2828
public void OnHeadersComplete()
2929
{
30-
var bitsToClear = _previousBits & ~_bits;
30+
var newHeaderFlags = _bits;
31+
var previousHeaderFlags = _previousBits;
3132
_previousBits = 0;
3233

33-
if (bitsToClear != 0)
34+
var headersToClear = (~newHeaderFlags) & previousHeaderFlags;
35+
if (headersToClear == 0)
3436
{
35-
// Some previous headers were not reused or overwritten.
36-
37-
// While they cannot be accessed by the current request (as they were not supplied by it)
38-
// there is no point in holding on to them, so clear them now,
39-
// to allow them to get collected by the GC.
40-
Clear(bitsToClear);
37+
// All headers were resued.
38+
return;
4139
}
40+
41+
// Some previous headers were not reused or overwritten.
42+
// While they cannot be accessed by the current request (as they were not supplied by it)
43+
// there is no point in holding on to them, so clear them now,
44+
// to allow them to get collected by the GC.
45+
Clear(headersToClear);
4246
}
4347

4448
protected override void ClearFast()

‎src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ private static unsafe string GetLatin1StringNonNullCharacters(this ReadOnlySpan<
221221
public static string GetRequestHeaderStringNonNullCharacters(this ReadOnlySpan<byte> span, bool useLatin1) =>
222222
useLatin1 ? GetLatin1StringNonNullCharacters(span) : GetAsciiOrUTF8StringNonNullCharacters(span);
223223

224-
public static string GetAsciiStringEscaped(this Span<byte> span, int maxChars)
224+
public static string GetAsciiStringEscaped(this ReadOnlySpan<byte> span, int maxChars)
225225
{
226226
var sb = new StringBuilder();
227227

‎src/Servers/Kestrel/Core/test/HttpParserTests.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,31 @@ public void ParseRequestLineTlsOverHttp()
429429
Assert.Equal(RequestRejectionReason.TlsOverHttpError, badHttpRequestException.Reason);
430430
}
431431

432+
[Theory]
433+
[MemberData(nameof(RequestHeaderInvalidData))]
434+
public void ParseHeadersThrowsOnInvalidRequestHeadersWithGratuitouslySplitBuffers(string rawHeaders, string expectedExceptionMessage)
435+
{
436+
var mockTrace = new Mock<IKestrelTrace>();
437+
mockTrace
438+
.Setup(trace => trace.IsEnabled(LogLevel.Information))
439+
.Returns(true);
440+
441+
var parser = CreateParser(mockTrace.Object);
442+
var buffer = BytePerSegmentTestSequenceFactory.Instance.CreateWithContent(rawHeaders);
443+
var requestHandler = new RequestHandler();
444+
445+
#pragma warning disable CS0618 // Type or member is obsolete
446+
var exception = Assert.Throws<BadHttpRequestException>(() =>
447+
#pragma warning restore CS0618 // Type or member is obsolete
448+
{
449+
var reader = new SequenceReader<byte>(buffer);
450+
parser.ParseHeaders(requestHandler, ref reader);
451+
});
452+
453+
Assert.Equal(expectedExceptionMessage, exception.Message);
454+
Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode);
455+
}
456+
432457
[Fact]
433458
public void ParseHeadersWithGratuitouslySplitBuffers()
434459
{

‎src/Servers/Kestrel/shared/test/HttpParsingData.cs

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -377,19 +377,19 @@ public static IEnumerable<string> TargetWithNullCharData
377377
new[] { "Header-1: value1\r\n\tHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"\x09Header-2: value2\x0D\x0A") },
378378

379379
// CR in value
380-
new[] { "Header-1: value1\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0D\x0D\x0A") },
381-
new[] { "Header-1: val\rue1\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: val\x0Due1\x0D\x0A") },
382-
new[] { "Header-1: value1\rHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0DHeader-2: value2\x0D\x0A") },
383-
new[] { "Header-1: value1\r\nHeader-2: value2\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: value2\x0D\x0D\x0A") },
384-
new[] { "Header-1: value1\r\nHeader-2: v\ralue2\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: v\x0Dalue2\x0D\x0A") },
385-
new[] { "Header-1: Value__\rVector16________Vector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value__\x0DVector16________Vector32\x0D\x0A") },
386-
new[] { "Header-1: Value___Vector16\r________Vector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16\x0D________Vector32\x0D\x0A") },
387-
new[] { "Header-1: Value___Vector16_______\rVector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16_______\x0DVector32\x0D\x0A") },
388-
new[] { "Header-1: Value___Vector16________Vector32\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32\x0D\x0D\x0A") },
389-
new[] { "Header-1: Value___Vector16________Vector32_\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32_\x0D\x0D\x0A") },
390-
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16_______\rVector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16_______\x0DVector32\x0D\x0A") },
391-
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") },
392-
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") },
380+
new[] { "Header-1: value1\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0D\x0D") },
381+
new[] { "Header-1: val\rue1\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: val\x0Du") },
382+
new[] { "Header-1: value1\rHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: value1\x0DH") },
383+
new[] { "Header-1: value1\r\nHeader-2: value2\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: value2\x0D\x0D") },
384+
new[] { "Header-1: value1\r\nHeader-2: v\ralue2\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2: v\x0Da") },
385+
new[] { "Header-1: Value__\rVector16________Vector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value__\x0DV") },
386+
new[] { "Header-1: Value___Vector16\r________Vector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16\x0D_") },
387+
new[] { "Header-1: Value___Vector16_______\rVector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16_______\x0DV") },
388+
new[] { "Header-1: Value___Vector16________Vector32\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32\x0D\x0D") },
389+
new[] { "Header-1: Value___Vector16________Vector32_\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32_\x0D\x0D") },
390+
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16_______\rVector32\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16_______\x0DV") },
391+
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\x0D\x0D") },
392+
new[] { "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\r\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\x0D\x0D") },
393393

394394
// Missing colon
395395
new[] { "Header-1 value1\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-1 value1\x0D\x0A") },
@@ -406,20 +406,20 @@ public static IEnumerable<string> TargetWithNullCharData
406406
// Whitespace in header name
407407
new[] { "Header : value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header : value\x0D\x0A") },
408408
new[] { "Header\t: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header\x09: value\x0D\x0A") },
409-
new[] { "Header\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header\x0D: value\x0D\x0A") },
410-
new[] { "Header_\rVector16: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header_\x0DVector16: value\x0D\x0A") },
411-
new[] { "Header__Vector16\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16\x0D: value\x0D\x0A") },
412-
new[] { "Header__Vector16_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16_\x0D: value\x0D\x0A") },
413-
new[] { "Header_\rVector16________Vector32: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header_\x0DVector16________Vector32: value\x0D\x0A") },
414-
new[] { "Header__Vector16________Vector32\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32\x0D: value\x0D\x0A") },
415-
new[] { "Header__Vector16________Vector32_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32_\x0D: value\x0D\x0A") },
416-
new[] { "Header__Vector16________Vector32Header_\rVector16________Vector32: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header_\x0DVector16________Vector32: value\x0D\x0A") },
417-
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") },
418-
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") },
409+
new[] { "Header\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header\x0D:") },
410+
new[] { "Header_\rVector16: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header_\x0DV") },
411+
new[] { "Header__Vector16\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16\x0D:") },
412+
new[] { "Header__Vector16_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16_\x0D:") },
413+
new[] { "Header_\rVector16________Vector32: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header_\x0DV") },
414+
new[] { "Header__Vector16________Vector32\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32\x0D:") },
415+
new[] { "Header__Vector16________Vector32_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32_\x0D:") },
416+
new[] { "Header__Vector16________Vector32Header_\rVector16________Vector32: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header_\x0DV") },
417+
new[] { "Header__Vector16________Vector32Header__Vector16________Vector32\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header__Vector16________Vector32\x0D:") },
418+
new[] { "Header__Vector16________Vector32Header__Vector16________Vector32_\r: value\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header__Vector16________Vector32Header__Vector16________Vector32_\x0D:") },
419419
new[] { "Header 1: value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1: value1\x0D\x0A") },
420420
new[] { "Header 1 : value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1 : value1\x0D\x0A") },
421421
new[] { "Header 1\t: value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1\x09: value1\x0D\x0A") },
422-
new[] { "Header 1\r: value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1\x0D: value1\x0D\x0A") },
422+
new[] { "Header 1\r: value1\r\nHeader-2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 1\x0D:") },
423423
new[] { "Header-1: value1\r\nHeader 2: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header 2: value2\x0D\x0A") },
424424
new[] { "Header-1: value1\r\nHeader-2 : value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2 : value2\x0D\x0A") },
425425
new[] { "Header-1: value1\r\nHeader-2\t: value2\r\n\r\n", CoreStrings.FormatBadRequest_InvalidRequestHeader_Detail(@"Header-2\x09: value2\x0D\x0A") },

‎src/Shared/ServerInfrastructure/BufferExtensions.cs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,50 @@ public static ArraySegment<byte> GetArray(this ReadOnlyMemory<byte> memory)
4040
return result;
4141
}
4242

43+
/// <summary>
44+
/// Returns position of first occurrence of item in the <see cref="ReadOnlySequence{T}"/>
45+
/// </summary>
46+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
47+
public static SequencePosition? PositionOfAny<T>(in this ReadOnlySequence<T> source, T value0, T value1) where T : IEquatable<T>
48+
{
49+
if (source.IsSingleSegment)
50+
{
51+
int index = source.First.Span.IndexOfAny(value0, value1);
52+
if (index != -1)
53+
{
54+
return source.GetPosition(index);
55+
}
56+
57+
return null;
58+
}
59+
else
60+
{
61+
return PositionOfAnyMultiSegment(source, value0, value1);
62+
}
63+
}
64+
65+
private static SequencePosition? PositionOfAnyMultiSegment<T>(in ReadOnlySequence<T> source, T value0, T value1) where T : IEquatable<T>
66+
{
67+
SequencePosition position = source.Start;
68+
SequencePosition result = position;
69+
while (source.TryGet(ref position, out ReadOnlyMemory<T> memory))
70+
{
71+
int index = memory.Span.IndexOfAny(value0, value1);
72+
if (index != -1)
73+
{
74+
return source.GetPosition(index, result);
75+
}
76+
else if (position.GetObject() == null)
77+
{
78+
break;
79+
}
80+
81+
result = position;
82+
}
83+
84+
return null;
85+
}
86+
4387
internal static void WriteAscii(ref this BufferWriter<PipeWriter> buffer, string data)
4488
{
4589
if (string.IsNullOrEmpty(data))

0 commit comments

Comments
 (0)