diff --git a/Directory.Build.props b/Directory.Build.props index 996c2be9577b..946014f57ef5 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,6 +1,8 @@ - + + false + true diff --git a/NuGet.config b/NuGet.config index 1ea5b4e25974..27fa359292cc 100644 --- a/NuGet.config +++ b/NuGet.config @@ -6,8 +6,10 @@ + + @@ -30,8 +32,10 @@ + + diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index b74ccf012e15..c38f553cac76 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -9,325 +9,325 @@ --> - + https://dev.azure.com/dnceng/internal/_git/dotnet-efcore - 001e1d31c562c1d246e5a6531b607bf6c851f688 + 60c2e9b8f50634a077d6c4d2a2a73a7f643c9b11 - + https://dev.azure.com/dnceng/internal/_git/dotnet-efcore - 001e1d31c562c1d246e5a6531b607bf6c851f688 + 60c2e9b8f50634a077d6c4d2a2a73a7f643c9b11 - + https://dev.azure.com/dnceng/internal/_git/dotnet-efcore - 001e1d31c562c1d246e5a6531b607bf6c851f688 + 60c2e9b8f50634a077d6c4d2a2a73a7f643c9b11 - + https://dev.azure.com/dnceng/internal/_git/dotnet-efcore - 001e1d31c562c1d246e5a6531b607bf6c851f688 + 60c2e9b8f50634a077d6c4d2a2a73a7f643c9b11 - + https://dev.azure.com/dnceng/internal/_git/dotnet-efcore - 001e1d31c562c1d246e5a6531b607bf6c851f688 + 60c2e9b8f50634a077d6c4d2a2a73a7f643c9b11 - + https://dev.azure.com/dnceng/internal/_git/dotnet-efcore - 001e1d31c562c1d246e5a6531b607bf6c851f688 + 60c2e9b8f50634a077d6c4d2a2a73a7f643c9b11 - + https://dev.azure.com/dnceng/internal/_git/dotnet-efcore - 001e1d31c562c1d246e5a6531b607bf6c851f688 + 60c2e9b8f50634a077d6c4d2a2a73a7f643c9b11 - + https://dev.azure.com/dnceng/internal/_git/dotnet-efcore - 001e1d31c562c1d246e5a6531b607bf6c851f688 + 60c2e9b8f50634a077d6c4d2a2a73a7f643c9b11 - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a https://github.com/dotnet/xdt @@ -367,9 +367,9 @@ bc1c3011064a493b0ca527df6fb7215e2e5cfa96 - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a @@ -380,9 +380,9 @@ - + https://dev.azure.com/dnceng/internal/_git/dotnet-runtime - 4250c8399aa851d2d6a95efbdcc5c4c12311e024 + a1e6809fb8318884882ceff057000654f558738a https://github.com/dotnet/winforms diff --git a/eng/Versions.props b/eng/Versions.props index d360eec21af5..2b2765f3ec08 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -67,92 +67,92 @@ --> - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15-servicing.26175.22 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15-servicing.26175.22 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15-servicing.26175.22 - 9.0.15-servicing.26175.22 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16-servicing.26229.23 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16-servicing.26229.23 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16-servicing.26229.23 + 9.0.16-servicing.26229.23 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 - 9.0.15-servicing.26175.22 - 9.0.15 + 9.0.16-servicing.26229.23 + 9.0.16 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 10.6.0-preview.1.26215.2 10.6.0-preview.1.26215.2 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 - 9.0.15 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 + 9.0.16 4.11.0-3.24554.2 4.11.0-3.24554.2 diff --git a/src/Components/Endpoints/src/FormMapping/FormDataReader.cs b/src/Components/Endpoints/src/FormMapping/FormDataReader.cs index 85c65e363e93..f142f0e9e003 100644 --- a/src/Components/Endpoints/src/FormMapping/FormDataReader.cs +++ b/src/Components/Endpoints/src/FormMapping/FormDataReader.cs @@ -7,6 +7,7 @@ using System.Globalization; using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping; @@ -23,7 +24,6 @@ internal struct FormDataReader : IDisposable // As an implementation detail, reuse FormKey for the values. // It's just a thin wrapper over ReadOnlyMemory that caches // the computed hash code. - private IReadOnlyDictionary>? _formDictionaryKeysByPrefix; private PrefixResolver _prefixResolver; @@ -48,6 +48,8 @@ public FormDataReader(IReadOnlyDictionary formCollection, public int MaxRecursionDepth { get; set; } = 64; + public int MaxCollectionSize { get; set; } = FormReader.DefaultValueCountLimit; + public Action? ErrorHandler { get; set; } public Action? AttachInstanceToErrorsHandler { get; set; } @@ -107,61 +109,52 @@ public void AttachInstanceToErrors(object value) internal FormKeyCollection GetKeys() { - if (_formDictionaryKeysByPrefix == null) - { - _formDictionaryKeysByPrefix = ProcessFormKeys(); - } + // Scan the input dictionary for keys matching the current prefix followed by a bracket segment. + // This avoids building a large upfront dictionary of all prefix→key mappings. + var prefix = _currentPrefixBuffer; + var result = new HashSet(); - if (_formDictionaryKeysByPrefix.TryGetValue(new FormKey(_currentPrefixBuffer), out var foundKeys)) + foreach (var kvp in _readOnlyMemoryKeys) { - return new FormKeyCollection(foundKeys); - } + var key = kvp.Key.Value; - return FormKeyCollection.Empty; - } + // The key must start with the current prefix (case-insensitive). + if (key.Length <= prefix.Length || + !key.Span[..prefix.Length].Equals(prefix.Span, StringComparison.OrdinalIgnoreCase)) + { + continue; + } - // Internal for testing purposes - internal IReadOnlyDictionary> ProcessFormKeys() - { - var keys = _readOnlyMemoryKeys.Keys; - var result = new Dictionary>(); - // We need to iterate over all the keys in the dictionary and process each key to split it into segments where - // the prefixes are string separated by . and the keys are enclosed in []. For example if the key is - // Customer.Orders[<>]BillingInfo.FirstName, then we need to split it into Customer.Orders, - // [<>] and BillingInfo.FirstName. We then, need to group all the keys by the prefix. So, for the - // above example, we will have an entry for the prefix Customer.Orders that will include [<>] as the - // key. - - foreach (var key in keys) - { - var startIndex = key.Value.Span.IndexOf('['); - while (startIndex >= 0) + // Immediately after the prefix, there must be a '['. + if (key.Span[prefix.Length] != '[') + { + continue; + } + + // Find the closing ']' after the '['. + var remaining = key[prefix.Length..]; + var closeIndex = remaining.Span[1..].IndexOf(']'); + if (closeIndex == -1) + { + // Malformed key — no closing bracket. Skip it. + continue; + } + + // Extract "[value]" (closeIndex is relative to position 1, so add 2 for the full segment). + var segment = remaining[..(closeIndex + 2)]; + result.Add(new FormKey(segment)); + + // Allow one extra item through so collection/dictionary binding can still detect overflow + // and report the existing max-size error instead of silently truncating the input. + if (result.Count > MaxCollectionSize) { - var endIndex = key.Value.Span[startIndex..].IndexOf(']') + startIndex; - if (endIndex == -1) - { - // Ignore malformed keys - break; - } - - var prefix = key.Value[..startIndex]; - var keyValue = key.Value[startIndex..(endIndex + 1)]; - if (result.TryGetValue(new FormKey(prefix), out var foundKeys)) - { - foundKeys.Add(new FormKey(keyValue)); - } - else - { - result.Add(new FormKey(prefix), new HashSet { new FormKey(keyValue) }); - } - - var nextOpenBracket = key.Value.Span[(endIndex + 1)..].IndexOf('['); - - startIndex = nextOpenBracket != -1 ? endIndex + 1 + nextOpenBracket : -1; + break; } } - return result; + return result.Count > 0 + ? new FormKeyCollection(result) + : FormKeyCollection.Empty; } // This only ever gets invoked if we have a recursive type. diff --git a/src/Components/Endpoints/src/FormMapping/HttpContextFormValueMapper.cs b/src/Components/Endpoints/src/FormMapping/HttpContextFormValueMapper.cs index db1e9af301cf..212bff678ba2 100644 --- a/src/Components/Endpoints/src/FormMapping/HttpContextFormValueMapper.cs +++ b/src/Components/Endpoints/src/FormMapping/HttpContextFormValueMapper.cs @@ -138,7 +138,8 @@ public override void Deserialize( ErrorHandler = context.OnError, AttachInstanceToErrorsHandler = context.MapErrorToContainer, MaxRecursionDepth = options.MaxRecursionDepth, - MaxErrorCount = options.MaxErrorCount + MaxErrorCount = options.MaxErrorCount, + MaxCollectionSize = options.MaxCollectionSize }; reader.PushPrefix(context.ParameterName); diff --git a/src/Components/Endpoints/test/Binding/FormDataMapperTests.cs b/src/Components/Endpoints/test/Binding/FormDataMapperTests.cs index 823c97143d9d..f7071d66f95c 100644 --- a/src/Components/Endpoints/test/Binding/FormDataMapperTests.cs +++ b/src/Components/Endpoints/test/Binding/FormDataMapperTests.cs @@ -1218,6 +1218,477 @@ public void Deserialize_SkipsElement_WhenFailsToParseKey() }); } + [Fact] + public void GetKeys_IgnoresMalformedKeysWithUnterminatedBracket() + { + // Arrange + var collection = new Dictionary() + { + ["customer[0"] = "10", + ["customer[1]"] = "11", + ["customer[2"] = "12", + }; + + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("customer"); + + // Act + var result = reader.GetKeys().Select(key => key.ToString()).ToArray(); + + // Assert + Assert.Equal(new[] { "[1]" }, result); + } + + [Theory] + [InlineData("prefix[", "prefix")] // unterminated [ at end + [InlineData("prefix[abc", "prefix")] // unterminated [ with content + [InlineData("prefix[value[[", "prefix")] // double [[ at end, all unterminated + [InlineData("[", "")] // key is just [ + [InlineData("[[", "prefix")] // doubled open brackets + [InlineData("]", "prefix")] // only closing bracket + [InlineData("]value[", "prefix")] // reversed brackets surrounding value + [InlineData("prefix][", "prefix")] // reversed brackets at boundary + [InlineData("[[value]", "prefix")] // double [[ at start, different prefix + public void GetKeys_MalformedKeyIgnored_ValidKeyPreserved(string malformedKey, string prefix) + { + var validKey = prefix.Length > 0 ? $"{prefix}[ok]" : "[ok]"; + var collection = new Dictionary() + { + [malformedKey] = "v1", + [validKey] = "v2", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + if (prefix.Length > 0) + { + reader.PushPrefix(prefix); + } + + var result = reader.GetKeys().Select(k => k.ToString()).ToArray(); + + Assert.Equal(new[] { "[ok]" }, result); + } + + [Theory] + [InlineData("prefix[]", "prefix", "[]")] // empty bracket pair + [InlineData("prefix[[value]", "prefix", "[[value]")] // double [[ in middle + [InlineData("a[b.c]", "a", "[b.c]")] // dots inside brackets + [InlineData("a[ ]", "a", "[ ]")] // whitespace inside brackets + [InlineData("prefix[val]]", "prefix", "[val]")] // double ]] at end + [InlineData("prefix[value][", "prefix", "[value]")] // valid then unterminated [ + [InlineData("a[b[c]d]", "a", "[b[c]")] // nested brackets + [InlineData("]]prefix[ok]", "]]prefix", "[ok]")] // double ]] at start of key + [InlineData("prefix[val]]rest[ok]", "prefix", "[val]")] // double ]] in middle + [InlineData("prefix[[a][b]", "prefix", "[[a]")] // double [[ with valid segment after + [InlineData("prefix[a]][b]", "prefix", "[a]")] // double ]] between segments + [InlineData("prefix[a][b", "prefix", "[a]")] // valid then unterminated (partial) + [InlineData("prefix[a]suffix[b", "prefix", "[a]")] // valid then unterminated in suffix + [InlineData("][value][", "]", "[value]")] // reversed ] at start + [InlineData("prefix][value][", "prefix]", "[value]")] // reversed ] in middle + [InlineData("prefix][a][b][", "prefix]", "[a]")] // reversed ] with multiple segments + [InlineData("a[x][y]", "a", "[x]")] // adjacent brackets — first + [InlineData("a[x][y]", "a[x]", "[y]")] // adjacent brackets — second (nested) + [InlineData("prefix[]]", "prefix", "[]")] // extra ] after empty pair + public void GetKeys_ExtractsExpectedKey(string formKey, string prefix, string expectedKey) + { + var collection = new Dictionary() + { + [formKey] = "v1", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + if (prefix.Length > 0) + { + reader.PushPrefix(prefix); + } + + var result = reader.GetKeys().Select(k => k.ToString()).ToArray(); + + Assert.Contains(expectedKey, result); + } + + [Fact] + public void GetKeys_LongRunOfOpenBrackets_NoHang() + { + var malformedKey = new string('[', 10000); + var collection = new Dictionary() + { + [malformedKey] = "v1", + ["prefix[ok]"] = "v2", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("prefix"); + + var result = reader.GetKeys().Select(k => k.ToString()).ToArray(); + + Assert.Equal(new[] { "[ok]" }, result); + } + + [Fact] + public void GetKeys_ManyValidSegments() + { + var sb = new StringBuilder(); + sb.Append("root"); + for (var i = 0; i < 64; i++) + { + sb.Append(CultureInfo.InvariantCulture, $"[{i}]"); + } + var collection = new Dictionary() + { + [sb.ToString()] = "v1", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("root"); + + // Only [0] is directly under "root"; [1] is under "root[0]", etc. + var result = reader.GetKeys().Select(k => k.ToString()).ToArray(); + + Assert.Contains("[0]", result); + } + + [Fact] + public void GetKeys_ManyKeysAllWellFormed() + { + var collection = new Dictionary(); + for (var i = 0; i < 1000; i++) + { + collection[$"prefix[{i}]"] = $"v{i}"; + } + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("prefix"); + + var result = reader.GetKeys().Select(k => k.ToString()).ToArray(); + + Assert.Equal(1000, result.Length); + } + + [Fact] + public void GetKeys_AllowsOneExtraItemForOverflowDetection() + { + var collection = new Dictionary(); + for (var i = 0; i < 200; i++) + { + collection[$"prefix[{i}]"] = $"v{i}"; + } + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("prefix"); + reader.MaxCollectionSize = 50; + + var result = reader.GetKeys().Select(k => k.ToString()).ToArray(); + + Assert.Equal(51, result.Length); + } + + [Fact] + public void Deserialize_Dictionary_WhenReaderMaxCollectionSizeIsSet_StillReportsOverflow() + { + var data = new Dictionary(Enumerable.Range(0, 3) + .Select(i => new KeyValuePair( + $"[{i.ToString(CultureInfo.InvariantCulture)}]", + (i + 10).ToString(CultureInfo.InvariantCulture)))); + + var reader = CreateFormDataReader(data, CultureInfo.InvariantCulture); + var errors = new List(); + reader.ErrorHandler = (key, message, attemptedValue) => + { + errors.Add(new FormDataMappingError(key, message, attemptedValue)); + }; + reader.MaxCollectionSize = 2; + + var options = new FormDataMapperOptions + { + MaxCollectionSize = 2 + }; + + var result = FormDataMapper.Map>(reader, options); + + Assert.Equal(2, result.Count); + var error = Assert.Single(errors); + Assert.Equal("", error.Key); + Assert.Equal("The number of elements in the dictionary exceeded the maximum number of '2' elements allowed.", error.Message.ToString(reader.Culture)); + Assert.Null(error.Value); + } + + [Fact] + public void GetKeys_ManyMalformedKeys_NoHang() + { + var collection = new Dictionary(); + for (var i = 0; i < 100; i++) + { + collection[$"prefix[{i}"] = $"v{i}"; + } + collection["prefix[ok]"] = "valid"; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("prefix"); + + var result = reader.GetKeys().Select(k => k.ToString()).ToArray(); + + Assert.Equal(new[] { "[ok]" }, result); + } + + [Fact] + public void GetLastPrefixSegment_EmptyPrefix() + { + var collection = new Dictionary() { ["key"] = "v1" }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + + var result = reader.GetLastPrefixSegment(); + + Assert.Equal("", result); + } + + [Fact] + public void GetLastPrefixSegment_SimplePrefix() + { + var collection = new Dictionary() { ["key"] = "v1" }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("foo"); + + var result = reader.GetLastPrefixSegment(); + + Assert.Equal("foo", result); + } + + [Fact] + public void GetLastPrefixSegment_DottedPrefix() + { + var collection = new Dictionary() { ["key"] = "v1" }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("foo"); + reader.PushPrefix("bar"); + + var result = reader.GetLastPrefixSegment(); + + Assert.Equal("bar", result); + } + + [Fact] + public void GetLastPrefixSegment_BracketedPrefix() + { + var collection = new Dictionary() { ["key"] = "v1" }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("foo"); + reader.PushPrefix("[0]"); + + var result = reader.GetLastPrefixSegment(); + + Assert.Equal("0", result); + } + + [Fact] + public void TryGetValue_SingleValue_ReturnsValue() + { + var collection = new Dictionary() + { + ["key"] = "hello", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("key"); + + var found = reader.TryGetValue(out var value); + + Assert.True(found); + Assert.Equal("hello", value); + } + + [Fact] + public void TryGetValue_MultipleValues_ReturnsFirstValue() + { + var collection = new Dictionary() + { + ["key"] = new StringValues(new[] { "first", "second", "third" }), + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("key"); + + var found = reader.TryGetValue(out var value); + + Assert.True(found); + Assert.Equal("first", value); + } + + [Fact] + public void TryGetValue_KeyNotFound_ReturnsFalse() + { + var collection = new Dictionary() + { + ["other"] = "hello", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("missing"); + + var found = reader.TryGetValue(out var value); + + Assert.False(found); + Assert.Null(value); + } + + [Fact] + public void CurrentPrefixExists_EmptyPrefix_WithKeys_ReturnsTrue() + { + var collection = new Dictionary() + { + ["key"] = "v1", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + + var result = reader.CurrentPrefixExists(); + + Assert.True(result); + } + + [Fact] + public void CurrentPrefixExists_EmptyPrefix_NoKeys_ReturnsFalse() + { + var collection = new Dictionary(); + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + + var result = reader.CurrentPrefixExists(); + + Assert.False(result); + } + + [Fact] + public void CurrentPrefixExists_MatchingPrefix_ReturnsTrue() + { + var collection = new Dictionary() + { + ["customer.name"] = "Alice", + ["customer.age"] = "30", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("customer"); + + var result = reader.CurrentPrefixExists(); + + Assert.True(result); + } + + [Fact] + public void CurrentPrefixExists_NoMatchingPrefix_ReturnsFalse() + { + var collection = new Dictionary() + { + ["other.name"] = "Alice", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("customer"); + + var result = reader.CurrentPrefixExists(); + + Assert.False(result); + } + + [Fact] + public void PushPrefix_ExceedingBufferLength_Throws() + { + var collection = new Dictionary() + { + ["key"] = "v1", + }; + var reader = new FormDataReader( + new Dictionary + { + [new FormKey("key".AsMemory())] = "v1" + }, + CultureInfo.InvariantCulture, + new char[20]); + + reader.PushPrefix("short"); + + Assert.ThrowsAny(() => reader.PushPrefix(new string('x', 100))); + } + + [Fact] + public void CurrentPrefixExists_BracketDelimitedKey_ReturnsTrue() + { + var collection = new Dictionary() + { + ["a[x]"] = "v1", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("a"); + + var result = reader.CurrentPrefixExists(); + + Assert.True(result); + } + + [Theory] + [InlineData("a[x]", "a")] + [InlineData("d[x].y", "d")] + [InlineData("d[x].y", "d[x]")] + [InlineData("d[x].y", "d[x].y")] + [InlineData("e.a.b[foo].bar", "e")] + [InlineData("e.a.b[foo].bar", "e.a.b")] + [InlineData("e.a.b[foo].bar", "e.a.b[foo]")] + [InlineData("e.a.b[foo].bar", "e.a.b[foo].bar")] + public void CurrentPrefixExists_DotAndBracketPrefixes_ReturnsTrue(string key, string prefix) + { + var collection = new Dictionary() + { + [key] = "v1", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix(prefix); + + var result = reader.CurrentPrefixExists(); + + Assert.True(result); + } + + [Fact] + public void GetKeys_MixedDotAndBracketKeys_ExtractsOnlyBracketedKeys() + { + var collection = new Dictionary() + { + ["foo[0].name"] = "Alice", + ["foo.age"] = "30", + ["foo[1].name"] = "Bob", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("foo"); + + var result = reader.GetKeys().Select(k => k.ToString()).OrderBy(k => k).ToArray(); + + Assert.Equal(new[] { "[0]", "[1]" }, result); + } + + [Fact] + public void GetKeys_NestedDotBracketPrefix_ExtractsNestedKeys() + { + var collection = new Dictionary() + { + ["person[0].address[0].street"] = "Main St", + ["person[0].address[1].street"] = "Oak Ave", + ["person[0].address[1].zip"] = "12345", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + reader.PushPrefix("person[0].address"); + + var result = reader.GetKeys().Select(k => k.ToString()).OrderBy(k => k).ToArray(); + + Assert.Equal(new[] { "[0]", "[1]" }, result); + } + + [Fact] + public void GetKeys_EmptyPrefix_ExtractsBracketedRootKeys() + { + var collection = new Dictionary() + { + ["[0].name"] = "Alice", + ["[0].address.street"] = "Main St", + ["[item1].name"] = "Bob", + ["[item1].age"] = "30", + ["foo"] = "bar", + ["foo.baz"] = "qux", + }; + var reader = CreateFormDataReader(collection, CultureInfo.InvariantCulture); + + var result = reader.GetKeys().Select(k => k.ToString()).OrderBy(k => k).ToArray(); + + Assert.Contains("[0]", result); + Assert.Contains("[item1]", result); + } + private void CanDeserialize_Dictionary(TImplementation expected) where TDictionary : IDictionary where TImplementation : TDictionary diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 159982900eda..5648fe9330e1 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -2149,6 +2149,10 @@ private static Expression BindComplexParameterFromFormItem( var setMaxRecursionDepthExpr = Expression.Assign( Expression.Property(formReader, nameof(FormDataReader.MaxRecursionDepth)), Expression.Constant(formDataMapperOptions.MaxRecursionDepth)); + // name_reader.MaxCollectionSize = formDataMapperOptions.MaxCollectionSize; + var setMaxCollectionSizeExpr = Expression.Assign( + Expression.Property(formReader, nameof(FormDataReader.MaxCollectionSize)), + Expression.Constant(formDataMapperOptions.MaxCollectionSize)); // FormDataMapper.Map(name_reader, FormDataMapperOptions); var invokeMapMethodExpr = Expression.Call( FormDataMapperMapMethod.MakeGenericMethod(parameter.ParameterType), @@ -2175,6 +2179,7 @@ private static Expression BindComplexParameterFromFormItem( // ProcessForm(context.Request.Form, form_dict, form_buffer); // name_reader = new FormDataReader(form_dict, CultureInfo.InvariantCulture, form_buffer.AsMemory(0, FormDataMapperOptions.MaxKeyBufferSize)); // name_reader.MaxRecursionDepth = formDataMapperOptions.MaxRecursionDepth; + // name_reader.MaxCollectionSize = formDataMapperOptions.MaxCollectionSize; // name_local = FormDataMapper.Map(name_reader, FormDataMapperOptions); // } // catch (FormDataMappingException e) @@ -2197,6 +2202,7 @@ private static Expression BindComplexParameterFromFormItem( processFormExpr, initializeReaderExpr, setMaxRecursionDepthExpr, + setMaxCollectionSizeExpr, Expression.Assign(formArgument, invokeMapMethodExpr)), conditionalReturnBufferExpr, Expression.Catch(formDataMappingException, Expression.Block( diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.FormMapping.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.FormMapping.cs index 31e2af50a478..0c3f4f360290 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.FormMapping.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.FormMapping.cs @@ -61,6 +61,44 @@ static void TestAction([FromForm] Dictionary args) { } Assert.Equal("The number of elements in the dictionary exceeded the maximum number of '2' elements allowed.", exception.Message); } + [Fact] + public async Task SupportsFormMappingOptionsInMetadata_PropagatesMaxCollectionSizeToReader() + { + // Arrange + static void TestAction([FromForm] Dictionary args) { } + var options = new RequestDelegateFactoryOptions + { + EndpointBuilder = CreateEndpointBuilder(new List() + { + new FormMappingOptionsMetadata(maxCollectionSize: 2) + }), + ThrowOnBadRequest = true + }; + var metadataResult = new RequestDelegateMetadataResult { EndpointMetadata = new List() }; + var httpContext = CreateHttpContext(); + httpContext.Request.Form = new FormCollection(new Dictionary + { + { + "[name1]", "value1" + }, + { + "[name2]", "value2" + }, + { + "[name3]", "value3" + } + }); + + var factoryResult = RequestDelegateFactory.Create(TestAction, options, metadataResult); + var requestDelegate = factoryResult.RequestDelegate; + + // Act + var exception = await Assert.ThrowsAsync(async () => await requestDelegate(httpContext)); + + // Assert + Assert.Equal("The number of elements in the dictionary exceeded the maximum number of '2' elements allowed.", exception.Message); + } + [Fact] public async Task SupportsFormMappingOptionsInMetadataFormFormWithAttributeName() { diff --git a/src/Middleware/ResponseCaching/samples/ResponseCachingSample/Startup.cs b/src/Middleware/ResponseCaching/samples/ResponseCachingSample/Startup.cs index 3cf2daf4b12d..e322ff2322fd 100644 --- a/src/Middleware/ResponseCaching/samples/ResponseCachingSample/Startup.cs +++ b/src/Middleware/ResponseCaching/samples/ResponseCachingSample/Startup.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.AspNetCore.ResponseCaching; using Microsoft.Net.Http.Headers; namespace ResponseCachingSample; @@ -20,17 +21,31 @@ public void Configure(IApplicationBuilder app) context.Response.GetTypedHeaders().CacheControl = new CacheControlHeaderValue() { Public = true, - MaxAge = TimeSpan.FromSeconds(10) + MaxAge = TimeSpan.FromSeconds(60) }; - context.Response.Headers.Vary = new string[] { "Accept-Encoding" }; - await context.Response.WriteAsync("Hello World! " + DateTime.UtcNow); + var responseCachingFeature = context.Features.Get(); + if (responseCachingFeature != null) + { + responseCachingFeature.VaryByQueryKeys = [ "*" ]; + } + + var user = context.Request.Query["user"].FirstOrDefault() ?? "(none)"; + var theme = context.Request.Query["theme"].FirstOrDefault() ?? "(none)"; + + context.Response.ContentType = "text/plain"; + await context.Response.WriteAsync($"User: {user} | Theme: {theme} | Generated: {DateTime.UtcNow:O}"); }); } public static Task Main(string[] args) { var host = new HostBuilder() + .ConfigureLogging(logging => + { + logging.AddConsole(); + logging.SetMinimumLevel(LogLevel.Information); + }) .ConfigureWebHost(webHostBuilder => { webHostBuilder diff --git a/src/Middleware/ResponseCaching/src/CacheKeyDelimiterException.cs b/src/Middleware/ResponseCaching/src/CacheKeyDelimiterException.cs new file mode 100644 index 000000000000..d2a4c6f4eb09 --- /dev/null +++ b/src/Middleware/ResponseCaching/src/CacheKeyDelimiterException.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.ResponseCaching; + +internal sealed class CacheKeyDelimiterException : Exception +{ + public CacheKeyDelimiterException() + : base("The value contains invalid characters to cache.") + { + } +} diff --git a/src/Middleware/ResponseCaching/src/LoggerExtensions.cs b/src/Middleware/ResponseCaching/src/LoggerExtensions.cs index 68887c41c3aa..83e969e14ad7 100644 --- a/src/Middleware/ResponseCaching/src/LoggerExtensions.cs +++ b/src/Middleware/ResponseCaching/src/LoggerExtensions.cs @@ -120,4 +120,8 @@ internal static partial class LoggerExtensions "However, the 'max-stale' cache directive was specified without an assigned value and a stale response of any age is accepted.", EventName = "ExpirationInfiniteMaxStaleSatisfied")] internal static partial void ExpirationInfiniteMaxStaleSatisfied(this ILogger logger, TimeSpan age, TimeSpan maxAge); + + [LoggerMessage(30, LogLevel.Debug, "The request contains invalid characters and will not be cached.", + EventName = "RequestContainsInvalidCacheSymbols")] + internal static partial void RequestContainsInvalidCacheSymbols(this ILogger logger); } diff --git a/src/Middleware/ResponseCaching/src/ResponseCachingKeyProvider.cs b/src/Middleware/ResponseCaching/src/ResponseCachingKeyProvider.cs index 54458ae19cf5..79c08b246e71 100644 --- a/src/Middleware/ResponseCaching/src/ResponseCachingKeyProvider.cs +++ b/src/Middleware/ResponseCaching/src/ResponseCachingKeyProvider.cs @@ -39,6 +39,10 @@ public string CreateBaseKey(ResponseCachingContext context) ArgumentNullException.ThrowIfNull(context); var request = context.HttpContext.Request; + + ThrowIfContainsDelimiters(request.PathBase.Value); + ThrowIfContainsDelimiters(request.Path.Value); + var builder = _builderPool.Get(); try @@ -117,6 +121,7 @@ public string CreateStorageVaryByKey(ResponseCachingContext context) for (var j = 0; j < headerValuesArray.Length; j++) { + ThrowIfContainsDelimiters(headerValuesArray[j]); builder.Append(headerValuesArray[j]); } } @@ -138,6 +143,8 @@ public string CreateStorageVaryByKey(ResponseCachingContext context) for (var i = 0; i < queryArray.Length; i++) { + ThrowIfContainsDelimiters(queryArray[i].Key); + builder.Append(KeyDelimiter) .AppendUpperInvariant(queryArray[i].Key) .Append('='); @@ -152,6 +159,7 @@ public string CreateStorageVaryByKey(ResponseCachingContext context) builder.Append(KeySubDelimiter); } + ThrowIfContainsDelimiters(queryValueArray[j]); builder.Append(queryValueArray[j]); } } @@ -176,6 +184,7 @@ public string CreateStorageVaryByKey(ResponseCachingContext context) builder.Append(KeySubDelimiter); } + ThrowIfContainsDelimiters(queryValueArray[j]); builder.Append(queryValueArray[j]); } } @@ -190,6 +199,14 @@ public string CreateStorageVaryByKey(ResponseCachingContext context) } } + internal static void ThrowIfContainsDelimiters(string? value) + { + if (!string.IsNullOrEmpty(value) && value.AsSpan().IndexOfAny(KeyDelimiter, KeySubDelimiter) >= 0) + { + throw new CacheKeyDelimiterException(); + } + } + private sealed class QueryKeyComparer : IComparer> { private readonly StringComparer _stringComparer; diff --git a/src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs b/src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs index 8131e512e7d3..6b9fc883c9cd 100644 --- a/src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs +++ b/src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs @@ -203,7 +203,16 @@ internal async Task TryServeCachedResponseAsync(ResponseCachingContext con internal async Task TryServeFromCacheAsync(ResponseCachingContext context) { - context.BaseKey = _keyProvider.CreateBaseKey(context); + try + { + context.BaseKey = _keyProvider.CreateBaseKey(context); + } + catch (CacheKeyDelimiterException) + { + _logger.RequestContainsInvalidCacheSymbols(); + return false; + } + var cacheEntry = _cache.Get(context.BaseKey); if (cacheEntry is CachedVaryByRules cachedVaryByRules) @@ -211,13 +220,21 @@ internal async Task TryServeFromCacheAsync(ResponseCachingContext context) // Request contains vary rules, recompute key(s) and try again context.CachedVaryByRules = cachedVaryByRules; - foreach (var varyKey in _keyProvider.CreateLookupVaryByKeys(context)) + try { - if (await TryServeCachedResponseAsync(context, _cache.Get(varyKey))) + foreach (var varyKey in _keyProvider.CreateLookupVaryByKeys(context)) { - return true; + if (await TryServeCachedResponseAsync(context, _cache.Get(varyKey))) + { + return true; + } } } + catch (CacheKeyDelimiterException) + { + _logger.RequestContainsInvalidCacheSymbols(); + return false; + } } else { @@ -263,7 +280,17 @@ private bool OnFinalizeCacheHeaders(ResponseCachingContext context) // Generate a base key if none exist if (string.IsNullOrEmpty(context.BaseKey)) { - context.BaseKey = _keyProvider.CreateBaseKey(context); + try + { + context.BaseKey = _keyProvider.CreateBaseKey(context); + } + catch (CacheKeyDelimiterException) + { + _logger.RequestContainsInvalidCacheSymbols(); + context.ShouldCacheResponse = false; + context.ResponseCachingStream.DisableBuffering(); + return false; + } } // Check if any vary rules exist @@ -290,7 +317,17 @@ private bool OnFinalizeCacheHeaders(ResponseCachingContext context) _logger.VaryByRulesUpdated(normalizedVaryHeaders.ToString(), normalizedVaryQueryKeys.ToString()); storeVaryByEntry = true; - context.StorageVaryKey = _keyProvider.CreateStorageVaryByKey(context); + try + { + context.StorageVaryKey = _keyProvider.CreateStorageVaryByKey(context); + } + catch (CacheKeyDelimiterException) + { + _logger.RequestContainsInvalidCacheSymbols(); + context.ShouldCacheResponse = false; + context.ResponseCachingStream.DisableBuffering(); + return false; + } } // Ensure date header is set diff --git a/src/Middleware/ResponseCaching/test/ResponseCachingKeyProviderTests.cs b/src/Middleware/ResponseCaching/test/ResponseCachingKeyProviderTests.cs index 1afb15dbd0e4..cc06219f0d6e 100644 --- a/src/Middleware/ResponseCaching/test/ResponseCachingKeyProviderTests.cs +++ b/src/Middleware/ResponseCaching/test/ResponseCachingKeyProviderTests.cs @@ -211,4 +211,106 @@ public void ResponseCachingKeyProvider_CreateStorageVaryKey_IncludesListedHeader Assert.Equal($"{context.CachedVaryByRules.VaryByKeyPrefix}{KeyDelimiter}H{KeyDelimiter}HeaderA=ValueA{KeyDelimiter}HeaderC={KeyDelimiter}Q{KeyDelimiter}QueryA=ValueA{KeyDelimiter}QueryC=", cacheKeyProvider.CreateStorageVaryByKey(context)); } + + [Theory] + [InlineData("\u001e")] + [InlineData("\u001f")] + [InlineData("before\u001eafter")] + [InlineData("before\u001fafter")] + public void ThrowIfContainsDelimiters_ThrowsForValuesWithDelimiters(string value) + { + Assert.Throws(() => ResponseCachingKeyProvider.ThrowIfContainsDelimiters(value)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("normalvalue")] + [InlineData("/path/to/resource")] + [InlineData("value with spaces")] + public void ThrowIfContainsDelimiters_DoesNotThrowForSafeValues(string value) + { + ResponseCachingKeyProvider.ThrowIfContainsDelimiters(value); + } + + [Fact] + public void CreateBaseKey_Throws_IfPathContainsDelimiter() + { + var cacheKeyProvider = TestUtils.CreateTestKeyProvider(); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.Method = HttpMethods.Get; + context.HttpContext.Request.Path = "/path\u001einjected"; + + Assert.Throws(() => cacheKeyProvider.CreateBaseKey(context)); + } + + [Fact] + public void CreateBaseKey_Throws_IfPathBaseContainsDelimiter() + { + var cacheKeyProvider = TestUtils.CreateTestKeyProvider(); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.Method = HttpMethods.Get; + context.HttpContext.Request.PathBase = "/base\u001finjected"; + + Assert.Throws(() => cacheKeyProvider.CreateBaseKey(context)); + } + + [Fact] + public void CreateStorageVaryByKey_Throws_IfHeaderValueContainsDelimiter() + { + var cacheKeyProvider = TestUtils.CreateTestKeyProvider(); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.Headers["HeaderA"] = "Value\u001eInjected"; + context.CachedVaryByRules = new CachedVaryByRules() + { + Headers = new string[] { "HeaderA" } + }; + + Assert.Throws(() => cacheKeyProvider.CreateStorageVaryByKey(context)); + } + + [Fact] + public void CreateStorageVaryByKey_Throws_IfQueryKeyContainsDelimiter_WildcardMode() + { + var cacheKeyProvider = TestUtils.CreateTestKeyProvider(); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.QueryString = new QueryString("?normal=value&injected\u001ekey=value"); + context.CachedVaryByRules = new CachedVaryByRules() + { + VaryByKeyPrefix = FastGuid.NewGuid().IdString, + QueryKeys = new string[] { "*" } + }; + + Assert.Throws(() => cacheKeyProvider.CreateStorageVaryByKey(context)); + } + + [Fact] + public void CreateStorageVaryByKey_Throws_IfQueryValueContainsDelimiter_WildcardMode() + { + var cacheKeyProvider = TestUtils.CreateTestKeyProvider(); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.QueryString = new QueryString("?key=value\u001einjected"); + context.CachedVaryByRules = new CachedVaryByRules() + { + VaryByKeyPrefix = FastGuid.NewGuid().IdString, + QueryKeys = new string[] { "*" } + }; + + Assert.Throws(() => cacheKeyProvider.CreateStorageVaryByKey(context)); + } + + [Fact] + public void CreateStorageVaryByKey_Throws_IfQueryValueContainsDelimiter_ExplicitMode() + { + var cacheKeyProvider = TestUtils.CreateTestKeyProvider(); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.QueryString = new QueryString("?QueryA=value\u001finjected"); + context.CachedVaryByRules = new CachedVaryByRules() + { + VaryByKeyPrefix = FastGuid.NewGuid().IdString, + QueryKeys = new string[] { "QueryA" } + }; + + Assert.Throws(() => cacheKeyProvider.CreateStorageVaryByKey(context)); + } } diff --git a/src/Middleware/ResponseCaching/test/ResponseCachingMiddlewareTests.cs b/src/Middleware/ResponseCaching/test/ResponseCachingMiddlewareTests.cs index c0ea3b50c595..dd3b29b00ba4 100644 --- a/src/Middleware/ResponseCaching/test/ResponseCachingMiddlewareTests.cs +++ b/src/Middleware/ResponseCaching/test/ResponseCachingMiddlewareTests.cs @@ -922,6 +922,104 @@ public async Task Invoke_AddsResponseCachingFeature_Always(bool allowResponseCac Assert.True(responseCachingFeatureAdded); } + [Fact] + public async Task TryServeFromCacheAsync_ReturnsFalse_IfBaseKeyContainsDelimiters() + { + var sink = new TestSink(); + var middleware = TestUtils.CreateTestMiddleware(testSink: sink); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.Path = "/path\u001einjected"; + + Assert.False(await middleware.TryServeFromCacheAsync(context)); + TestUtils.AssertLoggedMessages( + sink.Writes, + LoggedMessage.RequestContainsInvalidCacheSymbols); + } + + [Fact] + public async Task TryServeFromCacheAsync_ReturnsFalse_IfVaryByKeyContainsDelimiters() + { + var cache = new TestResponseCache(); + var sink = new TestSink(); + // Use real key provider so CreateLookupVaryByKeys invokes validation + var middleware = TestUtils.CreateTestMiddleware(testSink: sink, cache: cache); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.Method = HttpMethods.Get; + context.HttpContext.Request.QueryString = new QueryString("?key=value\u001einjected"); + + // Pre-set a base key so CreateBaseKey succeeds, then set up vary rules in cache + // so the middleware will call CreateLookupVaryByKeys -> CreateStorageVaryByKey + var baseKey = $"GET{(char)0x1e}{(char)0x1e}"; + context.BaseKey = baseKey; + var varyByRules = new CachedVaryByRules() + { + VaryByKeyPrefix = FastGuid.NewGuid().IdString, + QueryKeys = new string[] { "*" } + }; + cache.Set(baseKey, varyByRules, TimeSpan.Zero); + + Assert.False(await middleware.TryServeFromCacheAsync(context)); + TestUtils.AssertLoggedMessages( + sink.Writes, + LoggedMessage.RequestContainsInvalidCacheSymbols); + } + + [Fact] + public void FinalizeCacheHeaders_ReturnsFalse_IfBaseKeyContainsDelimiters() + { + var sink = new TestSink(); + var middleware = TestUtils.CreateTestMiddleware(testSink: sink, policyProvider: new ResponseCachingPolicyProvider()); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.Path = "/path\u001einjected"; + context.HttpContext.Response.Headers.CacheControl = new CacheControlHeaderValue() + { + Public = true + }.ToString(); + + middleware.ShimResponseStream(context); + middleware.FinalizeCacheHeaders(context); + + Assert.False(context.ShouldCacheResponse); + Assert.False(context.ResponseCachingStream.BufferingEnabled); + TestUtils.AssertLoggedMessages( + sink.Writes, + LoggedMessage.RequestContainsInvalidCacheSymbols); + } + + [Fact] + public void FinalizeCacheHeaders_ReturnsFalse_IfStorageVaryKeyContainsDelimiters() + { + var sink = new TestSink(); + var middleware = TestUtils.CreateTestMiddleware(testSink: sink, policyProvider: new ResponseCachingPolicyProvider()); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.QueryString = new QueryString("?key=value\u001einjected"); + context.HttpContext.Response.Headers.CacheControl = new CacheControlHeaderValue() + { + Public = true + }.ToString(); + context.HttpContext.Features.Set(new ResponseCachingFeature() + { + VaryByQueryKeys = new string[] { "*" } + }); + // Pre-set BaseKey so CreateBaseKey is skipped (it would also throw) + context.BaseKey = "SomeBaseKey"; + + // ShimResponseStream also calls AddResponseCachingFeature, so remove ours first + context.HttpContext.Features.Set(null); + middleware.ShimResponseStream(context); + // Now re-set VaryByQueryKeys on the feature that ShimResponseStream added + context.HttpContext.Features.Get()!.VaryByQueryKeys = new string[] { "*" }; + + middleware.FinalizeCacheHeaders(context); + + Assert.False(context.ShouldCacheResponse); + Assert.False(context.ResponseCachingStream.BufferingEnabled); + TestUtils.AssertLoggedMessages( + sink.Writes, + LoggedMessage.VaryByRulesUpdated, + LoggedMessage.RequestContainsInvalidCacheSymbols); + } + [Fact] public void GetOrderCasingNormalizedStringValues_NormalizesCasingToUpper() { diff --git a/src/Middleware/ResponseCaching/test/TestUtils.cs b/src/Middleware/ResponseCaching/test/TestUtils.cs index a3a64267e550..d489192be16e 100644 --- a/src/Middleware/ResponseCaching/test/TestUtils.cs +++ b/src/Middleware/ResponseCaching/test/TestUtils.cs @@ -301,6 +301,7 @@ internal class LoggedMessage internal static LoggedMessage ResponseNotCached => new LoggedMessage(27, LogLevel.Information); internal static LoggedMessage ResponseContentLengthMismatchNotCached => new LoggedMessage(28, LogLevel.Warning); internal static LoggedMessage ExpirationInfiniteMaxStaleSatisfied => new LoggedMessage(29, LogLevel.Debug); + internal static LoggedMessage RequestContainsInvalidCacheSymbols => new LoggedMessage(30, LogLevel.Debug); private LoggedMessage(int evenId, LogLevel logLevel) { diff --git a/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs b/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs index de74d6e5bf7c..98d647bc558b 100644 --- a/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs +++ b/src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs @@ -13,6 +13,8 @@ namespace Microsoft.AspNetCore.Authentication.Certificate; internal sealed class CertificateAuthenticationHandler : AuthenticationHandler { + internal const string CertificateSchemeCacheKeyItem = "__CertificateAuthScheme"; + private static readonly Oid ClientCertificateOid = new Oid("1.3.6.1.5.5.7.3.2"); private ICertificateValidationCache? _cache; @@ -67,6 +69,7 @@ protected override async Task HandleAuthenticateAsync() if (_cache != null) { + Context.Items[CertificateSchemeCacheKeyItem] = Scheme.Name; var cacheHit = _cache.Get(Context, clientCertificate); if (cacheHit != null) { diff --git a/src/Security/Authentication/Certificate/src/CertificateValidationCache.cs b/src/Security/Authentication/Certificate/src/CertificateValidationCache.cs index 432a0d2e47bb..27252d8506f0 100644 --- a/src/Security/Authentication/Certificate/src/CertificateValidationCache.cs +++ b/src/Security/Authentication/Certificate/src/CertificateValidationCache.cs @@ -39,7 +39,14 @@ public CertificateValidationCache(IOptions op /// The certificate. /// the public AuthenticateResult? Get(HttpContext context, X509Certificate2 certificate) - => _cache.Get(ComputeKey(certificate))?.Clone(); + { + var key = ComputeKey(context, certificate); + if (key is null) + { + return null; + } + return _cache.Get(key)?.Clone(); + } /// /// Store a for the connection and certificate @@ -49,6 +56,12 @@ public CertificateValidationCache(IOptions op /// the public void Put(HttpContext context, X509Certificate2 certificate, AuthenticateResult result) { + var key = ComputeKey(context, certificate); + if (key is null) + { + return; + } + // Never cache longer than 30 minutes var absoluteExpiration = _timeProvider.GetUtcNow().Add(TimeSpan.FromMinutes(30)); var notAfter = certificate.NotAfter.ToUniversalTime(); @@ -56,14 +69,21 @@ public void Put(HttpContext context, X509Certificate2 certificate, AuthenticateR { absoluteExpiration = notAfter; } - _cache.Set(ComputeKey(certificate), result.Clone(), new MemoryCacheEntryOptions() + _cache.Set(key, result.Clone(), new MemoryCacheEntryOptions() .SetSize(1) .SetSlidingExpiration(_options.CacheEntryExpiration) .SetAbsoluteExpiration(absoluteExpiration)); } - private static string ComputeKey(X509Certificate2 certificate) - => certificate.GetCertHashString(HashAlgorithmName.SHA256); + private static string? ComputeKey(HttpContext context, X509Certificate2 certificate) + { + if (context.Items.TryGetValue(CertificateAuthenticationHandler.CertificateSchemeCacheKeyItem, out var schemeObj) + && schemeObj is string schemeName) + { + return $"{schemeName}:{certificate.GetCertHashString(HashAlgorithmName.SHA256)}"; + } + return null; + } private sealed class CachingClock : Extensions.Internal.ISystemClock { diff --git a/src/Security/Authentication/test/CertificateTests.cs b/src/Security/Authentication/test/CertificateTests.cs index 5e6d73eeca94..80a567d2c5ea 100644 --- a/src/Security/Authentication/test/CertificateTests.cs +++ b/src/Security/Authentication/test/CertificateTests.cs @@ -935,5 +935,130 @@ private static async Task CreateHost( return Task.CompletedTask; } }; + + [Fact] + public async Task VerifyCacheIsIsolatedAcrossSchemes() + { + var scheme1ValidationCount = 0; + var scheme2ValidationCount = 0; + + using var host = new HostBuilder() + .ConfigureWebHost(builder => + builder.UseTestServer() + .Configure(app => + { + app.Use((context, next) => + { + context.Connection.ClientCertificate = Certificates.SelfSignedValidWithNoEku; + return next(context); + }); + + app.UseAuthentication(); + + app.Run(async context => + { + var schemeName = context.Request.Query["scheme"].ToString(); + var result = await context.AuthenticateAsync(schemeName); + + if (result.Succeeded) + { + context.Response.StatusCode = (int)HttpStatusCode.OK; + context.Response.ContentType = "text/plain"; + await context.Response.WriteAsync("Authenticated"); + } + else + { + context.Response.StatusCode = (int)HttpStatusCode.Forbidden; + context.Response.ContentType = "text/plain"; + await context.Response.WriteAsync("Denied"); + } + }); + }) + .ConfigureServices(services => + { + services.AddAuthentication() + .AddCertificate("scheme1", options => + { + options.AllowedCertificateTypes = CertificateTypes.SelfSigned; + options.Events = new CertificateAuthenticationEvents + { + OnCertificateValidated = context => + { + scheme1ValidationCount++; + context.Principal = new ClaimsPrincipal( + new ClaimsIdentity( + [new Claim(ClaimTypes.Name, "scheme1User")], + context.Scheme.Name)); + context.Success(); + return Task.CompletedTask; + } + }; + }) + .AddCertificate("scheme2", options => + { + options.AllowedCertificateTypes = CertificateTypes.SelfSigned; + options.Events = new CertificateAuthenticationEvents + { + OnCertificateValidated = context => + { + scheme2ValidationCount++; + context.Fail("Certificate does not meet scheme2 requirements"); + return Task.CompletedTask; + } + }; + }) + .AddCertificateCache(); + })) + .Build(); + + await host.StartAsync(); + + using var server = host.GetTestServer(); + var client = server.CreateClient(); + + // 1. Authenticate with the scheme1 — should succeed and cache + var response1 = await client.GetAsync("https://example.com/?scheme=scheme1"); + Assert.Equal(HttpStatusCode.OK, response1.StatusCode); + Assert.Equal(1, scheme1ValidationCount); + + // 2. Authenticate with the scheme2 — must NOT reuse the scheme1's cached success + var response2 = await client.GetAsync("https://example.com/?scheme=scheme2"); + Assert.Equal(HttpStatusCode.Forbidden, response2.StatusCode); + Assert.Equal(1, scheme2ValidationCount); + + // 3. Authenticate with the scheme1 again — should use the cache (no re-validation) + var response3 = await client.GetAsync("https://example.com/?scheme=scheme1"); + Assert.Equal(HttpStatusCode.OK, response3.StatusCode); + Assert.Equal(1, scheme1ValidationCount); // Still 1 — served from cache + } + + [Fact] + public void VerifyCacheNoOpsWithoutSchemeInHttpContextItems() + { + var cache = new CertificateValidationCache(Options.Create(new CertificateValidationCacheOptions())); + var certificate = Certificates.SelfSignedValidWithNoEku; + + var httpContext = new DefaultHttpContext(); + // Do NOT set httpContext.Items[CertificateAuthenticationHandler.CertificateSchemeCacheKeyItem] + + // Put should no-op (no scheme = no cache key) + var successResult = AuthenticateResult.Success( + new AuthenticationTicket( + new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim(ClaimTypes.Name, "test") }, "test")), + "test")); + cache.Put(httpContext, certificate, successResult); + + // Get should return null even after Put + var cached = cache.Get(httpContext, certificate); + Assert.Null(cached); + + // Now set the scheme item — Put and Get should work + httpContext.Items[CertificateAuthenticationHandler.CertificateSchemeCacheKeyItem] = "MyScheme"; + cache.Put(httpContext, certificate, successResult); + + var cachedWithScheme = cache.Get(httpContext, certificate); + Assert.NotNull(cachedWithScheme); + Assert.True(cachedWithScheme.Succeeded); + } } diff --git a/src/Servers/HttpSys/src/RequestProcessing/Request.cs b/src/Servers/HttpSys/src/RequestProcessing/Request.cs index f02507251cab..e4e8ebcf92fb 100644 --- a/src/Servers/HttpSys/src/RequestProcessing/Request.cs +++ b/src/Servers/HttpSys/src/RequestProcessing/Request.cs @@ -19,6 +19,8 @@ namespace Microsoft.AspNetCore.Server.HttpSys; internal sealed partial class Request { + private static readonly bool AllowKeepAliveAfterCLTE = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.HttpSys.AllowKeepAliveAfterCLTE", out var value) && value; + private X509Certificate2? _clientCert; // TODO: https://github.com/aspnet/HttpSysServer/issues/231 // private byte[] _providedTokenBindingId; @@ -201,6 +203,8 @@ internal Request(RequestContext requestContext) private RequestContext RequestContext { get; } + public bool KeepAlive { get; private set; } = true; + // With the leading ?, if any public string QueryString { get; } @@ -503,22 +507,34 @@ private void RemoveContentLengthIfTransferEncodingContainsChunked() return; } - // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2 + // https://www.rfc-editor.org/rfc/rfc9112#section-6.2 // A sender MUST NOT send a Content-Length header field in any message // that contains a Transfer-Encoding header field. - // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3 + // https://www.rfc-editor.org/rfc/rfc9112#section-6.3-2.3 // If a message is received with both a Transfer-Encoding and a // Content-Length header field, the Transfer-Encoding overrides the - // Content-Length. Such a message might indicate an attempt to - // perform request smuggling (Section 9.5) or response splitting - // (Section 9.4) and ought to be handled as an error. A sender MUST - // remove the received Content-Length field prior to forwarding such - // a message downstream. + // Content-Length. Such a message might indicate an attempt to + // perform request smuggling (Section 11.2) or response splitting + // (Section 11.1) and ought to be handled as an error. An intermediary + // that chooses to forward the message MUST first remove the received + // Content-Length field and process the Transfer-Encoding + // (as described below) prior to forwarding the message downstream. // We should remove the Content-Length request header in this case, for compatibility - // reasons, include X-Content-Length so that the original Content-Length is still available. + // reasons, include x-Content-Length so that the original Content-Length is still available. IHeaderDictionary headerDictionary = Headers; headerDictionary.Add("X-Content-Length", headerDictionary[HeaderNames.ContentLength]); Headers.ContentLength = StringValues.Empty; + + if (!AllowKeepAliveAfterCLTE) + { + // https://www.rfc-editor.org/rfc/rfc9112#section-6.1 + // A server MAY reject a request that contains both Content-Length + // and Transfer-Encoding or process such a request in accordance + // with the Transfer-Encoding alone. Regardless, the server MUST + // close the connection after responding to such a request to + // avoid the potential attacks. + KeepAlive = false; + } } private static bool IsChunked(string? transferEncoding) diff --git a/src/Servers/HttpSys/src/RequestProcessing/Response.cs b/src/Servers/HttpSys/src/RequestProcessing/Response.cs index 0fb275227994..acb6b8b82dbc 100644 --- a/src/Servers/HttpSys/src/RequestProcessing/Response.cs +++ b/src/Servers/HttpSys/src/RequestProcessing/Response.cs @@ -403,7 +403,7 @@ internal uint ComputeHeaders(long writeCount, bool endOfRequest = false) var statusCanHaveBody = CanSendResponseBody(RequestContext.Response.StatusCode); // Determine if the connection will be kept alive or closed. - var keepConnectionAlive = true; + var keepConnectionAlive = Request.KeepAlive; // An HTTP/1.1 server may also establish persistent connections with // HTTP/1.0 clients upon receipt of a Keep-Alive connection token. diff --git a/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs b/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs index 8bb2f555e900..75612b15b037 100644 --- a/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs +++ b/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs @@ -482,6 +482,11 @@ private uint ComputeLeftToWrite(long writeCount, bool endOfRequest = false) } } + if (!_requestContext.Request.KeepAlive) + { + flags |= PInvoke.HTTP_SEND_RESPONSE_FLAG_DISCONNECT; + } + if (endOfRequest && _requestContext.Response.BoundaryType == BoundaryType.Close) { flags |= PInvoke.HTTP_SEND_RESPONSE_FLAG_DISCONNECT; diff --git a/src/Servers/HttpSys/test/FunctionalTests/RequestHeaderTests.cs b/src/Servers/HttpSys/test/FunctionalTests/RequestHeaderTests.cs index e32aeb750486..30c2e8164a6a 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/RequestHeaderTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/RequestHeaderTests.cs @@ -177,6 +177,79 @@ public async Task RequestHeaders_ClientSendTransferEncodingAndContentLength_Cont } } + [ConditionalFact] + public async Task CloseConnectionAfterProcessingContentLengthPlusChunkedRequest() + { + string address; + using (Utilities.CreateHttpServer(out address, async httpContext => + { + var requestHeaders = httpContext.Request.Headers; + var request = httpContext.Features.Get().Request; + Assert.Single(requestHeaders["Transfer-Encoding"]); + Assert.Equal("chunked", requestHeaders.TransferEncoding); + + Assert.Null(request.ContentLength); + Assert.True(request.HasEntityBody); + + Assert.False(requestHeaders.ContainsKey("Content-Length")); + Assert.Null(requestHeaders.ContentLength); + + Assert.Single(requestHeaders["X-Content-Length"]); + Assert.Equal("1", requestHeaders["X-Content-Length"]); + + await httpContext.Response.WriteAsync("Hello World"); + }, LoggerFactory)) + { + var uri = new Uri(address); + using (Socket socket = new Socket(SocketType.Stream, ProtocolType.Tcp)) + { + socket.Connect(uri.Host, uri.Port); + // Send 2 requests with both CL and TE header + // expect the second to not be processed and the connection to be closed + for (var i = 0; i < 2; i++) + { + socket.Send(Encoding.ASCII.GetBytes(string.Join("\r\n", + "POST / HTTP/1.1", + $"Host: {uri.Authority}", + "Transfer-Encoding: chunked", + "Connection: keep-alive", + "Content-Length: 1", + "", + "5", "Hello", + "6", " World", + "0", + "", + ""))); + } + byte[] response = new byte[1024 * 5]; + var sb = new StringBuilder(); + + int read = 0; + do + { + read = await Task.Run(() => socket.Receive(response)); + var s = Encoding.ASCII.GetString(response, 0, read); + sb.Append(s); + } while (read != 0); + + var resp = sb.ToString(); + + Assert.Matches(@"HTTP/1\.1 200 OK +Transfer-Encoding: chunked +Server: Microsoft-HTTPAPI/2\.0 +Date: .+ +Connection: close + +B +Hello World +0 + +$", + resp); + } + } + } + [ConditionalFact] public async Task RequestHeaders_AllKnownHeadersKeys_Received() { diff --git a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp index 23639860527a..040a91f573e3 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp @@ -478,6 +478,16 @@ http_close_connection( return S_OK; } +EXTERN_C __declspec(dllexport) +HRESULT +http_set_close( + _In_ IN_PROCESS_HANDLER* pInProcessHandler +) +{ + pInProcessHandler->QueryHttpContext()->GetResponse()->SetNeedDisconnect(); + return S_OK; +} + EXTERN_C __declspec(dllexport) HRESULT http_response_set_unknown_header( diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs index f0e04f3b2a95..f96a03fb0720 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs @@ -29,6 +29,8 @@ namespace Microsoft.AspNetCore.Server.IIS.Core; internal abstract partial class IISHttpContext : NativeRequestContext, IThreadPoolWorkItem, IDisposable { + private static readonly bool AllowKeepAliveAfterCLTE = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.IIS.AllowKeepAliveAfterCLTE", out var value) && value; + private const int MinAllocBufferSize = 2048; protected readonly NativeSafeHandle _requestNativeHandle; @@ -41,6 +43,7 @@ internal abstract partial class IISHttpContext : NativeRequestContext, IThreadPo private int _statusCode; private string? _reasonPhrase; + // Used to synchronize callback registration and native method calls internal readonly object _contextLock = new object(); @@ -367,23 +370,35 @@ private bool CheckRequestCanHaveBody() string transferEncoding = RequestHeaders.TransferEncoding.ToString(); if (IsChunked(transferEncoding)) { - // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2 + // https://www.rfc-editor.org/rfc/rfc9112#section-6.2 // A sender MUST NOT send a Content-Length header field in any message // that contains a Transfer-Encoding header field. - // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3 + // https://www.rfc-editor.org/rfc/rfc9112#section-6.3-2.3 // If a message is received with both a Transfer-Encoding and a // Content-Length header field, the Transfer-Encoding overrides the - // Content-Length. Such a message might indicate an attempt to - // perform request smuggling (Section 9.5) or response splitting - // (Section 9.4) and ought to be handled as an error. A sender MUST - // remove the received Content-Length field prior to forwarding such - // a message downstream. + // Content-Length. Such a message might indicate an attempt to + // perform request smuggling (Section 11.2) or response splitting + // (Section 11.1) and ought to be handled as an error. An intermediary + // that chooses to forward the message MUST first remove the received + // Content-Length field and process the Transfer-Encoding + // (as described below) prior to forwarding the message downstream. // We should remove the Content-Length request header in this case, for compatibility // reasons, include X-Content-Length so that the original Content-Length is still available. - if (RequestHeaders.ContentLength.HasValue) + if (RequestHeaders.TryGetValue(HeaderNames.ContentLength, out var contentLength)) { - RequestHeaders.Add("X-Content-Length", RequestHeaders[HeaderNames.ContentLength]); + RequestHeaders.Add("X-Content-Length", contentLength); RequestHeaders.ContentLength = null; + + if (!AllowKeepAliveAfterCLTE) + { + // https://www.rfc-editor.org/rfc/rfc9112#section-6.1 + // A server MAY reject a request that contains both Content-Length + // and Transfer-Encoding or process such a request in accordance + // with the Transfer-Encoding alone. Regardless, the server MUST + // close the connection after responding to such a request to + // avoid the potential attacks. + NativeMethods.HttpSetClose(_requestNativeHandle); + } } return true; } diff --git a/src/Servers/IIS/IIS/src/NativeMethods.cs b/src/Servers/IIS/IIS/src/NativeMethods.cs index 6aa3d68ed35e..1fe994ec4ff4 100644 --- a/src/Servers/IIS/IIS/src/NativeMethods.cs +++ b/src/Servers/IIS/IIS/src/NativeMethods.cs @@ -143,6 +143,9 @@ private static unsafe partial int http_websockets_write_bytes( [LibraryImport(AspNetCoreModuleDll)] private static partial int http_close_connection(NativeSafeHandle pInProcessHandler); + [LibraryImport(AspNetCoreModuleDll)] + private static partial int http_set_close(NativeSafeHandle pInProcessHandler); + [LibraryImport(AspNetCoreModuleDll)] private static partial int http_response_set_need_goaway(NativeSafeHandle pInProcessHandler); @@ -306,6 +309,11 @@ public static void HttpCloseConnection(NativeSafeHandle pInProcessHandler) Validate(http_close_connection(pInProcessHandler)); } + public static void HttpSetClose(NativeSafeHandle pInProcessHandler) + { + Validate(http_set_close(pInProcessHandler)); + } + public static unsafe void HttpResponseSetUnknownHeader(NativeSafeHandle pInProcessHandler, byte* pszHeaderName, byte* pszHeaderValue, ushort usHeaderValueLength, bool fReplace) { Validate(http_response_set_unknown_header(pInProcessHandler, pszHeaderName, pszHeaderValue, usHeaderValueLength, fReplace)); diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs index 5181cee0f75f..d8a1d0a624a7 100644 --- a/src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs @@ -10,8 +10,8 @@ using System.Net.Http.Headers; using System.Text; using System.Threading.Tasks; -using Microsoft.AspNetCore.Server.IntegrationTesting; using Microsoft.AspNetCore.InternalTesting; +using Microsoft.AspNetCore.Server.IntegrationTesting; using Xunit; #if !IIS_FUNCTIONALS @@ -789,6 +789,50 @@ await connection.Receive( } } + [ConditionalFact] + public async Task CloseConnectionAfterProcessingContentLengthPlusChunkedRequest() + { + using (var connection = _fixture.CreateTestConnection()) + { + for (var i = 0; i < 2; i++) + { + await connection.Send( + "POST /ReadAndWriteEcho HTTP/1.1", + "Host: localhost", + "Transfer-Encoding: chunked", + "Connection: keep-alive", + "Content-Length: 25", + "", + "5", "Hello", + "6", " World", + "0", + "", + ""); + } + + var recv = await connection.Receive(4096); + var recvString = Encoding.UTF8.GetString(recv.Span); + + Assert.Matches(@"HTTP/1\.1 200 OK +Server: Microsoft-IIS/10\.0 +Date: .+ +Connection: close + +Hello World + +$", + recvString); + + // Double check that there is no second response since we're using + // Assert.Matches which only checks for a first match + Assert.Equal(0, recvString.LastIndexOf("HTTP/1", StringComparison.InvariantCultureIgnoreCase)); + + // Verify the connection is closed + recv = await connection.Receive(100); + Assert.Equal(0, recv.Length); + } + } + private async Task<(int Status, string Body)> SendSocketRequestAsync(string path) { using (var connection = _fixture.CreateTestConnection()) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs index d14c25e49910..178ba3be43b1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs @@ -15,6 +15,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; internal abstract class Http1MessageBody : MessageBody { + private static readonly bool ContinueProcessingAfterCLTE = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.AllowKeepAliveAfterCLTE", out var value) && value; + protected readonly Http1Connection _context; private bool _readerCompleted; @@ -166,17 +168,18 @@ public static MessageBody For( KestrelBadHttpRequestException.Throw(RequestRejectionReason.FinalTransferCodingNotChunked, transferEncoding); } - // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2 + // https://www.rfc-editor.org/rfc/rfc9112#section-6.2 // A sender MUST NOT send a Content-Length header field in any message // that contains a Transfer-Encoding header field. - // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3 + // https://www.rfc-editor.org/rfc/rfc9112#section-6.3-2.3 // If a message is received with both a Transfer-Encoding and a // Content-Length header field, the Transfer-Encoding overrides the - // Content-Length. Such a message might indicate an attempt to - // perform request smuggling (Section 9.5) or response splitting - // (Section 9.4) and ought to be handled as an error. A sender MUST - // remove the received Content-Length field prior to forwarding such - // a message downstream. + // Content-Length. Such a message might indicate an attempt to + // perform request smuggling (Section 11.2) or response splitting + // (Section 11.1) and ought to be handled as an error. An intermediary + // that chooses to forward the message MUST first remove the received + // Content-Length field and process the Transfer-Encoding + // (as described below) prior to forwarding the message downstream. // We should remove the Content-Length request header in this case, for compatibility // reasons, include x-Content-Length so that the original Content-Length is still available. if (headers.ContentLength.HasValue) @@ -184,6 +187,17 @@ public static MessageBody For( IHeaderDictionary headerDictionary = headers; headerDictionary.Add("X-Content-Length", headerDictionary[HeaderNames.ContentLength]); headers.ContentLength = null; + + if (!ContinueProcessingAfterCLTE) + { + // https://www.rfc-editor.org/rfc/rfc9112#section-6.1 + // A server MAY reject a request that contains both Content-Length + // and Transfer-Encoding or process such a request in accordance + // with the Transfer-Encoding alone. Regardless, the server MUST + // close the connection after responding to such a request to + // avoid the potential attacks. + keepAlive = false; + } } // TODO may push more into the wrapper rather than just calling into the message body diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedRequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedRequestTests.cs index 7859716d6ff1..0b62c480575c 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedRequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedRequestTests.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport; +using Microsoft.DotNet.RemoteExecutor; using Microsoft.Extensions.Diagnostics.Metrics.Testing; using Microsoft.Extensions.Logging; using BadHttpRequestException = Microsoft.AspNetCore.Server.Kestrel.Core.BadHttpRequestException; @@ -119,7 +120,7 @@ private async Task PipeApp(HttpContext httpContext) } } - private async Task AppChunked(HttpContext httpContext) + private static async Task AppChunked(HttpContext httpContext) { var request = httpContext.Request; var response = httpContext.Response; @@ -1264,4 +1265,88 @@ await connection.ReceiveEnd( } } } + + [Fact] + public async Task CloseConnectionAfterProcessingContentLengthPlusChunkedRequest() + { + var testContext = new TestServiceContext(LoggerFactory); + + await using (var server = new TestServer(AppChunked, testContext)) + { + using (var connection = server.CreateConnection()) + { + for (var i = 0; i < 2; i++) + { + await connection.Send( + "POST / HTTP/1.1", + "Host:", + "Transfer-Encoding: chunked", + "Connection: keep-alive", + "Content-Length: 7", + "", + "5", "Hello", + "6", " World", + "0", + "", + ""); + } + + await connection.ReceiveEnd( + "HTTP/1.1 200 OK", + "Content-Length: 11", + "Connection: close", + $"Date: {testContext.DateHeaderValue}", + "", + "Hello World"); + } + } + } + + [ConditionalFact] + [RemoteExecutionSupported] + public void CanKeepProcessingRequestsAfterContentLengthPlusChunkedRequest_WithAppContext() + { + var options = new RemoteInvokeOptions(); + options.RuntimeConfigurationOptions.Add("Microsoft.AspNetCore.Server.Kestrel.AllowKeepAliveAfterCLTE", "true"); + + using var remoteHandle = RemoteExecutor.Invoke(static async () => + { + var testContext = new TestServiceContext(); + + await using (var server = new TestServer(AppChunked, testContext)) + { + using (var connection = server.CreateConnection()) + { + for (var i = 0; i < 2; i++) + { + await connection.Send( + "POST / HTTP/1.1", + "Host:", + "Transfer-Encoding: chunked", + "Connection: keep-alive", + "Content-Length: 7", + "", + "5", "Hello", + "6", " World", + "0", + "", + ""); + } + + await connection.Receive( + "HTTP/1.1 200 OK", + "Content-Length: 11", + $"Date: {testContext.DateHeaderValue}", + "", + "Hello World"); + await connection.Receive( + "HTTP/1.1 200 OK", + "Content-Length: 11", + $"Date: {testContext.DateHeaderValue}", + "", + "Hello World"); + } + } + }, options); + } }