From efec4ac19e5e381ce03f66722fdf6407ff3b14c8 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Mon, 4 Oct 2021 18:10:29 -0700 Subject: [PATCH 1/2] Fix form key accumulator Fixes #36987 --- src/Http/Headers/src/HeaderUtilities.cs | 4 ++-- src/Http/Http/src/Internal/ResponseCookies.cs | 2 +- src/Http/WebUtilities/src/KeyValueAccumulator.cs | 2 +- src/Http/WebUtilities/test/FormPipeReaderTests.cs | 15 ++++++++++----- .../src/ResponseCachingKeyProvider.cs | 4 ++-- src/Servers/IIS/IIS/src/Core/IISHttpContext.cs | 7 +++++-- .../Kestrel/Core/src/Internal/Http/HttpHeaders.cs | 2 +- src/Shared/RangeHelper/RangeHelper.cs | 2 +- 8 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/Http/Headers/src/HeaderUtilities.cs b/src/Http/Headers/src/HeaderUtilities.cs index b1b69a0a40e1..bb4d101b126d 100644 --- a/src/Http/Headers/src/HeaderUtilities.cs +++ b/src/Http/Headers/src/HeaderUtilities.cs @@ -241,7 +241,7 @@ public static bool TryParseSeconds(StringValues headerValues, string targetValue for (var i = 0; i < headerValues.Count; i++) { - var segment = headerValues[i]!; + var segment = headerValues[i] ?? string.Empty; // Trim leading white space var current = HttpRuleParser.GetWhitespaceLength(segment, 0); @@ -297,7 +297,7 @@ public static bool ContainsCacheDirective(StringValues cacheControlDirectives, s for (var i = 0; i < cacheControlDirectives.Count; i++) { - var segment = cacheControlDirectives[i]!; + var segment = cacheControlDirectives[i] ?? string.Empty; // Trim leading white space var current = HttpRuleParser.GetWhitespaceLength(segment, 0); diff --git a/src/Http/Http/src/Internal/ResponseCookies.cs b/src/Http/Http/src/Internal/ResponseCookies.cs index 45f2aba75cd6..2e326cb2e9da 100644 --- a/src/Http/Http/src/Internal/ResponseCookies.cs +++ b/src/Http/Http/src/Internal/ResponseCookies.cs @@ -192,7 +192,7 @@ public void Delete(string key, CookieOptions options) for (var i = 0; i < values.Length; i++) { - var value = values[i]!; + var value = values[i] ?? string.Empty; if (!rejectPredicate(value, encodedKeyPlusEquals, options)) { newValues.Add(value); diff --git a/src/Http/WebUtilities/src/KeyValueAccumulator.cs b/src/Http/WebUtilities/src/KeyValueAccumulator.cs index bab02cc7ec5c..280e6066168b 100644 --- a/src/Http/WebUtilities/src/KeyValueAccumulator.cs +++ b/src/Http/WebUtilities/src/KeyValueAccumulator.cs @@ -30,7 +30,7 @@ public void Append(string key, string value) StringValues values; if (_accumulator.TryGetValue(key, out values)) { - if (StringValues.IsNullOrEmpty(values)) + if (values.Count == 0) { // Marker entry for this key to indicate entry already in expanding list dictionary _expandingAccumulator[key].Add(value); diff --git a/src/Http/WebUtilities/test/FormPipeReaderTests.cs b/src/Http/WebUtilities/test/FormPipeReaderTests.cs index b99736fa6635..a34deb1d925f 100644 --- a/src/Http/WebUtilities/test/FormPipeReaderTests.cs +++ b/src/Http/WebUtilities/test/FormPipeReaderTests.cs @@ -1,15 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Buffers; -using System.Collections.Generic; -using System.IO; using System.IO.Pipelines; using System.Text; -using System.Threading.Tasks; using Microsoft.Extensions.Primitives; -using Xunit; namespace Microsoft.AspNetCore.WebUtilities { @@ -567,6 +562,16 @@ public void ParseFormWithIncompleteValueWhenIsFinalBlockSucceeds(ReadOnlySequenc Assert.Equal("", values["b"]); } + [Fact] + public async Task ReadFormAsync_AccumulatesEmptyKeys() + { + var bodyPipe = await MakePipeReader("&&&"); + + var formCollection = await ReadFormAsync(new FormPipeReader(bodyPipe)); + + Assert.Single(formCollection); + } + public static TheoryData> IncompleteFormKeys => new TheoryData> { diff --git a/src/Middleware/ResponseCaching/src/ResponseCachingKeyProvider.cs b/src/Middleware/ResponseCaching/src/ResponseCachingKeyProvider.cs index 238840fb7da4..ad7ed060fdc8 100644 --- a/src/Middleware/ResponseCaching/src/ResponseCachingKeyProvider.cs +++ b/src/Middleware/ResponseCaching/src/ResponseCachingKeyProvider.cs @@ -120,7 +120,7 @@ public string CreateStorageVaryByKey(ResponseCachingContext context) var requestHeaders = context.HttpContext.Request.Headers; for (var i = 0; i < headersCount; i++) { - var header = varyByRules!.Headers[i]!; + var header = varyByRules!.Headers[i] ?? string.Empty; var headerValues = requestHeaders[header]; builder.Append(KeyDelimiter) .Append(header) @@ -174,7 +174,7 @@ public string CreateStorageVaryByKey(ResponseCachingContext context) { for (var i = 0; i < varyByRules.QueryKeys.Count; i++) { - var queryKey = varyByRules.QueryKeys[i]!; + var queryKey = varyByRules.QueryKeys[i] ?? string.Empty; var queryKeyValues = context.HttpContext.Request.Query[queryKey]; builder.Append(KeyDelimiter) .Append(queryKey) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs index d102a4dd8404..51a6fdb49c61 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs @@ -431,13 +431,16 @@ public unsafe void SetResponseHeaders() var knownHeaderIndex = HttpApiTypes.HTTP_RESPONSE_HEADER_ID.IndexOfKnownHeader(headerPair.Key); for (var i = 0; i < headerValues.Count; i++) { - if (string.IsNullOrEmpty(headerValues[i])) + var headerValue = headerValues[i] ?? string.Empty; + + if (string.IsNullOrEmpty(headerValue)) { continue; } var isFirst = i == 0; - var headerValueBytes = Encoding.UTF8.GetBytes(headerValues[i]!); + var headerValueBytes = Encoding.UTF8.GetBytes(headerValue); + fixed (byte* pHeaderValue = headerValueBytes) { if (knownHeaderIndex == -1) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs index 41c98ac38895..6b2bb96ac07c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs @@ -268,7 +268,7 @@ public static void ValidateHeaderValueCharacters(string headerName, StringValues var count = headerValues.Count; for (var i = 0; i < count; i++) { - ValidateHeaderValueCharacters(headerValues[i]!, requireAscii); + ValidateHeaderValueCharacters(headerValues[i] ?? string.Empty, requireAscii); } } diff --git a/src/Shared/RangeHelper/RangeHelper.cs b/src/Shared/RangeHelper/RangeHelper.cs index f76cd7c98615..451d05e2799c 100644 --- a/src/Shared/RangeHelper/RangeHelper.cs +++ b/src/Shared/RangeHelper/RangeHelper.cs @@ -45,7 +45,7 @@ public static (bool isRangeRequest, RangeItemHeaderValue? range) ParseRange( } // Perf: Check for a single entry before parsing it - if (rawRangeHeader.Count > 1 || rawRangeHeader[0]!.IndexOf(',') >= 0) + if (rawRangeHeader.Count > 1 || (rawRangeHeader[0] ?? string.Empty).IndexOf(',') >= 0) { logger.LogDebug("Multiple ranges are not supported."); From 70c8f78271455a4a6203db31c8b644231004b819 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Tue, 5 Oct 2021 06:54:35 -0700 Subject: [PATCH 2/2] Remove unnecessary check --- src/Servers/IIS/IIS/src/Core/IISHttpContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs index 51a6fdb49c61..6f2a732344be 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs @@ -431,7 +431,7 @@ public unsafe void SetResponseHeaders() var knownHeaderIndex = HttpApiTypes.HTTP_RESPONSE_HEADER_ID.IndexOfKnownHeader(headerPair.Key); for (var i = 0; i < headerValues.Count; i++) { - var headerValue = headerValues[i] ?? string.Empty; + var headerValue = headerValues[i]; if (string.IsNullOrEmpty(headerValue)) {