From 67588d5a264a2677564fbe21e3bb69278b272cf8 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Sat, 25 Apr 2020 01:26:39 +0100 Subject: [PATCH 1/2] Make GetFinalTransferCoding "safe" --- .../Core/src/Internal/Http/HttpHeaders.cs | 148 +++++++++++++----- 1 file changed, 112 insertions(+), 36 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs index e1175fcde98f..2e4072587b4a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs @@ -2,10 +2,12 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Buffers.Binary; using System.Collections; using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Runtime.Intrinsics.X86; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; @@ -377,63 +379,111 @@ public static unsafe ConnectionOptions ParseConnection(StringValues connection) return connectionOptions; } - public static unsafe TransferCoding GetFinalTransferCoding(StringValues transferEncoding) + private static char ToLowerCase(char value) => (char)(value | (char)0x0020); + + public static TransferCoding GetFinalTransferCoding(StringValues transferEncoding) { + const ulong chunkedStart = 0x006b_006e_0075_0068; // 4 chars "hunk" + const uint chunkedEnd = 0x0064_0065; // 2 chars "ed" + var transferEncodingOptions = TransferCoding.None; var transferEncodingCount = transferEncoding.Count; for (var i = 0; i < transferEncodingCount; i++) { - var value = transferEncoding[i]; - fixed (char* ptr = value) - { - var ch = ptr; - var tokenEnd = ch; - var end = ch + value.Length; + var values = transferEncoding[i].AsSpan(); - while (ch < end) + while (values.Length > 0) + { + int offset; + char c = '\0'; + // Skip any spaces and empty values. + for (offset = 0; offset < values.Length; offset++) { - while (tokenEnd < end && *tokenEnd != ',') + c = values[offset]; + if (c == ' ' || c == ',') { - tokenEnd++; + continue; } - while (ch < tokenEnd && *ch == ' ') + break; + } + + // Skip last read char. + offset++; + if ((uint)offset > (uint)values.Length) + { + // Consumed entire string, move to next. + break; + } + + // Remove leading spaces or empty values. + values = values.Slice(offset); + offset = 0; + + var byteValue = MemoryMarshal.AsBytes(values); + + if (ToLowerCase(c) == 'c' && + TryReadLowerCaseUInt64(byteValue, out var result64) && result64 == chunkedStart) + { + offset += sizeof(ulong) / 2; + byteValue = byteValue.Slice(sizeof(ulong)); + + if (TryReadLowerCaseUInt32(byteValue, out var result32) && result32 == chunkedEnd) { - ch++; + offset += sizeof(uint) / 2; + transferEncodingOptions = TransferCoding.Chunked; } + } - var tokenLength = tokenEnd - ch; + if ((uint)offset >= (uint)values.Length) + { + // Consumed entire string, move to next string. + break; + } + else + { + values = values.Slice(offset); + } - if (tokenLength >= 7 && (*ch | 0x20) == 'c') + for (offset = 0; offset < values.Length; offset++) + { + c = values[offset]; + if (c == ' ') { - if ((*++ch | 0x20) == 'h' && - (*++ch | 0x20) == 'u' && - (*++ch | 0x20) == 'n' && - (*++ch | 0x20) == 'k' && - (*++ch | 0x20) == 'e' && - (*++ch | 0x20) == 'd') - { - ch++; - while (ch < tokenEnd && *ch == ' ') - { - ch++; - } - - if (ch == tokenEnd) - { - transferEncodingOptions = TransferCoding.Chunked; - } - } + continue; } - - if (tokenLength > 0 && ch != tokenEnd) + if (c == ',') { + break; + } + else + { + // Value contains extra chars; Chunked is not the matched one. transferEncodingOptions = TransferCoding.Other; + continue; } + } - tokenEnd++; - ch = tokenEnd; + if ((uint)offset >= (uint)values.Length) + { + // Consumed entire string, move to next string. + break; + } + else if (c == ',') + { + // Consumed value, move to next value. + offset++; + if ((uint)offset >= (uint)values.Length) + { + // Consumed entire string, move to next string. + break; + } + else + { + values = values.Slice(offset); + continue; + } } } } @@ -441,6 +491,32 @@ public static unsafe TransferCoding GetFinalTransferCoding(StringValues transfer return transferEncodingOptions; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool TryReadLowerCaseUInt64(ReadOnlySpan byteValue, out ulong value) + { + if (BinaryPrimitives.TryReadUInt64LittleEndian(byteValue, out var result)) + { + value = result | 0x0020_0020_0020_0020; + return true; + } + + value = 0; + return false; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool TryReadLowerCaseUInt32(ReadOnlySpan byteValue, out uint value) + { + if (BinaryPrimitives.TryReadUInt32LittleEndian(byteValue, out var result)) + { + value = result | 0x0020_0020; + return true; + } + + value = 0; + return false; + } + private static void ThrowInvalidContentLengthException(long value) { throw new ArgumentOutOfRangeException(CoreStrings.FormatInvalidContentLength_InvalidNumber(value)); From 650f7d9dc08fd69ebca2e04850def2f50bf2543e Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 28 Apr 2020 01:59:39 +0100 Subject: [PATCH 2/2] Feedback --- .../Kestrel/Core/src/Internal/Http/HttpHeaders.cs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs index 2e4072587b4a..b2e981289d6c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs @@ -401,12 +401,10 @@ public static TransferCoding GetFinalTransferCoding(StringValues transferEncodin for (offset = 0; offset < values.Length; offset++) { c = values[offset]; - if (c == ' ' || c == ',') + if (c != ' ' && c != ',') { - continue; + break; } - - break; } // Skip last read char. @@ -449,19 +447,14 @@ public static TransferCoding GetFinalTransferCoding(StringValues transferEncodin for (offset = 0; offset < values.Length; offset++) { c = values[offset]; - if (c == ' ') - { - continue; - } if (c == ',') { break; } - else + else if (c != ' ') { // Value contains extra chars; Chunked is not the matched one. transferEncodingOptions = TransferCoding.Other; - continue; } }