Skip to content

Commit 7672ca9

Browse files
JamesNKJamesNK
authored
HTTP/2: Improve incoming header performance (#38834)
1 parent d97c348 commit 7672ca9

File tree

13 files changed

+200
-125
lines changed

13 files changed

+200
-125
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7714,12 +7714,11 @@ public unsafe void Append(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value, boo
77147714
}
77157715

77167716
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
7717-
public unsafe bool TryHPackAppend(int index, ReadOnlySpan<byte> value)
7717+
public unsafe bool TryHPackAppend(int index, ReadOnlySpan<byte> value, bool checkForNewlineChars)
77187718
{
77197719
ref StringValues values = ref Unsafe.AsRef<StringValues>(null);
77207720
var nameStr = string.Empty;
77217721
var flag = 0L;
7722-
var checkForNewlineChars = true;
77237722

77247723
// Does the HPack static index match any "known" headers
77257724
switch (index)

‎src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs‎

Lines changed: 86 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,28 +1265,41 @@ private void UpdateConnectionState()
12651265

12661266
public void OnHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
12671267
{
1268-
OnHeaderCore(index: null, indexedValue: false, name, value);
1268+
OnHeaderCore(HeaderType.NameAndValue, staticTableIndex: null, name, value);
1269+
}
1270+
1271+
public void OnDynamicIndexedHeader(int? index, ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
1272+
{
1273+
OnHeaderCore(HeaderType.Dynamic, index, name, value);
12691274
}
12701275

12711276
public void OnStaticIndexedHeader(int index)
12721277
{
12731278
Debug.Assert(index <= H2StaticTable.Count);
12741279

12751280
ref readonly var entry = ref H2StaticTable.Get(index - 1);
1276-
OnHeaderCore(index, indexedValue: true, entry.Name, entry.Value);
1281+
OnHeaderCore(HeaderType.Static, index, entry.Name, entry.Value);
12771282
}
12781283

12791284
public void OnStaticIndexedHeader(int index, ReadOnlySpan<byte> value)
12801285
{
12811286
Debug.Assert(index <= H2StaticTable.Count);
12821287

1283-
OnHeaderCore(index, indexedValue: false, H2StaticTable.Get(index - 1).Name, value);
1288+
OnHeaderCore(HeaderType.StaticAndValue, index, H2StaticTable.Get(index - 1).Name, value);
1289+
}
1290+
1291+
private enum HeaderType
1292+
{
1293+
Static,
1294+
StaticAndValue,
1295+
Dynamic,
1296+
NameAndValue
12841297
}
12851298

12861299
// We can't throw a Http2StreamErrorException here, it interrupts the header decompression state and may corrupt subsequent header frames on other streams.
12871300
// For now these either need to be connection errors or BadRequests. If we want to downgrade any of them to stream errors later then we need to
12881301
// rework the flow so that the remaining headers are drained and the decompression state is maintained.
1289-
private void OnHeaderCore(int? index, bool indexedValue, ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
1302+
private void OnHeaderCore(HeaderType headerType, int? staticTableIndex, ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
12901303
{
12911304
Debug.Assert(_currentHeadersStream != null);
12921305

@@ -1298,32 +1311,59 @@ private void OnHeaderCore(int? index, bool indexedValue, ReadOnlySpan<byte> name
12981311
throw new Http2ConnectionErrorException(CoreStrings.BadRequest_HeadersExceedMaxTotalSize, Http2ErrorCode.PROTOCOL_ERROR);
12991312
}
13001313

1301-
if (index != null)
1302-
{
1303-
ValidateStaticHeader(index.Value, value);
1304-
}
1305-
else
1306-
{
1307-
ValidateHeader(name, value);
1308-
}
1309-
13101314
try
13111315
{
13121316
if (_requestHeaderParsingState == RequestHeaderParsingState.Trailers)
13131317
{
1318+
// Just use name + value bytes and do full validation for request trailers.
1319+
// Potential performance improvement here to check for indexed headers and optimize validation.
1320+
UpdateHeaderParsingState(value, GetPseudoHeaderField(name));
1321+
ValidateHeaderContent(name, value);
1322+
13141323
_currentHeadersStream.OnTrailer(name, value);
13151324
}
13161325
else
13171326
{
13181327
// Throws BadRequest for header count limit breaches.
13191328
// Throws InvalidOperation for bad encoding.
1320-
if (index != null)
1329+
switch (headerType)
13211330
{
1322-
_currentHeadersStream.OnHeader(index.Value, indexedValue, name, value);
1323-
}
1324-
else
1325-
{
1326-
_currentHeadersStream.OnHeader(name, value, checkForNewlineChars : true);
1331+
case HeaderType.Static:
1332+
UpdateHeaderParsingState(value, GetPseudoHeaderField(staticTableIndex.GetValueOrDefault()));
1333+
1334+
_currentHeadersStream.OnHeader(staticTableIndex.GetValueOrDefault(), indexedValue: true, name, value);
1335+
break;
1336+
case HeaderType.StaticAndValue:
1337+
UpdateHeaderParsingState(value, GetPseudoHeaderField(staticTableIndex.GetValueOrDefault()));
1338+
1339+
// Value is new will get validated (i.e. check value doesn't contain newlines)
1340+
_currentHeadersStream.OnHeader(staticTableIndex.GetValueOrDefault(), indexedValue: false, name, value);
1341+
break;
1342+
case HeaderType.Dynamic:
1343+
// It is faster to set a header using a static table index than a name.
1344+
if (staticTableIndex != null)
1345+
{
1346+
UpdateHeaderParsingState(value, GetPseudoHeaderField(staticTableIndex.GetValueOrDefault()));
1347+
1348+
_currentHeadersStream.OnHeader(staticTableIndex.GetValueOrDefault(), indexedValue: true, name, value);
1349+
}
1350+
else
1351+
{
1352+
UpdateHeaderParsingState(value, GetPseudoHeaderField(name));
1353+
1354+
_currentHeadersStream.OnHeader(name, value, checkForNewlineChars: false);
1355+
}
1356+
break;
1357+
case HeaderType.NameAndValue:
1358+
UpdateHeaderParsingState(value, GetPseudoHeaderField(name));
1359+
1360+
// Header and value are new and will get validated (i.e. check name is lower-case, check value doesn't contain newlines)
1361+
ValidateHeaderContent(name, value);
1362+
_currentHeadersStream.OnHeader(name, value, checkForNewlineChars: true);
1363+
break;
1364+
default:
1365+
Debug.Fail($"Unexpected header type: {headerType}");
1366+
break;
13271367
}
13281368
}
13291369
}
@@ -1340,12 +1380,8 @@ private void OnHeaderCore(int? index, bool indexedValue, ReadOnlySpan<byte> name
13401380
public void OnHeadersComplete(bool endStream)
13411381
=> _currentHeadersStream!.OnHeadersComplete();
13421382

1343-
private void ValidateHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
1383+
private void ValidateHeaderContent(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
13441384
{
1345-
// Clients will normally send pseudo headers as an indexed header.
1346-
// Because pseudo headers can still be sent by name we need to check for them.
1347-
UpdateHeaderParsingState(value, GetPseudoHeaderField(name));
1348-
13491385
if (IsConnectionSpecificHeaderField(name, value))
13501386
{
13511387
throw new Http2ConnectionErrorException(CoreStrings.HttpErrorConnectionSpecificHeaderField, Http2ErrorCode.PROTOCOL_ERROR);
@@ -1369,33 +1405,6 @@ private void ValidateHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
13691405
}
13701406
}
13711407

1372-
private void ValidateStaticHeader(int index, ReadOnlySpan<byte> value)
1373-
{
1374-
var headerField = index switch
1375-
{
1376-
1 => PseudoHeaderFields.Authority,
1377-
2 => PseudoHeaderFields.Method,
1378-
3 => PseudoHeaderFields.Method,
1379-
4 => PseudoHeaderFields.Path,
1380-
5 => PseudoHeaderFields.Path,
1381-
6 => PseudoHeaderFields.Scheme,
1382-
7 => PseudoHeaderFields.Scheme,
1383-
8 => PseudoHeaderFields.Status,
1384-
9 => PseudoHeaderFields.Status,
1385-
10 => PseudoHeaderFields.Status,
1386-
11 => PseudoHeaderFields.Status,
1387-
12 => PseudoHeaderFields.Status,
1388-
13 => PseudoHeaderFields.Status,
1389-
14 => PseudoHeaderFields.Status,
1390-
_ => PseudoHeaderFields.None
1391-
};
1392-
1393-
UpdateHeaderParsingState(value, headerField);
1394-
1395-
// http://httpwg.org/specs/rfc7540.html#rfc.section.8.1.2
1396-
// No need to validate if header name if it is specified using a static index.
1397-
}
1398-
13991408
private void UpdateHeaderParsingState(ReadOnlySpan<byte> value, PseudoHeaderFields headerField)
14001409
{
14011410
// http://httpwg.org/specs/rfc7540.html#rfc.section.8.1.2.1
@@ -1464,6 +1473,32 @@ implementations to these vulnerabilities.*/
14641473
}
14651474
}
14661475

1476+
private static PseudoHeaderFields GetPseudoHeaderField(int staticTableIndex)
1477+
{
1478+
Debug.Assert(staticTableIndex > 0, "Static table starts at 1.");
1479+
1480+
var headerField = staticTableIndex switch
1481+
{
1482+
1 => PseudoHeaderFields.Authority,
1483+
2 => PseudoHeaderFields.Method,
1484+
3 => PseudoHeaderFields.Method,
1485+
4 => PseudoHeaderFields.Path,
1486+
5 => PseudoHeaderFields.Path,
1487+
6 => PseudoHeaderFields.Scheme,
1488+
7 => PseudoHeaderFields.Scheme,
1489+
8 => PseudoHeaderFields.Status,
1490+
9 => PseudoHeaderFields.Status,
1491+
10 => PseudoHeaderFields.Status,
1492+
11 => PseudoHeaderFields.Status,
1493+
12 => PseudoHeaderFields.Status,
1494+
13 => PseudoHeaderFields.Status,
1495+
14 => PseudoHeaderFields.Status,
1496+
_ => PseudoHeaderFields.None
1497+
};
1498+
1499+
return headerField;
1500+
}
1501+
14671502
private static PseudoHeaderFields GetPseudoHeaderField(ReadOnlySpan<byte> name)
14681503
{
14691504
if (name.IsEmpty || name[0] != (byte)':')

‎src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,9 @@ public override void OnHeader(int index, bool indexedValue, ReadOnlySpan<byte> n
675675
// HPack append will return false if the index is not a known request header.
676676
// For example, someone could send the index of "Server" (a response header) in the request.
677677
// If that happens then fallback to using Append with the name bytes.
678-
if (!HttpRequestHeaders.TryHPackAppend(index, value))
678+
//
679+
// If the value is indexed then we know it doesn't contain new lines and can skip checking.
680+
if (!HttpRequestHeaders.TryHPackAppend(index, value, checkForNewlineChars: !indexedValue))
679681
{
680682
AppendHeader(name, value);
681683
}

‎src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
#nullable enable
2+
Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.IHttpHeadersHandler.OnDynamicIndexedHeader(int? index, System.ReadOnlySpan<byte> name, System.ReadOnlySpan<byte> value) -> void

‎src/Servers/Kestrel/shared/KnownHeaders.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,12 +1303,11 @@ public unsafe void Append(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value, boo
13031303
}}
13041304
13051305
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
1306-
public unsafe bool TryHPackAppend(int index, ReadOnlySpan<byte> value)
1306+
public unsafe bool TryHPackAppend(int index, ReadOnlySpan<byte> value, bool checkForNewlineChars)
13071307
{{
13081308
ref StringValues values = ref Unsafe.AsRef<StringValues>(null);
13091309
var nameStr = string.Empty;
13101310
var flag = 0L;
1311-
var checkForNewlineChars = true;
13121311
13131312
// Does the HPack static index match any ""known"" headers
13141313
{AppendHPackSwitch(GroupHPack(loop.Headers))}

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,27 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests;
2626

2727
public class Http2StreamTests : Http2TestBase
2828
{
29+
[Theory]
30+
[InlineData("\r")]
31+
[InlineData("\n")]
32+
[InlineData("\r\n")]
33+
public async Task HEADERS_Received_NewLineCharactersInValue_ConnectionError(string headerValue)
34+
{
35+
var headers = new[]
36+
{
37+
new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
38+
new KeyValuePair<string, string>(HeaderNames.Path, "/"),
39+
new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
40+
new KeyValuePair<string, string>(HeaderNames.Authority, "localhost:80"),
41+
new KeyValuePair<string, string>("TestHeader", headerValue),
42+
};
43+
await InitializeConnectionAsync(_noopApplication);
44+
45+
await StartStreamAsync(1, headers, endStream: true);
46+
47+
await WaitForConnectionErrorAsync<Exception>(ignoreNonGoAwayFrames: false, 1, Http2ErrorCode.PROTOCOL_ERROR, "Malformed request: invalid headers.");
48+
}
49+
2950
[Fact]
3051
public async Task HEADERS_Received_EmptyMethod_Reset()
3152
{

‎src/Shared/runtime/Http2/Hpack/DynamicTable.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ public ref readonly HeaderField this[int index]
4646
}
4747

4848
public void Insert(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
49+
{
50+
Insert(staticTableIndex: null, name, value);
51+
}
52+
53+
public void Insert(int? staticTableIndex, ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
4954
{
5055
int entryLength = HeaderField.GetLength(name.Length, value.Length);
5156
EnsureAvailable(entryLength);
@@ -59,7 +64,7 @@ public void Insert(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
5964
return;
6065
}
6166

62-
var entry = new HeaderField(name, value);
67+
var entry = new HeaderField(staticTableIndex, name, value);
6368
_buffer[_insertIndex] = entry;
6469
_insertIndex = (_insertIndex + 1) % _buffer.Length;
6570
_size += entry.Length;

0 commit comments

Comments
 (0)