Skip to content

Commit 3af92e2

Browse files
benaadamsbenaadams
authored
Make connection options parsing "safe" (#21004)
1 parent 9b44d28 commit 3af92e2

File tree

4 files changed

+135
-73
lines changed

4 files changed

+135
-73
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ public static MessageBody For(
124124
{
125125
var connectionOptions = HttpHeaders.ParseConnection(headers.HeaderConnection);
126126

127-
upgrade = (connectionOptions & ConnectionOptions.Upgrade) == ConnectionOptions.Upgrade;
128-
keepAlive = (connectionOptions & ConnectionOptions.KeepAlive) == ConnectionOptions.KeepAlive;
127+
upgrade = (connectionOptions & ConnectionOptions.Upgrade) != 0;
128+
keepAlive = (connectionOptions & ConnectionOptions.KeepAlive) != 0;
129129
}
130130

131131
if (upgrade)

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

Lines changed: 124 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,9 @@
88
using System.Linq;
99
using System.Runtime.CompilerServices;
1010
using System.Runtime.InteropServices;
11-
using System.Runtime.Intrinsics.X86;
1211
using Microsoft.AspNetCore.Http;
1312
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
1413
using Microsoft.Extensions.Primitives;
15-
using Microsoft.Net.Http.Headers;
1614

1715
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
1816
{
@@ -277,108 +275,165 @@ public static void ValidateHeaderNameCharacters(string headerCharacters)
277275
}
278276
}
279277

280-
public static unsafe ConnectionOptions ParseConnection(StringValues connection)
278+
public static ConnectionOptions ParseConnection(StringValues connection)
281279
{
280+
// Keep-alive
281+
const ulong lowerCaseKeep = 0x0000_0020_0020_0020; // Don't lowercase hyphen
282+
const ulong keepAliveStart = 0x002d_0070_0065_0065; // 4 chars "eep-"
283+
const ulong keepAliveMiddle = 0x0076_0069_006c_0061; // 4 chars "aliv"
284+
const ushort keepAliveEnd = 0x0065; // 1 char "e"
285+
// Upgrade
286+
const ulong upgradeStart = 0x0061_0072_0067_0070; // 4 chars "pgra"
287+
const uint upgradeEnd = 0x0065_0064; // 2 chars "de"
288+
// Close
289+
const ulong closeEnd = 0x0065_0073_006f_006c; // 4 chars "lose"
290+
282291
var connectionOptions = ConnectionOptions.None;
283292

284293
var connectionCount = connection.Count;
285294
for (var i = 0; i < connectionCount; i++)
286295
{
287-
var value = connection[i];
288-
fixed (char* ptr = value)
296+
var value = connection[i].AsSpan();
297+
while (value.Length > 0)
289298
{
290-
var ch = ptr;
291-
var tokenEnd = ch;
292-
var end = ch + value.Length;
293-
294-
while (ch < end)
299+
int offset;
300+
char c = '\0';
301+
// Skip any spaces and empty values.
302+
for (offset = 0; offset < value.Length; offset++)
295303
{
296-
while (tokenEnd < end && *tokenEnd != ',')
304+
c = value[offset];
305+
if (c != ' ' && c != ',')
297306
{
298-
tokenEnd++;
307+
break;
299308
}
309+
}
300310

301-
while (ch < tokenEnd && *ch == ' ')
302-
{
303-
ch++;
304-
}
311+
// Skip last read char.
312+
offset++;
313+
if ((uint)offset > (uint)value.Length)
314+
{
315+
// Consumed enitre string, move to next.
316+
break;
317+
}
305318

306-
var tokenLength = tokenEnd - ch;
319+
// Remove leading spaces or empty values.
320+
value = value.Slice(offset);
321+
c = ToLowerCase(c);
322+
323+
var byteValue = MemoryMarshal.AsBytes(value);
324+
325+
offset = 0;
326+
var potentialConnectionOptions = ConnectionOptions.None;
307327

308-
if (tokenLength >= 9 && (*ch | 0x20) == 'k')
328+
if (c == 'k' && byteValue.Length >= (2 * sizeof(ulong) + sizeof(ushort)))
329+
{
330+
if ((BinaryPrimitives.ReadUInt64LittleEndian(byteValue) | lowerCaseKeep) == keepAliveStart)
309331
{
310-
if ((*++ch | 0x20) == 'e' &&
311-
(*++ch | 0x20) == 'e' &&
312-
(*++ch | 0x20) == 'p' &&
313-
*++ch == '-' &&
314-
(*++ch | 0x20) == 'a' &&
315-
(*++ch | 0x20) == 'l' &&
316-
(*++ch | 0x20) == 'i' &&
317-
(*++ch | 0x20) == 'v' &&
318-
(*++ch | 0x20) == 'e')
332+
offset += sizeof(ulong) / 2;
333+
byteValue = byteValue.Slice(sizeof(ulong));
334+
335+
if (ReadLowerCaseUInt64(byteValue) == keepAliveMiddle)
319336
{
320-
ch++;
321-
while (ch < tokenEnd && *ch == ' ')
322-
{
323-
ch++;
324-
}
337+
offset += sizeof(ulong) / 2;
338+
byteValue = byteValue.Slice(sizeof(ulong));
325339

326-
if (ch == tokenEnd)
340+
if (ReadLowerCaseUInt16(byteValue) == keepAliveEnd)
327341
{
328-
connectionOptions |= ConnectionOptions.KeepAlive;
342+
offset += sizeof(ushort) / 2;
343+
potentialConnectionOptions = ConnectionOptions.KeepAlive;
329344
}
330345
}
331346
}
332-
else if (tokenLength >= 7 && (*ch | 0x20) == 'u')
347+
}
348+
else if (c == 'u' && byteValue.Length >= (sizeof(ulong) + sizeof(uint)))
349+
{
350+
if (ReadLowerCaseUInt64(byteValue) == upgradeStart)
333351
{
334-
if ((*++ch | 0x20) == 'p' &&
335-
(*++ch | 0x20) == 'g' &&
336-
(*++ch | 0x20) == 'r' &&
337-
(*++ch | 0x20) == 'a' &&
338-
(*++ch | 0x20) == 'd' &&
339-
(*++ch | 0x20) == 'e')
340-
{
341-
ch++;
342-
while (ch < tokenEnd && *ch == ' ')
343-
{
344-
ch++;
345-
}
352+
offset += sizeof(ulong) / 2;
353+
byteValue = byteValue.Slice(sizeof(ulong));
346354

347-
if (ch == tokenEnd)
348-
{
349-
connectionOptions |= ConnectionOptions.Upgrade;
350-
}
355+
if (ReadLowerCaseUInt32(byteValue) == upgradeEnd)
356+
{
357+
offset += sizeof(uint) / 2;
358+
potentialConnectionOptions = ConnectionOptions.Upgrade;
351359
}
352360
}
353-
else if (tokenLength >= 5 && (*ch | 0x20) == 'c')
361+
}
362+
else if (c == 'c' && byteValue.Length >= sizeof(ulong))
363+
{
364+
if (ReadLowerCaseUInt64(byteValue) == closeEnd)
354365
{
355-
if ((*++ch | 0x20) == 'l' &&
356-
(*++ch | 0x20) == 'o' &&
357-
(*++ch | 0x20) == 's' &&
358-
(*++ch | 0x20) == 'e')
359-
{
360-
ch++;
361-
while (ch < tokenEnd && *ch == ' ')
362-
{
363-
ch++;
364-
}
366+
offset += sizeof(ulong) / 2;
367+
potentialConnectionOptions = ConnectionOptions.Close;
368+
}
369+
}
365370

366-
if (ch == tokenEnd)
367-
{
368-
connectionOptions |= ConnectionOptions.Close;
369-
}
370-
}
371+
if ((uint)offset >= (uint)value.Length)
372+
{
373+
// Consumed enitre string, move to next string.
374+
connectionOptions |= potentialConnectionOptions;
375+
break;
376+
}
377+
else
378+
{
379+
value = value.Slice(offset);
380+
}
381+
382+
for (offset = 0; offset < value.Length; offset++)
383+
{
384+
c = value[offset];
385+
if (c == ',')
386+
{
387+
break;
371388
}
389+
else if (c != ' ')
390+
{
391+
// Value contains extra chars; this is not the matched one.
392+
potentialConnectionOptions = ConnectionOptions.None;
393+
}
394+
}
372395

373-
tokenEnd++;
374-
ch = tokenEnd;
396+
if ((uint)offset >= (uint)value.Length)
397+
{
398+
// Consumed enitre string, move to next string.
399+
connectionOptions |= potentialConnectionOptions;
400+
break;
401+
}
402+
else if (c == ',')
403+
{
404+
// Consumed value corretly.
405+
connectionOptions |= potentialConnectionOptions;
406+
// Skip comma.
407+
offset++;
408+
if ((uint)offset >= (uint)value.Length)
409+
{
410+
// Consumed enitre string, move to next string.
411+
break;
412+
}
413+
else
414+
{
415+
// Move to next value.
416+
value = value.Slice(offset);
417+
}
375418
}
376419
}
377420
}
378421

379422
return connectionOptions;
380423
}
381424

425+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
426+
private static ulong ReadLowerCaseUInt64(ReadOnlySpan<byte> value)
427+
=> BinaryPrimitives.ReadUInt64LittleEndian(value) | 0x0020_0020_0020_0020;
428+
429+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
430+
private static uint ReadLowerCaseUInt32(ReadOnlySpan<byte> value)
431+
=> BinaryPrimitives.ReadUInt32LittleEndian(value) | 0x0020_0020;
432+
433+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
434+
private static ushort ReadLowerCaseUInt16(ReadOnlySpan<byte> value)
435+
=> (ushort)(BinaryPrimitives.ReadUInt16LittleEndian(value) | 0x0020);
436+
382437
private static char ToLowerCase(char value) => (char)(value | (char)0x0020);
383438

384439
public static TransferCoding GetFinalTransferCoding(StringValues transferEncoding)

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,10 +1070,11 @@ private HttpResponseHeaders CreateResponseHeaders(bool appCompleted)
10701070
{
10711071
var responseHeaders = HttpResponseHeaders;
10721072
var hasConnection = responseHeaders.HasConnection;
1073-
var connectionOptions = HttpHeaders.ParseConnection(responseHeaders.HeaderConnection);
10741073
var hasTransferEncoding = responseHeaders.HasTransferEncoding;
10751074

1076-
if (_keepAlive && hasConnection && (connectionOptions & ConnectionOptions.KeepAlive) != ConnectionOptions.KeepAlive)
1075+
if (_keepAlive &&
1076+
hasConnection &&
1077+
(HttpHeaders.ParseConnection(responseHeaders.HeaderConnection) & ConnectionOptions.KeepAlive) == 0)
10771078
{
10781079
_keepAlive = false;
10791080
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public class HttpHeadersTests
2222
[InlineData(",, ", (int)(ConnectionOptions.None))]
2323
[InlineData(" , ,", (int)(ConnectionOptions.None))]
2424
[InlineData(" , , ", (int)(ConnectionOptions.None))]
25+
[InlineData("KEEP-ALIVE", (int)(ConnectionOptions.KeepAlive))]
2526
[InlineData("keep-alive", (int)(ConnectionOptions.KeepAlive))]
2627
[InlineData("keep-alive, upgrade", (int)(ConnectionOptions.KeepAlive | ConnectionOptions.Upgrade))]
2728
[InlineData("keep-alive,upgrade", (int)(ConnectionOptions.KeepAlive | ConnectionOptions.Upgrade))]
@@ -40,6 +41,8 @@ public class HttpHeadersTests
4041
[InlineData(", ,keep-alive", (int)(ConnectionOptions.KeepAlive))]
4142
[InlineData(",, keep-alive", (int)(ConnectionOptions.KeepAlive))]
4243
[InlineData(", , keep-alive", (int)(ConnectionOptions.KeepAlive))]
44+
[InlineData("UPGRADE", (int)(ConnectionOptions.Upgrade))]
45+
[InlineData("upgrade", (int)(ConnectionOptions.Upgrade))]
4346
[InlineData("upgrade,", (int)(ConnectionOptions.Upgrade))]
4447
[InlineData("upgrade,,", (int)(ConnectionOptions.Upgrade))]
4548
[InlineData("upgrade, ", (int)(ConnectionOptions.Upgrade))]
@@ -72,6 +75,7 @@ public class HttpHeadersTests
7275
[InlineData("u,keep-alive", (int)(ConnectionOptions.KeepAlive))]
7376
[InlineData("ke,upgrade", (int)(ConnectionOptions.Upgrade))]
7477
[InlineData("up,keep-alive", (int)(ConnectionOptions.KeepAlive))]
78+
[InlineData("CLOSE", (int)(ConnectionOptions.Close))]
7579
[InlineData("close", (int)(ConnectionOptions.Close))]
7680
[InlineData("upgrade,close", (int)(ConnectionOptions.Close | ConnectionOptions.Upgrade))]
7781
[InlineData("close,upgrade", (int)(ConnectionOptions.Close | ConnectionOptions.Upgrade))]
@@ -87,6 +91,8 @@ public class HttpHeadersTests
8791
[InlineData("close2 ", (int)(ConnectionOptions.None))]
8892
[InlineData("close2 ,", (int)(ConnectionOptions.None))]
8993
[InlineData("close2,", (int)(ConnectionOptions.None))]
94+
[InlineData("close close", (int)(ConnectionOptions.None))]
95+
[InlineData("close dclose", (int)(ConnectionOptions.None))]
9096
[InlineData("keep-alivekeep-alive", (int)(ConnectionOptions.None))]
9197
[InlineData("keep-aliveupgrade", (int)(ConnectionOptions.None))]
9298
[InlineData("upgradeupgrade", (int)(ConnectionOptions.None))]

0 commit comments

Comments
 (0)