Skip to content

Commit a63aa66

Browse files
benaadamsbenaadams
committed
Make connection options parsing "safe"
1 parent f1dcc59 commit a63aa66

File tree

4 files changed

+146
-73
lines changed

4 files changed

+146
-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: 135 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Buffers.Binary;
56
using System.Collections;
67
using System.Collections.Generic;
78
using System.Linq;
89
using System.Runtime.CompilerServices;
9-
using System.Runtime.Intrinsics.X86;
10+
using System.Runtime.InteropServices;
1011
using Microsoft.AspNetCore.Http;
1112
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
1213
using Microsoft.Extensions.Primitives;
13-
using Microsoft.Net.Http.Headers;
1414

1515
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
1616
{
@@ -275,108 +275,174 @@ public static void ValidateHeaderNameCharacters(string headerCharacters)
275275
}
276276
}
277277

278-
public static unsafe ConnectionOptions ParseConnection(StringValues connection)
278+
public static ConnectionOptions ParseConnection(StringValues connection)
279279
{
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 keppAliveEnd = 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+
280291
var connectionOptions = ConnectionOptions.None;
281292

282293
var connectionCount = connection.Count;
283294
for (var i = 0; i < connectionCount; i++)
284295
{
285-
var value = connection[i];
286-
fixed (char* ptr = value)
296+
var value = connection[i].AsSpan();
297+
while (value.Length > 0)
287298
{
288-
var ch = ptr;
289-
var tokenEnd = ch;
290-
var end = ch + value.Length;
291-
292-
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++)
293303
{
294-
while (tokenEnd < end && *tokenEnd != ',')
304+
c = value[offset];
305+
if (c == ' ' || c == ',')
295306
{
296-
tokenEnd++;
307+
continue;
297308
}
298309

299-
while (ch < tokenEnd && *ch == ' ')
300-
{
301-
ch++;
302-
}
310+
break;
311+
}
303312

304-
var tokenLength = tokenEnd - ch;
313+
// Skip last read char.
314+
offset++;
315+
if ((uint)offset > (uint)value.Length)
316+
{
317+
// Consumed enitre string, move to next.
318+
break;
319+
}
320+
321+
// Remove leading spaces or empty values.
322+
value = value.Slice(offset);
323+
c = ToLowerCase(c);
324+
325+
var byteValue = MemoryMarshal.AsBytes(value);
305326

306-
if (tokenLength >= 9 && (*ch | 0x20) == 'k')
327+
offset = 0;
328+
var potentialConnectionOptions = ConnectionOptions.None;
329+
330+
if (c == 'k' && byteValue.Length >= (2 * sizeof(ulong) + sizeof(ushort)))
331+
{
332+
if ((BinaryPrimitives.ReadUInt64LittleEndian(byteValue) | lowerCaseKeep) == keepAliveStart)
307333
{
308-
if ((*++ch | 0x20) == 'e' &&
309-
(*++ch | 0x20) == 'e' &&
310-
(*++ch | 0x20) == 'p' &&
311-
*++ch == '-' &&
312-
(*++ch | 0x20) == 'a' &&
313-
(*++ch | 0x20) == 'l' &&
314-
(*++ch | 0x20) == 'i' &&
315-
(*++ch | 0x20) == 'v' &&
316-
(*++ch | 0x20) == 'e')
334+
offset += sizeof(ulong) / 2;
335+
byteValue = byteValue.Slice(sizeof(ulong));
336+
337+
if (ReadLowerCaseUInt64(byteValue) == keepAliveMiddle)
317338
{
318-
ch++;
319-
while (ch < tokenEnd && *ch == ' ')
320-
{
321-
ch++;
322-
}
339+
offset += sizeof(ulong) / 2;
340+
byteValue = byteValue.Slice(sizeof(ulong));
323341

324-
if (ch == tokenEnd)
342+
if (ReadLowerCaseUInt16(byteValue) == keppAliveEnd)
325343
{
326-
connectionOptions |= ConnectionOptions.KeepAlive;
344+
offset += sizeof(ushort) / 2;
345+
potentialConnectionOptions = ConnectionOptions.KeepAlive;
327346
}
328347
}
329348
}
330-
else if (tokenLength >= 7 && (*ch | 0x20) == 'u')
349+
}
350+
else if (c == 'u' && byteValue.Length >= (sizeof(ulong) + sizeof(uint)))
351+
{
352+
if (ReadLowerCaseUInt64(byteValue) == upgradeStart)
331353
{
332-
if ((*++ch | 0x20) == 'p' &&
333-
(*++ch | 0x20) == 'g' &&
334-
(*++ch | 0x20) == 'r' &&
335-
(*++ch | 0x20) == 'a' &&
336-
(*++ch | 0x20) == 'd' &&
337-
(*++ch | 0x20) == 'e')
338-
{
339-
ch++;
340-
while (ch < tokenEnd && *ch == ' ')
341-
{
342-
ch++;
343-
}
354+
offset += sizeof(ulong) / 2;
355+
byteValue = byteValue.Slice(sizeof(ulong));
344356

345-
if (ch == tokenEnd)
346-
{
347-
connectionOptions |= ConnectionOptions.Upgrade;
348-
}
357+
if (ReadLowerCaseUInt32(byteValue) == upgradeEnd)
358+
{
359+
offset += sizeof(uint) / 2;
360+
potentialConnectionOptions = ConnectionOptions.Upgrade;
349361
}
350362
}
351-
else if (tokenLength >= 5 && (*ch | 0x20) == 'c')
363+
}
364+
else if (c == 'c' && byteValue.Length >= sizeof(ulong))
365+
{
366+
if (ReadLowerCaseUInt64(byteValue) == closeEnd)
352367
{
353-
if ((*++ch | 0x20) == 'l' &&
354-
(*++ch | 0x20) == 'o' &&
355-
(*++ch | 0x20) == 's' &&
356-
(*++ch | 0x20) == 'e')
357-
{
358-
ch++;
359-
while (ch < tokenEnd && *ch == ' ')
360-
{
361-
ch++;
362-
}
368+
offset += sizeof(ulong) / 2;
369+
potentialConnectionOptions = ConnectionOptions.Close;
370+
}
371+
}
363372

364-
if (ch == tokenEnd)
365-
{
366-
connectionOptions |= ConnectionOptions.Close;
367-
}
368-
}
373+
if ((uint)offset >= (uint)value.Length)
374+
{
375+
// Consumed enitre string, move to next string.
376+
connectionOptions |= potentialConnectionOptions;
377+
break;
378+
}
379+
else
380+
{
381+
value = value.Slice(offset);
382+
}
383+
384+
for (offset = 0; offset < value.Length; offset++)
385+
{
386+
c = value[offset];
387+
if (c == ' ')
388+
{
389+
continue;
369390
}
391+
if (c == ',')
392+
{
393+
break;
394+
}
395+
else
396+
{
397+
// Value contains extra chars; this is not the matched one.
398+
potentialConnectionOptions = ConnectionOptions.None;
399+
continue;
400+
}
401+
}
370402

371-
tokenEnd++;
372-
ch = tokenEnd;
403+
if ((uint)offset >= (uint)value.Length)
404+
{
405+
// Consumed enitre string, move to next string.
406+
connectionOptions |= potentialConnectionOptions;
407+
break;
408+
}
409+
else if (c == ',')
410+
{
411+
// Consumed value corretly.
412+
connectionOptions |= potentialConnectionOptions;
413+
// Skip comma.
414+
offset++;
415+
if ((uint)offset >= (uint)value.Length)
416+
{
417+
// Consumed enitre string, move to next string.
418+
break;
419+
}
420+
else
421+
{
422+
// Move to next value.
423+
value = value.Slice(offset);
424+
}
373425
}
374426
}
375427
}
376428

377429
return connectionOptions;
378430
}
379431

432+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
433+
private static ulong ReadLowerCaseUInt64(ReadOnlySpan<byte> value)
434+
=> BinaryPrimitives.ReadUInt64LittleEndian(value) | 0x0020_0020_0020_0020;
435+
436+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
437+
private static uint ReadLowerCaseUInt32(ReadOnlySpan<byte> value)
438+
=> BinaryPrimitives.ReadUInt32LittleEndian(value) | 0x0020_0020;
439+
440+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
441+
private static ushort ReadLowerCaseUInt16(ReadOnlySpan<byte> value)
442+
=> (ushort)(BinaryPrimitives.ReadUInt16LittleEndian(value) | 0x0020);
443+
444+
private static char ToLowerCase(char value) => (char)(value | (char)0x0020);
445+
380446
public static unsafe TransferCoding GetFinalTransferCoding(StringValues transferEncoding)
381447
{
382448
var transferEncodingOptions = TransferCoding.None;

‎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)