From aacacbc25abc5b4c464483e6d8c34c317bd1c692 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Sun, 30 Jul 2023 19:48:56 -0700 Subject: [PATCH 1/2] Add support for options in form-binding and anti-forgery --- .../Metadata/FormMappingOptionsMetadata.cs | 25 ++ .../src/Metadata/IFormOptionsMetadata.cs | 76 +++++ .../src/PublicAPI.Unshipped.txt | 16 + .../src/RequestDelegateFactory.cs | 47 ++- ...RequestDelegateFactoryTests.FormMapping.cs | 172 ++++++++++ src/Http/Http/src/Features/FormFeature.cs | 3 + .../Http/src/Microsoft.AspNetCore.Http.csproj | 1 + ...tingEndpointConventionBuilderExtensions.cs | 59 ++++ .../Routing/src/EndpointRoutingMiddleware.cs | 48 +++ .../src/Internal/FormOptionsMetadata.cs | 28 ++ .../Internal/FormOptionsMetadataExtensions.cs | 26 ++ .../src/Microsoft.AspNetCore.Routing.csproj | 4 + src/Http/Routing/src/PublicAPI.Unshipped.txt | 2 + ...ntiforgeryTests.cs => MinimalFormTests.cs} | 203 +++++++++++- ...RoutingApplicationBuilderExtensionsTest.cs | 2 +- ...ndpointRoutingMiddlewareFormOptionsTest.cs | 232 ++++++++++++++ .../test/UnitTests/RoutingMetricsTests.cs | 18 +- .../TestObjects/TestServiceProvider.cs | 5 +- .../src/RequestFormLimitsAttribute.cs | 23 +- .../AntiforgeryMiddlewareTest.cs | 296 ++++++++++++++++++ ...soft.AspNetCore.Mvc.FunctionalTests.csproj | 4 + 21 files changed, 1271 insertions(+), 19 deletions(-) create mode 100644 src/Http/Http.Abstractions/src/Metadata/FormMappingOptionsMetadata.cs create mode 100644 src/Http/Http.Abstractions/src/Metadata/IFormOptionsMetadata.cs create mode 100644 src/Http/Http.Extensions/test/RequestDelegateFactoryTests.FormMapping.cs create mode 100644 src/Http/Routing/src/Internal/FormOptionsMetadata.cs create mode 100644 src/Http/Routing/src/Internal/FormOptionsMetadataExtensions.cs rename src/Http/Routing/test/FunctionalTests/{AntiforgeryTests.cs => MinimalFormTests.cs} (68%) create mode 100644 src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareFormOptionsTest.cs create mode 100644 src/Mvc/test/Mvc.FunctionalTests/AntiforgeryMiddlewareTest.cs diff --git a/src/Http/Http.Abstractions/src/Metadata/FormMappingOptionsMetadata.cs b/src/Http/Http.Abstractions/src/Metadata/FormMappingOptionsMetadata.cs new file mode 100644 index 000000000000..384f72161426 --- /dev/null +++ b/src/Http/Http.Abstractions/src/Metadata/FormMappingOptionsMetadata.cs @@ -0,0 +1,25 @@ +// 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.Http.Metadata; + +/// +/// Supports configuring the behavior of form mapping in a minimal API. +/// +public class FormMappingOptionsMetadata(int? maxCollectionSize = null, int? maxRecursionDepth = null, int? maxKeySize = null) +{ + /// + /// Gets or sets the maximum number of elements allowed in a form collection. + /// + public int? MaxCollectionSize { get; } = maxCollectionSize; + + /// + /// Gets or sets the maximum depth allowed when recursively mapping form data. + /// + public int? MaxRecursionDepth { get; } = maxRecursionDepth; + + /// + /// Gets or sets the maximum size of the buffer used to read form data keys. + /// + public int? MaxKeySize { get; } = maxKeySize; +} diff --git a/src/Http/Http.Abstractions/src/Metadata/IFormOptionsMetadata.cs b/src/Http/Http.Abstractions/src/Metadata/IFormOptionsMetadata.cs new file mode 100644 index 000000000000..b5b4850b6f02 --- /dev/null +++ b/src/Http/Http.Abstractions/src/Metadata/IFormOptionsMetadata.cs @@ -0,0 +1,76 @@ +// 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.Http.Metadata; + +/// +/// Interface marking attributes that specify limits associated with reading a form. +/// +public interface IFormOptionsMetadata +{ + /// + /// Enables full request body buffering. Use this if multiple components need to read the raw stream. Defaults to false. + /// + bool? BufferBody { get; } + + /// + /// If BufferBody is enabled, this many bytes of the body will be buffered in memory. + /// If this threshold is exceeded then the buffer will be moved to a temp file on disk instead. + /// This also applies when buffering individual multipart section bodies. Defaults to 65,536 bytes, which is approximately 64KB. + /// + int? MemoryBufferThreshold { get; } + + /// + /// If BufferBody is enabled, this is the limit for the total number of bytes that will be buffered. + /// Forms that exceed this limit will throw an InvalidDataException when parsed. Defaults to 134,217,728 bytes, which is approximately 128MB. + /// + long? BufferBodyLengthLimit { get; } + + /// + /// A limit for the number of form entries to allow. Forms that exceed this limit will throw an InvalidDataException when parsed. Defaults to 1024. + /// + int? ValueCountLimit { get; } + + /// + /// A limit on the length of individual keys. Forms containing keys that + /// exceed this limit will throw an InvalidDataException when parsed. + /// Defaults to 2,048 bytes, which is approximately 2KB. + /// + int? KeyLengthLimit { get; } + + /// + /// A limit on the length of individual form values. Forms containing + /// values that exceed this limit will throw an InvalidDataException + /// when parsed. Defaults to 4,194,304 bytes, which is approximately 4MB. + /// + int? ValueLengthLimit { get; } + + /// + /// A limit for the length of the boundary identifier. Forms with boundaries + /// that exceed this limit will throw an InvalidDataException when parsed. + /// Defaults to 128 bytes. + /// + int? MultipartBoundaryLengthLimit { get; } + + /// + /// A limit for the number of headers to allow in each multipart section. + /// Headers with the same name will be combined. Form sections that exceed + /// this limit will throw an InvalidDataException when parsed. Defaults to 16. + /// + int? MultipartHeadersCountLimit { get; } + + /// + /// A limit for the total length of the header keys and values in each + /// multipart section. Form sections that exceed this limit will throw + /// an InvalidDataException when parsed. Defaults to 16,384 bytes, + /// which is approximately 16KB. + /// + int? MultipartHeadersLengthLimit { get; } + + /// + /// /A limit for the length of each multipart body. Forms sections that + /// exceed this limit will throw an InvalidDataException when parsed. + /// Defaults to 134,217,728 bytes, which is approximately 128MB. + /// + long? MultipartBodyLengthLimit { get; } +} diff --git a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt index de149f734dde..3ac3bc3a4d15 100644 --- a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt @@ -13,6 +13,22 @@ Microsoft.AspNetCore.Http.Metadata.AcceptsMetadata.AcceptsMetadata(string![]! co Microsoft.AspNetCore.Http.Metadata.AcceptsMetadata.ContentTypes.get -> System.Collections.Generic.IReadOnlyList! Microsoft.AspNetCore.Http.Metadata.AcceptsMetadata.IsOptional.get -> bool Microsoft.AspNetCore.Http.Metadata.AcceptsMetadata.RequestType.get -> System.Type? +Microsoft.AspNetCore.Http.Metadata.FormMappingOptionsMetadata +Microsoft.AspNetCore.Http.Metadata.FormMappingOptionsMetadata.FormMappingOptionsMetadata(int? maxCollectionSize = null, int? maxRecursionDepth = null, int? maxKeySize = null) -> void +Microsoft.AspNetCore.Http.Metadata.FormMappingOptionsMetadata.MaxCollectionSize.get -> int? +Microsoft.AspNetCore.Http.Metadata.FormMappingOptionsMetadata.MaxKeySize.get -> int? +Microsoft.AspNetCore.Http.Metadata.FormMappingOptionsMetadata.MaxRecursionDepth.get -> int? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.BufferBody.get -> bool? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.BufferBodyLengthLimit.get -> long? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.KeyLengthLimit.get -> int? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.MemoryBufferThreshold.get -> int? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.MultipartBodyLengthLimit.get -> long? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.MultipartBoundaryLengthLimit.get -> int? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.MultipartHeadersCountLimit.get -> int? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.MultipartHeadersLengthLimit.get -> int? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.ValueCountLimit.get -> int? +Microsoft.AspNetCore.Http.Metadata.IFormOptionsMetadata.ValueLengthLimit.get -> int? Microsoft.AspNetCore.Http.Metadata.IRouteDiagnosticsMetadata Microsoft.AspNetCore.Http.Metadata.IRouteDiagnosticsMetadata.Route.get -> string! Microsoft.AspNetCore.Http.ProblemDetailsContext.Exception.get -> System.Exception? diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index a3d9182ebb02..26e39643c027 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -39,7 +39,6 @@ namespace Microsoft.AspNetCore.Http; public static partial class RequestDelegateFactory { private static readonly ParameterBindingMethodCache ParameterBindingMethodCache = new(); - private static readonly FormDataMapperOptions FormDataMapperOptions = new(); private static readonly MethodInfo ExecuteTaskWithEmptyResultMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteTaskWithEmptyResult), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo ExecuteValueTaskWithEmptyResultMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteValueTaskWithEmptyResult), BindingFlags.NonPublic | BindingFlags.Static)!; @@ -333,6 +332,11 @@ private static IReadOnlyList AsReadOnlyList(IList metadata) // inference is skipped internally if necessary. factoryContext.ArgumentExpressions ??= CreateArgumentsAndInferMetadata(methodInfo, factoryContext); + // Although we can re-use the cached argument expressions for most cases, parameters that are bound + // using the new form mapping logic are a special exception because we need to account for the `FormOptionsMetadata` + // added to the builder _during_ the construction of the parameter binding. + UpdateFormBindingArgumentExpressions(factoryContext); + factoryContext.MethodCall = CreateMethodCall(methodInfo, targetExpression, factoryContext.ArgumentExpressions); EndpointFilterDelegate? filterPipeline = null; var returnType = methodInfo.ReturnType; @@ -753,7 +757,7 @@ private static Expression CreateArgument(ParameterInfo parameter, RequestDelegat (parameter.ParameterType.IsArray && ParameterBindingMethodCache.HasTryParseMethod(parameter.ParameterType.GetElementType()!)); return useSimpleBinding ? BindParameterFromFormItem(parameter, formAttribute.Name ?? parameter.Name, factoryContext) - : BindComplexParameterFromFormItem(parameter, formAttribute.Name ?? parameter.Name, factoryContext); + : BindComplexParameterFromFormItem(parameter, string.IsNullOrEmpty(formAttribute.Name) ? parameter.Name : formAttribute.Name, factoryContext); } else if (parameter.CustomAttributes.Any(a => typeof(IFromServiceMetadata).IsAssignableFrom(a.AttributeType))) { @@ -1982,14 +1986,40 @@ private static Expression BindParameterFromFormItem( "form"); } + private static void UpdateFormBindingArgumentExpressions(RequestDelegateFactoryContext factoryContext) + { + if (factoryContext.ArgumentExpressions == null || factoryContext.ArgumentExpressions.Length == 0) + { + return; + } + + for (var i = 0; i < factoryContext.ArgumentExpressions.Length; i++) + { + var parameter = factoryContext.Parameters[i]; + var key = parameter.Name!; + if (factoryContext.TrackedParameters.TryGetValue(key, out var trackedParameter) && trackedParameter == RequestDelegateFactoryConstants.FormBindingAttribute) + { + factoryContext.ArgumentExpressions[i] = BindComplexParameterFromFormItem(parameter, key, factoryContext); + } + } + } + private static Expression BindComplexParameterFromFormItem( ParameterInfo parameter, string key, RequestDelegateFactoryContext factoryContext) { factoryContext.FirstFormRequestBodyParameter ??= parameter; - factoryContext.TrackedParameters.Add(key, RequestDelegateFactoryConstants.FormAttribute); + factoryContext.TrackedParameters.TryAdd(key, RequestDelegateFactoryConstants.FormBindingAttribute); factoryContext.ReadForm = true; + var formDataMapperOptions = new FormDataMapperOptions(); + var formMappingOptionsMetadatas = factoryContext.EndpointBuilder.Metadata.OfType(); + foreach (var formMappingOptionsMetadata in formMappingOptionsMetadatas) + { + formDataMapperOptions.MaxRecursionDepth = formMappingOptionsMetadata.MaxRecursionDepth ?? formDataMapperOptions.MaxRecursionDepth; + formDataMapperOptions.MaxCollectionSize = formMappingOptionsMetadata.MaxCollectionSize ?? formDataMapperOptions.MaxCollectionSize; + formDataMapperOptions.MaxKeyBufferSize = formMappingOptionsMetadata.MaxKeySize ?? formDataMapperOptions.MaxKeyBufferSize; + } // var name_local; // var name_reader; @@ -2001,19 +2031,19 @@ private static Expression BindComplexParameterFromFormItem( var formBuffer = Expression.Variable(typeof(char[]), "form_buffer"); // ProcessForm(context.Request.Form, form_dict, form_buffer); - var processFormExpr = Expression.Call(ProcessFormMethod, FormExpr, formDict, formBuffer); + var processFormExpr = Expression.Call(ProcessFormMethod, FormExpr, Expression.Constant(formDataMapperOptions.MaxKeyBufferSize), formDict, formBuffer); // name_reader = new FormDataReader(form_dict, CultureInfo.InvariantCulture, form_buffer.AsMemory(0, FormDataMapperOptions.MaxKeyBufferSize)); var initializeReaderExpr = Expression.Assign( formReader, Expression.New(FormDataReaderConstructor, formDict, Expression.Constant(CultureInfo.InvariantCulture), - Expression.Call(AsMemoryMethod, formBuffer, Expression.Constant(0), Expression.Constant(FormDataMapperOptions.MaxKeyBufferSize)))); + Expression.Call(AsMemoryMethod, formBuffer, Expression.Constant(0), Expression.Constant(formDataMapperOptions.MaxKeyBufferSize)))); // FormDataMapper.Map(name_reader, FormDataMapperOptions); var invokeMapMethodExpr = Expression.Call( FormDataMapperMapMethod.MakeGenericMethod(parameter.ParameterType), formReader, - Expression.Constant(FormDataMapperOptions)); + Expression.Constant(formDataMapperOptions)); // if (form_buffer != null) // { // ArrayPool.Shared.Return(form_buffer, false); @@ -2039,7 +2069,7 @@ private static Expression BindComplexParameterFromFormItem( ); } - private static void ProcessForm(IFormCollection form, ref IReadOnlyDictionary formDictionary, ref char[] buffer) + private static void ProcessForm(IFormCollection form, int maxKeyBufferSize, ref IReadOnlyDictionary formDictionary, ref char[] buffer) { var dictionary = new Dictionary(); foreach (var (key, value) in form) @@ -2047,7 +2077,7 @@ private static void ProcessForm(IFormCollection form, ref IReadOnlyDictionary.Shared.Rent(FormDataMapperOptions.MaxKeyBufferSize); + buffer = ArrayPool.Shared.Rent(maxKeyBufferSize); } private static Expression BindParameterFromFormFiles( @@ -2447,6 +2477,7 @@ private static class RequestDelegateFactoryConstants public const string ServiceAttribute = "Service (Attribute)"; public const string FormFileAttribute = "Form File (Attribute)"; public const string FormAttribute = "Form (Attribute)"; + public const string FormBindingAttribute = "Form Binding (Attribute)"; public const string RouteParameter = "Route (Inferred)"; public const string QueryStringParameter = "Query String (Inferred)"; public const string ServiceParameter = "Services (Inferred)"; diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.FormMapping.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.FormMapping.cs new file mode 100644 index 000000000000..d96b9c973f6f --- /dev/null +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.FormMapping.cs @@ -0,0 +1,172 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System.Globalization; +using Microsoft.AspNetCore.Components.Endpoints.FormMapping; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Metadata; +using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Primitives; + +namespace Microsoft.AspNetCore.Routing.Internal; + +public partial class RequestDelegateFactoryTests : LoggedTest +{ + [Fact] + public async Task SupportsFormMappingOptionsInMetadata() + { + // Arrange + static void TestAction([FromForm] Dictionary args) { } + var options = new RequestDelegateFactoryOptions + { + EndpointBuilder = CreateEndpointBuilder(new List() + { + new FormMappingOptionsMetadata(maxCollectionSize: 2) + }), + }; + var metadataResult = new RequestDelegateMetadataResult { EndpointMetadata = new List() }; + var httpContext = CreateHttpContext(); + httpContext.Request.Form = new FormCollection(new Dictionary + { + { + "[name1]", "value1" + }, + { + "[name2]", "value2" + }, + { + "[name3]", "value3" + }, + { + "[name4]", "value4" + }, + { + "[name5]", "value5" + }, + { + "[name6]", "value6" + } + }); + + 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.Error.Message.ToString(CultureInfo.InvariantCulture)); + } + + [Fact] + public async Task SupportsFormMappingOptionsInMetadataFormFormWithAttributeName() + { + // Arrange + static void TestAction([FromForm(Name = "shouldSetKeyCorrectly")] Dictionary args) { } + var options = new RequestDelegateFactoryOptions + { + EndpointBuilder = CreateEndpointBuilder(new List() + { + new FormMappingOptionsMetadata(maxCollectionSize: 2) + }), + }; + var metadataResult = new RequestDelegateMetadataResult { EndpointMetadata = new List() }; + var httpContext = CreateHttpContext(); + httpContext.Request.Form = new FormCollection(new Dictionary + { + { + "[name1]", "value1" + }, + { + "[name2]", "value2" + }, + { + "[name3]", "value3" + }, + { + "[name4]", "value4" + }, + { + "[name5]", "value5" + }, + { + "[name6]", "value6" + } + }); + + 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.Error.Message.ToString(CultureInfo.InvariantCulture)); + } + + [Fact] + public async Task SupportsMergingFormMappingOptionsInMetadata() + { + // Arrange + static void TestAction([FromForm] Dictionary args) { } + var options = new RequestDelegateFactoryOptions + { + EndpointBuilder = CreateEndpointBuilder(new List() + { + new FormMappingOptionsMetadata(maxCollectionSize: 2), + new FormMappingOptionsMetadata(maxKeySize: 23) + }), + }; + var metadataResult = new RequestDelegateMetadataResult { EndpointMetadata = new List() }; + var httpContext = CreateHttpContext(); + httpContext.Request.Form = new FormCollection(new Dictionary + { + { + "[name1]", "value1" + }, + { + "[name2]", "value2" + }, + { + "[name3]", "value3" + }, + { + "[name4]", "value4" + }, + { + "[name5]", "value5" + }, + { + "[name6]", "value6" + } + }); + + 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.Error.Message.ToString(CultureInfo.InvariantCulture)); + + // Arrange - 2 + httpContext = CreateHttpContext(); + httpContext.Request.Form = new FormCollection(new Dictionary + { + { + "[name1name1name1name1name1name1name1]", "value1" + }, + { + "[name2name2name2name2name2name2name2]", "value2" + }, + }); + + // Act - 2 + var anotherException = await Assert.ThrowsAsync(async () => await requestDelegate(httpContext)); + + // Assert - 2 + Assert.Equal("Specified argument was out of the range of valid values.", anotherException.Message); + + } +} diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index 2c8cca1b9886..e0ca4a21b105 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -56,6 +56,9 @@ public FormFeature(HttpRequest request, FormOptions options) _options = options; } + // Internal for testing. + internal FormOptions FormOptions => _options; + private MediaTypeHeaderValue? ContentType { get diff --git a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj index fd043ae6e330..ecd36ad92a8c 100644 --- a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj +++ b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj @@ -39,5 +39,6 @@ + diff --git a/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs b/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs index b414c55fddae..7fa497ac34d4 100644 --- a/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.WebUtilities; namespace Microsoft.AspNetCore.Builder; @@ -133,7 +134,65 @@ public static TBuilder WithGroupName(this TBuilder builder, string end /// The . public static TBuilder DisableAntiforgery(this TBuilder builder) where TBuilder : IEndpointConventionBuilder { + ArgumentNullException.ThrowIfNull(builder); + builder.WithMetadata(AntiforgeryMetadata.ValidationNotRequired); return builder; } + + /// + /// Configures for all endpoints produced + /// on the target . + /// + /// The . + /// The maximum number of elements allowed in a form collection. Defaults to >. + /// The maximum depth allowed when recursively mapping form data. Defaults to 64. + /// The maximum size of the buffer used to read form data keys. Defaults to + /// The . + public static TBuilder WithFormMappingOptions( + this TBuilder builder, + int? maxCollectionSize = null, + int? maxRecursionDepth = null, + int? maxKeySize = null) where TBuilder : IEndpointConventionBuilder + { + ArgumentNullException.ThrowIfNull(builder); + + builder.WithMetadata(new FormMappingOptionsMetadata(maxCollectionSize, maxRecursionDepth, maxKeySize)); + return builder; + } + + /// + /// Configures for all endpoints produced + /// on the target . + /// + /// The . + /// Enables full request body buffering. Defaults to false. + /// Configures how many bytes of the body will be buffered in memory. Defaults to 65,536 bytes, which is approximately 64KB. + /// Limit for the total number of bytes that will be buffered. Defaults to 128MB. + /// Limit for the number of form entries to allow. Defaults to . + /// Limit on the length of individual keys. Defaults to . + /// Limit on the length of individual form values. Defaults to . + /// Limit for the length of the boundary identifier. Defaults to 128 bytes. + /// Limit for the number of headers to allow in each multipart section. Defaults to . + /// Limit for the total length of the header keys and values in each multipart section. Defaults to . + /// Limit for the length of each multipart body. Defaults to 134,217,728 bytes, which is approximately 128MB. + /// The . + public static TBuilder WithFormOptions( + this TBuilder builder, + bool? bufferBody = null, + int? memoryBufferThreshold = null, + long? bufferBodyLengthLimit = null, + int? valueCountLimit = null, + int? keyLengthLimit = null, + int? valueLengthLimit = null, + int? multipartBoundaryLengthLimit = null, + int? multipartHeadersCountLimit = null, + int? multipartHeadersLengthLimit = null, + long? multipartBodyLengthLimit = null) where TBuilder : IEndpointConventionBuilder + { + ArgumentNullException.ThrowIfNull(builder); + + builder.WithMetadata(new FormOptionsMetadata(bufferBody, memoryBufferThreshold, bufferBodyLengthLimit, valueCountLimit, keyLengthLimit, valueLengthLimit, multipartBoundaryLengthLimit, multipartHeadersCountLimit, multipartHeadersLengthLimit, multipartBodyLengthLimit)); + return builder; + } } diff --git a/src/Http/Routing/src/EndpointRoutingMiddleware.cs b/src/Http/Routing/src/EndpointRoutingMiddleware.cs index 357add4f3412..bad134387628 100644 --- a/src/Http/Routing/src/EndpointRoutingMiddleware.cs +++ b/src/Http/Routing/src/EndpointRoutingMiddleware.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.ShortCircuit; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -136,6 +137,8 @@ private Task SetRoutingAndContinue(HttpContext httpContext) // We do this during endpoint routing to ensure that successive middlewares in the pipeline // can access the feature with the correct value. SetMaxRequestBodySize(httpContext); + // Map IFormOptionsMetadata to IFormFeature if present on the endpoint. + SetFormOptions(httpContext, endpoint); var shortCircuitMetadata = endpoint.Metadata.GetMetadata(); if (shortCircuitMetadata is not null) @@ -334,6 +337,45 @@ private void SetMaxRequestBodySize(HttpContext context) } } + private void SetFormOptions(HttpContext context, Endpoint endpoint) + { + var features = context.Features; + var formFeature = features.Get(); + + // Request form has not been read yet, so set the limits + if (formFeature == null || formFeature is { Form: null }) + { + var baseFormOptions = context.RequestServices.GetService>()?.Value ?? new FormOptions(); + var finalFormOptionsMetadata = new FormOptionsMetadata(); + var formOptionsMetadatas = endpoint.Metadata + .GetOrderedMetadata(); + foreach (var formOptionsMetadata in formOptionsMetadatas) + { + finalFormOptionsMetadata = finalFormOptionsMetadata.MergeWith(formOptionsMetadata); + } + + var formOptions = new FormOptions + { + BufferBody = finalFormOptionsMetadata.BufferBody ?? baseFormOptions.BufferBody, + MemoryBufferThreshold = finalFormOptionsMetadata.MemoryBufferThreshold ?? baseFormOptions.MemoryBufferThreshold, + BufferBodyLengthLimit = finalFormOptionsMetadata.BufferBodyLengthLimit ?? baseFormOptions.BufferBodyLengthLimit, + ValueCountLimit = finalFormOptionsMetadata.ValueCountLimit ?? baseFormOptions.ValueCountLimit, + KeyLengthLimit = finalFormOptionsMetadata.KeyLengthLimit ?? baseFormOptions.KeyLengthLimit, + ValueLengthLimit = finalFormOptionsMetadata.ValueLengthLimit ?? baseFormOptions.ValueLengthLimit, + MultipartBoundaryLengthLimit = finalFormOptionsMetadata.MultipartBoundaryLengthLimit ?? baseFormOptions.MultipartBoundaryLengthLimit, + MultipartHeadersCountLimit = finalFormOptionsMetadata.MultipartHeadersCountLimit ?? baseFormOptions.MultipartHeadersCountLimit, + MultipartHeadersLengthLimit = finalFormOptionsMetadata.MultipartHeadersLengthLimit ?? baseFormOptions.MultipartHeadersLengthLimit, + MultipartBodyLengthLimit = finalFormOptionsMetadata.MultipartBodyLengthLimit ?? baseFormOptions.MultipartBodyLengthLimit + }; + features.Set(new FormFeature(context.Request, formOptions)); + Log.AppliedFormOptions(_logger); + } + else + { + Log.CannotApplyFormOptions(_logger); + } + } + private static partial class Log { public static void MatchSuccess(ILogger logger, Endpoint endpoint) @@ -377,5 +419,11 @@ public static void MatchSkipped(ILogger logger, Endpoint endpoint) [LoggerMessage(12, LogLevel.Debug, "The maximum request body size has been disabled.", EventName = "MaxRequestBodySizeDisabled")] public static partial void MaxRequestBodySizeDisabled(ILogger logger); + + [LoggerMessage(13, LogLevel.Warning, "Unable to apply configured form options since the request form has already been read.", EventName = "CannotApplyFormOptions")] + public static partial void CannotApplyFormOptions(ILogger logger); + + [LoggerMessage(14, LogLevel.Debug, "Applied the configured form options on the current request.", EventName = "AppliedFormOptions")] + public static partial void AppliedFormOptions(ILogger logger); } } diff --git a/src/Http/Routing/src/Internal/FormOptionsMetadata.cs b/src/Http/Routing/src/Internal/FormOptionsMetadata.cs new file mode 100644 index 000000000000..e25e1e9fac3d --- /dev/null +++ b/src/Http/Routing/src/Internal/FormOptionsMetadata.cs @@ -0,0 +1,28 @@ +// 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.Http.Metadata; + +internal class FormOptionsMetadata( + bool? bufferBody = null, + int? memoryBufferThreshold = null, + long? bufferBodyLengthLimit = null, + int? valueCountLimit = null, + int? keyLengthLimit = null, + int? valueLengthLimit = null, + int? multipartBoundaryLengthLimit = null, + int? multipartHeadersCountLimit = null, + int? multipartHeadersLengthLimit = null, + long? multipartBodyLengthLimit = null) : IFormOptionsMetadata +{ + public bool? BufferBody { get; } = bufferBody; + public int? MemoryBufferThreshold { get; } = memoryBufferThreshold; + public long? BufferBodyLengthLimit { get; } = bufferBodyLengthLimit; + public int? ValueCountLimit { get; } = valueCountLimit; + public int? KeyLengthLimit { get; } = keyLengthLimit; + public int? ValueLengthLimit { get; } = valueLengthLimit; + public int? MultipartBoundaryLengthLimit { get; } = multipartBoundaryLengthLimit; + public int? MultipartHeadersCountLimit { get; } = multipartHeadersCountLimit; + public int? MultipartHeadersLengthLimit { get; } = multipartHeadersLengthLimit; + public long? MultipartBodyLengthLimit { get; } = multipartBodyLengthLimit; +} diff --git a/src/Http/Routing/src/Internal/FormOptionsMetadataExtensions.cs b/src/Http/Routing/src/Internal/FormOptionsMetadataExtensions.cs new file mode 100644 index 000000000000..a78cc71d2ff1 --- /dev/null +++ b/src/Http/Routing/src/Internal/FormOptionsMetadataExtensions.cs @@ -0,0 +1,26 @@ +// 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.Http.Metadata; + +namespace Microsoft.AspNetCore.Routing; + +internal static class FormOptionsMetadataExtensions +{ + public static FormOptionsMetadata MergeWith( + this IFormOptionsMetadata formOptionsMetadata, + IFormOptionsMetadata otherFormOptionsMetadata) + { + var _bufferBody = otherFormOptionsMetadata.BufferBody != null ? otherFormOptionsMetadata.BufferBody : formOptionsMetadata.BufferBody; + var _memoryBufferThreshold = otherFormOptionsMetadata.MemoryBufferThreshold != null ? otherFormOptionsMetadata.MemoryBufferThreshold : formOptionsMetadata.MemoryBufferThreshold; + var _bufferBodyLengthLimit = otherFormOptionsMetadata.BufferBodyLengthLimit != null ? otherFormOptionsMetadata.BufferBodyLengthLimit : formOptionsMetadata.BufferBodyLengthLimit; + var _valueCountLimit = otherFormOptionsMetadata.ValueCountLimit != null ? otherFormOptionsMetadata.ValueCountLimit : formOptionsMetadata.ValueCountLimit; + var _keyLengthLimit = otherFormOptionsMetadata.KeyLengthLimit != null ? otherFormOptionsMetadata.KeyLengthLimit : formOptionsMetadata.KeyLengthLimit; + var _valueLengthLimit = otherFormOptionsMetadata.ValueLengthLimit != null ? otherFormOptionsMetadata.ValueLengthLimit : formOptionsMetadata.ValueLengthLimit; + var _multipartBoundaryLengthLimit = otherFormOptionsMetadata.MultipartBoundaryLengthLimit != null ? otherFormOptionsMetadata.MultipartBoundaryLengthLimit : formOptionsMetadata.MultipartBoundaryLengthLimit; + var _multipartHeadersCountLimit = otherFormOptionsMetadata.MultipartHeadersCountLimit != null ? otherFormOptionsMetadata.MultipartHeadersCountLimit : formOptionsMetadata.MultipartHeadersCountLimit; + var _multipartHeadersLengthLimit = otherFormOptionsMetadata.MultipartHeadersLengthLimit != null ? otherFormOptionsMetadata.MultipartHeadersLengthLimit : formOptionsMetadata.MultipartHeadersLengthLimit; + var _multipartBodyLengthLimit = otherFormOptionsMetadata.MultipartBodyLengthLimit != null ? otherFormOptionsMetadata.MultipartBodyLengthLimit : formOptionsMetadata.MultipartBodyLengthLimit; + return new FormOptionsMetadata(_bufferBody, _memoryBufferThreshold, _bufferBodyLengthLimit, _valueCountLimit, _keyLengthLimit, _valueLengthLimit, _multipartBoundaryLengthLimit, _multipartHeadersCountLimit, _multipartHeadersLengthLimit, _multipartBodyLengthLimit); + } +} diff --git a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj index c89dd902787a..0595339a4d9b 100644 --- a/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj +++ b/src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj @@ -41,8 +41,12 @@ + + + + diff --git a/src/Http/Routing/src/PublicAPI.Unshipped.txt b/src/Http/Routing/src/PublicAPI.Unshipped.txt index b34723807c93..c9438c691b5f 100644 --- a/src/Http/Routing/src/PublicAPI.Unshipped.txt +++ b/src/Http/Routing/src/PublicAPI.Unshipped.txt @@ -12,6 +12,8 @@ Microsoft.AspNetCore.Builder.RouteShortCircuitEndpointConventionBuilderExtension Microsoft.AspNetCore.Routing.RouteShortCircuitEndpointRouteBuilderExtensions static Microsoft.AspNetCore.Builder.RouteShortCircuitEndpointConventionBuilderExtensions.ShortCircuit(this Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! builder, int? statusCode = null) -> Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! static Microsoft.AspNetCore.Builder.RoutingEndpointConventionBuilderExtensions.DisableAntiforgery(this TBuilder builder) -> TBuilder +static Microsoft.AspNetCore.Builder.RoutingEndpointConventionBuilderExtensions.WithFormMappingOptions(this TBuilder builder, int? maxCollectionSize = null, int? maxRecursionDepth = null, int? maxKeySize = null) -> TBuilder +static Microsoft.AspNetCore.Builder.RoutingEndpointConventionBuilderExtensions.WithFormOptions(this TBuilder builder, bool? bufferBody = null, int? memoryBufferThreshold = null, long? bufferBodyLengthLimit = null, int? valueCountLimit = null, int? keyLengthLimit = null, int? valueLengthLimit = null, int? multipartBoundaryLengthLimit = null, int? multipartHeadersCountLimit = null, int? multipartHeadersLengthLimit = null, long? multipartBodyLengthLimit = null) -> TBuilder static Microsoft.AspNetCore.Routing.RouteHandlerServices.Map(Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! handler, System.Collections.Generic.IEnumerable? httpMethods, System.Func! populateMetadata, System.Func! createRequestDelegate) -> Microsoft.AspNetCore.Builder.RouteHandlerBuilder! static Microsoft.AspNetCore.Routing.RouteShortCircuitEndpointRouteBuilderExtensions.MapShortCircuit(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! builder, int statusCode, params string![]! routePrefixes) -> Microsoft.AspNetCore.Builder.IEndpointConventionBuilder! static Microsoft.Extensions.DependencyInjection.RoutingServiceCollectionExtensions.AddRoutingCore(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! diff --git a/src/Http/Routing/test/FunctionalTests/AntiforgeryTests.cs b/src/Http/Routing/test/FunctionalTests/MinimalFormTests.cs similarity index 68% rename from src/Http/Routing/test/FunctionalTests/AntiforgeryTests.cs rename to src/Http/Routing/test/FunctionalTests/MinimalFormTests.cs index 3cdb604236a8..79f17ae158a6 100644 --- a/src/Http/Routing/test/FunctionalTests/AntiforgeryTests.cs +++ b/src/Http/Routing/test/FunctionalTests/MinimalFormTests.cs @@ -6,6 +6,7 @@ using System.Text.Json; using Microsoft.AspNetCore.Antiforgery; using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Components.Endpoints.FormMapping; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -16,7 +17,7 @@ using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Routing.FunctionalTests; -public class AntiforgeryTests +public class MinimalFormTests { private static readonly JsonSerializerOptions SerializerOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web); @@ -452,6 +453,206 @@ public async Task MapPost_WithForm_ValidToken_RequestSizeLimit_Works(bool hasLim } } + [Fact] + public async Task MapPost_WithForm_AndFormMapperOptions_ValidToken_Works() + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .Configure(app => + { + app.UseRouting(); + app.UseAntiforgery(); + app.UseEndpoints(b => + b.MapPost("/todo", ([FromForm] Dictionary todo) => todo) + .WithFormMappingOptions(maxCollectionSize: 2)); + }) + .UseTestServer(); + }) + .ConfigureServices(services => + { + services.AddRouting(); + services.AddAntiforgery(); + }) + .Build(); + + using var server = host.GetTestServer(); + await host.StartAsync(); + var client = server.CreateClient(); + + var antiforgery = host.Services.GetRequiredService(); + var antiforgeryOptions = host.Services.GetRequiredService>(); + var tokens = antiforgery.GetAndStoreTokens(new DefaultHttpContext()); + var request = new HttpRequestMessage(HttpMethod.Post, "todo"); + request.Headers.Add("Cookie", antiforgeryOptions.Value.Cookie.Name + "=" + tokens.CookieToken); + var nameValueCollection = new List> + { + new KeyValuePair("__RequestVerificationToken", tokens.RequestToken), + new KeyValuePair("[name]", "Test task"), + new KeyValuePair("[name1]", "Test task"), + new KeyValuePair("[isComplete]", "false"), + new KeyValuePair("[isComplete1]", "false"), + new KeyValuePair("[dueDate]", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + new KeyValuePair("[dueDate1]", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + }; + request.Content = new FormUrlEncodedContent(nameValueCollection); + + var exception = await Record.ExceptionAsync(async () => await client.SendAsync(request)); + Assert.NotNull(exception); + Assert.Contains("An error occurred while trying to map a value from form data.", exception.Message); + } + + [Fact] + public async Task SupportsMergingFormMappingOptionsFromGroupAndEndpoint() + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .Configure(app => + { + app.UseRouting(); + app.UseAntiforgery(); + app.UseEndpoints(b => + { + var g = b.MapGroup("/todos").WithFormMappingOptions(maxCollectionSize: 2); + g.MapPost("/1", ([FromForm] Dictionary todo) => todo) + .WithFormMappingOptions(maxCollectionSize: 7); + }); + }) + .UseTestServer(); + }) + .ConfigureServices(services => + { + services.AddRouting(); + services.AddAntiforgery(); + }) + .Build(); + + using var server = host.GetTestServer(); + await host.StartAsync(); + var client = server.CreateClient(); + + var antiforgery = host.Services.GetRequiredService(); + var antiforgeryOptions = host.Services.GetRequiredService>(); + var tokens = antiforgery.GetAndStoreTokens(new DefaultHttpContext()); + var request = new HttpRequestMessage(HttpMethod.Post, "/todos/1"); + request.Headers.Add("Cookie", antiforgeryOptions.Value.Cookie.Name + "=" + tokens.CookieToken); + var nameValueCollection = new List> + { + new KeyValuePair("__RequestVerificationToken", tokens.RequestToken), + new KeyValuePair("[name]", "Test task"), + new KeyValuePair("[name1]", "Test task"), + new KeyValuePair("[isComplete]", "false"), + new KeyValuePair("[isComplete1]", "false"), + new KeyValuePair("[dueDate]", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + new KeyValuePair("[dueDate1]", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + }; + request.Content = new FormUrlEncodedContent(nameValueCollection); + + var response = await client.SendAsync(request); + response.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task SupportsMergingFormOptionsFromGroupAndEndpoint() + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .Configure(app => + { + app.UseRouting(); + app.UseAntiforgery(); + app.UseEndpoints(b => + { + var g = b.MapGroup("/todos").WithFormOptions(valueCountLimit: 7); + g.MapPost("/1", ([FromForm] Dictionary todo) => todo) + .WithFormOptions(valueCountLimit: 2); + }); + }) + .UseTestServer(); + }) + .ConfigureServices(services => + { + services.AddRouting(); + services.AddAntiforgery(); + }) + .Build(); + + using var server = host.GetTestServer(); + await host.StartAsync(); + var client = server.CreateClient(); + + var antiforgery = host.Services.GetRequiredService(); + var antiforgeryOptions = host.Services.GetRequiredService>(); + var tokens = antiforgery.GetAndStoreTokens(new DefaultHttpContext()); + var request = new HttpRequestMessage(HttpMethod.Post, "/todos/1"); + request.Headers.Add("Cookie", antiforgeryOptions.Value.Cookie.Name + "=" + tokens.CookieToken); + var nameValueCollection = new List> + { + new KeyValuePair("__RequestVerificationToken", tokens.RequestToken), + new KeyValuePair("[name]", "Test task"), + new KeyValuePair("[name1]", "Test task"), + new KeyValuePair("[isComplete]", "false"), + new KeyValuePair("[isComplete1]", "false"), + new KeyValuePair("[dueDate]", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + new KeyValuePair("[dueDate1]", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + }; + request.Content = new FormUrlEncodedContent(nameValueCollection); + + var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task MapPost_WithForm_AndRequestLimits_ValidToken_Works() + { + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .Configure(app => + { + app.UseRouting(); + app.UseAntiforgery(); + app.UseEndpoints(b => + b.MapPost("/todo", ([FromForm] Todo todo) => todo) + .WithFormOptions(keyLengthLimit: 8)); + }) + .UseTestServer(); + }) + .ConfigureServices(services => + { + services.AddRouting(); + services.AddAntiforgery(); + }) + .Build(); + + using var server = host.GetTestServer(); + await host.StartAsync(); + var client = server.CreateClient(); + + var antiforgery = host.Services.GetRequiredService(); + var antiforgeryOptions = host.Services.GetRequiredService>(); + var tokens = antiforgery.GetAndStoreTokens(new DefaultHttpContext()); + var request = new HttpRequestMessage(HttpMethod.Post, "todo"); + request.Headers.Add("Cookie", antiforgeryOptions.Value.Cookie.Name + "=" + tokens.CookieToken); + var nameValueCollection = new List> + { + new KeyValuePair("__RequestVerificationToken", tokens.RequestToken), + new KeyValuePair("name", "Test task"), + new KeyValuePair("isComplete", "false"), + new KeyValuePair("dueDate", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + }; + request.Content = new FormUrlEncodedContent(nameValueCollection); + + var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + class Todo { public string Name { get; set; } diff --git a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs index e8924d7733e6..f9edf32c10e9 100644 --- a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs @@ -95,7 +95,7 @@ public async Task UseRouting_ServicesRegistered_Match_DoesNotSetsFeature() }); var appFunc = app.Build(); - var httpContext = new DefaultHttpContext(); + var httpContext = new DefaultHttpContext { RequestServices = services }; // Act await appFunc(httpContext); diff --git a/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareFormOptionsTest.cs b/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareFormOptionsTest.cs new file mode 100644 index 000000000000..2407eadf0a8b --- /dev/null +++ b/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareFormOptionsTest.cs @@ -0,0 +1,232 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Http.Metadata; +using Microsoft.AspNetCore.Routing.Matching; +using Microsoft.AspNetCore.Routing.TestObjects; +using Microsoft.AspNetCore.Testing; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Logging.Testing; +using Microsoft.Extensions.Options; +using Moq; + +namespace Microsoft.AspNetCore.Routing; + +public class EndpointRoutingMiddlewareFormOptionsTest +{ + [Fact] + public async Task LogsWhenFormOptionsAreNotApplied() + { + var sink = new TestSink(TestSink.EnableWithTypeName); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + var logger = new Logger(loggerFactory); + + var httpContext = CreateHttpContext(); + var presetFormFeature = new FormFeature(httpContext.Request, FormOptions.Default); + presetFormFeature.Form = new FormCollection(new()); + httpContext.Features.Set(presetFormFeature); + + var formOptionsMetadata = new FormOptionsMetadata(bufferBody: false, valueCountLimit: 54); + var middleware = CreateMiddleware( + logger: logger, + matcherFactory: new TestMatcherFactory(isHandled: true, setEndpointCallback: c => + { + c.SetEndpoint(new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(formOptionsMetadata), "myapp")); + })); + + // Act + await middleware.Invoke(httpContext); + + var formFeature = httpContext.Features.Get(); + var formOptions = Assert.IsType(formFeature).FormOptions; + // Configured values are set + Assert.False(formOptions.BufferBody); + Assert.Equal(FormReader.DefaultValueCountLimit, formOptions.ValueCountLimit); + // Unset values are set to default instead of null + Assert.Equal(FormOptions.DefaultMemoryBufferThreshold, formOptions.MemoryBufferThreshold); + + // Logs that FormOptions were not applied + var write = Assert.Single(sink.Writes.Where(w => w.EventId.Name == "CannotApplyFormOptions")); + Assert.Equal("Unable to apply configured form options since the request form has already been read.", write.Message); + } + + [Fact] + public async Task SupportsSettingFormOptionsFromMetadata() + { + var sink = new TestSink(TestSink.EnableWithTypeName); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + var logger = new Logger(loggerFactory); + + var httpContext = CreateHttpContext(); + + var formOptionsMetadata = new FormOptionsMetadata(bufferBody: false, valueCountLimit: 54); + var middleware = CreateMiddleware( + logger: logger, + matcherFactory: new TestMatcherFactory(isHandled: true, setEndpointCallback: c => + { + c.SetEndpoint(new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(formOptionsMetadata), "myapp")); + })); + + // Act + await middleware.Invoke(httpContext); + + var formFeature = httpContext.Features.Get(); + var formOptions = Assert.IsType(formFeature).FormOptions; + // Configured values are set + Assert.False(formOptions.BufferBody); + Assert.Equal(54, formOptions.ValueCountLimit); + // Unset values are set to default instead of null + Assert.Equal(FormOptions.DefaultMemoryBufferThreshold, formOptions.MemoryBufferThreshold); + + // Logs that FormOptions were applied + var write = Assert.Single(sink.Writes.Where(w => w.EventId.Name == "AppliedFormOptions")); + Assert.Equal("Applied the configured form options on the current request.", write.Message); + } + + [Fact] + public async Task SupportsMergingSettingsFromMultipleFormOptionsMetadata() + { + var sink = new TestSink(TestSink.EnableWithTypeName); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + var logger = new Logger(loggerFactory); + + var httpContext = CreateHttpContext(); + + var formOptionsMetadata1 = new FormOptionsMetadata(bufferBody: false, valueCountLimit: 54); + var formOptionsMetadata2 = new FormOptionsMetadata(valueLengthLimit: 45); + var formOptionsMetadata3 = new FormOptionsMetadata(bufferBody: true); + var middleware = CreateMiddleware( + logger: logger, + matcherFactory: new TestMatcherFactory(isHandled: true, setEndpointCallback: c => + { + c.SetEndpoint(new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(formOptionsMetadata1, formOptionsMetadata2, formOptionsMetadata3), "myapp")); + })); + + // Act + await middleware.Invoke(httpContext); + + var formFeature = httpContext.Features.Get(); + var formOptions = Assert.IsType(formFeature).FormOptions; + // Favor the most specific (last) value set for a property + Assert.True(formOptions.BufferBody); + Assert.Equal(54, formOptions.ValueCountLimit); + Assert.Equal(45, formOptions.ValueLengthLimit); + // Unset values are set to default instead of null + Assert.Equal(FormOptions.DefaultMemoryBufferThreshold, formOptions.MemoryBufferThreshold); + + // Logs that FormOptions were applied + var write = Assert.Single(sink.Writes.Where(w => w.EventId.Name == "AppliedFormOptions")); + Assert.Equal("Applied the configured form options on the current request.", write.Message); + } + + [Fact] + public async Task SupportsMergingSettingsFromMetadataAndServices() + { + var sink = new TestSink(TestSink.EnableWithTypeName); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + var logger = new Logger(loggerFactory); + + var serviceCollection = new ServiceCollection(); + serviceCollection.Configure(options => + { + options.BufferBody = true; + options.ValueLengthLimit = 45; + }); + var httpContext = new DefaultHttpContext + { + RequestServices = serviceCollection.BuildServiceProvider() + }; + + var formOptionsMetadata = new FormOptionsMetadata(bufferBody: false, valueCountLimit: 54); + var middleware = CreateMiddleware( + logger: logger, + matcherFactory: new TestMatcherFactory(isHandled: true, setEndpointCallback: c => + { + c.SetEndpoint(new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(formOptionsMetadata), "myapp")); + })); + + // Act + await middleware.Invoke(httpContext); + + var formFeature = httpContext.Features.Get(); + var formOptions = Assert.IsType(formFeature).FormOptions; + // Favor the most specific (last) value set for a property + Assert.False(formOptions.BufferBody); + Assert.Equal(54, formOptions.ValueCountLimit); + Assert.Equal(45, formOptions.ValueLengthLimit); + // Unset values are set to default instead of null + Assert.Equal(FormOptions.DefaultMemoryBufferThreshold, formOptions.MemoryBufferThreshold); + } + + [Fact] + public async Task SettingEndpointManuallyDoesNotOverwriteOptions() + { + var sink = new TestSink(TestSink.EnableWithTypeName); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + var logger = new Logger(loggerFactory); + var httpContext = CreateHttpContext(); + var endpointMetadata = new FormOptionsMetadata(bufferBody: true, valueCountLimit: 70); + var endpoint = new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(endpointMetadata), "myapp"); + + var formOptionsMetadata = new FormOptionsMetadata(bufferBody: false, valueCountLimit: 54); + var middleware = CreateMiddleware( + logger: logger, + matcherFactory: new TestMatcherFactory(isHandled: true, setEndpointCallback: c => + { + c.SetEndpoint(new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(formOptionsMetadata), "myapp")); + })); + + // Act + await middleware.Invoke(httpContext); + httpContext.SetEndpoint(endpoint); + + var formFeature = httpContext.Features.Get(); + var formOptions = Assert.IsType(formFeature).FormOptions; + // Favor the most specific (last) value set for a property + Assert.False(formOptions.BufferBody); + Assert.Equal(54, formOptions.ValueCountLimit); + // Unset values are set to default instead of null + Assert.Equal(FormOptions.DefaultMemoryBufferThreshold, formOptions.MemoryBufferThreshold); + } + + private HttpContext CreateHttpContext() + { + var httpContext = new DefaultHttpContext + { + RequestServices = new TestServiceProvider() + }; + + return httpContext; + } + + private EndpointRoutingMiddleware CreateMiddleware( + ILogger logger = null, + MatcherFactory matcherFactory = null, + DiagnosticListener listener = null, + RequestDelegate next = null) + { + next ??= c => Task.CompletedTask; + logger ??= new Logger(NullLoggerFactory.Instance); + matcherFactory ??= new TestMatcherFactory(true); + listener ??= new DiagnosticListener("Microsoft.AspNetCore"); + var metrics = new RoutingMetrics(new TestMeterFactory()); + + var middleware = new EndpointRoutingMiddleware( + matcherFactory, + logger, + new DefaultEndpointRouteBuilder(Mock.Of()), + new DefaultEndpointDataSource(), + listener, + Options.Create(new RouteOptions()), + metrics, + next); + + return middleware; + } +} diff --git a/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs b/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs index 18ed872ce99a..b4ea027bf4b0 100644 --- a/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs +++ b/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs @@ -31,7 +31,7 @@ public async Task Match_Success() c.SetEndpoint(routeEndpointBuilder.Build()); }), meterFactory: meterFactory); - var httpContext = new DefaultHttpContext(); + var httpContext = CreateHttpContext(); var meter = meterFactory.Meters.Single(); using var routingMatchAttemptsCollector = new MetricCollector(meterFactory, RoutingMetrics.MeterName, "aspnetcore.routing.match_attempts"); @@ -65,7 +65,7 @@ public async Task Match_SuccessFallback_SetTagIfPresent(bool hasFallbackMetadata c.SetEndpoint(routeEndpointBuilder.Build()); }), meterFactory: meterFactory); - var httpContext = new DefaultHttpContext(); + var httpContext = CreateHttpContext(); var meter = meterFactory.Meters.Single(); using var routingMatchAttemptsCollector = new MetricCollector(meterFactory, RoutingMetrics.MeterName, "aspnetcore.routing.match_attempts"); @@ -92,7 +92,7 @@ public async Task Match_Success_MissingRoute() c.SetEndpoint(new Endpoint(c => Task.CompletedTask, EndpointMetadataCollection.Empty, "Test name")); }), meterFactory: meterFactory); - var httpContext = new DefaultHttpContext(); + var httpContext = CreateHttpContext(); var meter = meterFactory.Meters.Single(); using var routingMatchAttemptsCollector = new MetricCollector(meterFactory, RoutingMetrics.MeterName, "aspnetcore.routing.match_attempts"); @@ -116,7 +116,7 @@ public async Task Match_Failure() var middleware = CreateMiddleware( matcherFactory: new TestMatcherFactory(false), meterFactory: meterFactory); - var httpContext = new DefaultHttpContext(); + var httpContext = CreateHttpContext(); var meter = meterFactory.Meters.Single(); using var routingMatchAttemptsCollector = new MetricCollector(meterFactory, RoutingMetrics.MeterName, "aspnetcore.routing.match_attempts"); @@ -171,4 +171,14 @@ private EndpointRoutingMiddleware CreateMiddleware( return middleware; } + + private HttpContext CreateHttpContext() + { + var httpContext = new DefaultHttpContext + { + RequestServices = new TestServiceProvider() + }; + + return httpContext; + } } diff --git a/src/Http/Routing/test/UnitTests/TestObjects/TestServiceProvider.cs b/src/Http/Routing/test/UnitTests/TestObjects/TestServiceProvider.cs index 1d45e841c6f4..ec4dec2022e9 100644 --- a/src/Http/Routing/test/UnitTests/TestObjects/TestServiceProvider.cs +++ b/src/Http/Routing/test/UnitTests/TestObjects/TestServiceProvider.cs @@ -5,8 +5,5 @@ namespace Microsoft.AspNetCore.Routing.TestObjects; internal class TestServiceProvider : IServiceProvider { - public object GetService(Type serviceType) - { - throw new NotImplementedException(); - } + public object GetService(Type serviceType) => null; } diff --git a/src/Mvc/Mvc.Core/src/RequestFormLimitsAttribute.cs b/src/Mvc/Mvc.Core/src/RequestFormLimitsAttribute.cs index a766675041e5..7b5757a2680c 100644 --- a/src/Mvc/Mvc.Core/src/RequestFormLimitsAttribute.cs +++ b/src/Mvc/Mvc.Core/src/RequestFormLimitsAttribute.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.DependencyInjection; @@ -12,7 +13,7 @@ namespace Microsoft.AspNetCore.Mvc; /// Sets the specified limits to the . /// [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] -public class RequestFormLimitsAttribute : Attribute, IFilterFactory, IOrderedFilter +public class RequestFormLimitsAttribute : Attribute, IFilterFactory, IOrderedFilter, IFormOptionsMetadata { /// /// Gets the order value for determining the order of execution of filters. Filters execute in @@ -48,6 +49,8 @@ public bool BufferBody set => FormOptions.BufferBody = value; } + bool? IFormOptionsMetadata.BufferBody => BufferBody; + /// /// If is enabled, this many bytes of the body will be buffered in memory. /// If this threshold is exceeded then the buffer will be moved to a temp file on disk instead. @@ -59,6 +62,8 @@ public int MemoryBufferThreshold set => FormOptions.MemoryBufferThreshold = value; } + int? IFormOptionsMetadata.MemoryBufferThreshold => MemoryBufferThreshold; + /// /// If is enabled, this is the limit for the total number of bytes that will /// be buffered. Forms that exceed this limit will throw an when parsed. @@ -69,6 +74,8 @@ public long BufferBodyLengthLimit set => FormOptions.BufferBodyLengthLimit = value; } + long? IFormOptionsMetadata.BufferBodyLengthLimit => BufferBodyLengthLimit; + /// /// A limit for the number of form entries to allow. /// Forms that exceed this limit will throw an when parsed. @@ -79,6 +86,8 @@ public int ValueCountLimit set => FormOptions.ValueCountLimit = value; } + int? IFormOptionsMetadata.ValueCountLimit => ValueCountLimit; + /// /// A limit on the length of individual keys. Forms containing keys that exceed this limit will /// throw an when parsed. @@ -89,6 +98,8 @@ public int KeyLengthLimit set => FormOptions.KeyLengthLimit = value; } + int? IFormOptionsMetadata.KeyLengthLimit => KeyLengthLimit; + /// /// A limit on the length of individual form values. Forms containing values that exceed this /// limit will throw an when parsed. @@ -99,6 +110,8 @@ public int ValueLengthLimit set => FormOptions.ValueLengthLimit = value; } + int? IFormOptionsMetadata.ValueLengthLimit => ValueLengthLimit; + /// /// A limit for the length of the boundary identifier. Forms with boundaries that exceed this /// limit will throw an when parsed. @@ -109,6 +122,8 @@ public int MultipartBoundaryLengthLimit set => FormOptions.MultipartBoundaryLengthLimit = value; } + int? IFormOptionsMetadata.MultipartBoundaryLengthLimit => MultipartBoundaryLengthLimit; + /// /// A limit for the number of headers to allow in each multipart section. Headers with the same name will /// be combined. Form sections that exceed this limit will throw an @@ -120,6 +135,8 @@ public int MultipartHeadersCountLimit set => FormOptions.MultipartHeadersCountLimit = value; } + int? IFormOptionsMetadata.MultipartHeadersCountLimit => MultipartHeadersCountLimit; + /// /// A limit for the total length of the header keys and values in each multipart section. /// Form sections that exceed this limit will throw an when parsed. @@ -130,6 +147,8 @@ public int MultipartHeadersLengthLimit set => FormOptions.MultipartHeadersLengthLimit = value; } + int? IFormOptionsMetadata.MultipartHeadersLengthLimit => MultipartHeadersLengthLimit; + /// /// A limit for the length of each multipart body. Forms sections that exceed this limit will throw an /// when parsed. @@ -140,6 +159,8 @@ public long MultipartBodyLengthLimit set => FormOptions.MultipartBodyLengthLimit = value; } + long? IFormOptionsMetadata.MultipartBodyLengthLimit => MultipartBodyLengthLimit; + /// public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) { diff --git a/src/Mvc/test/Mvc.FunctionalTests/AntiforgeryMiddlewareTest.cs b/src/Mvc/test/Mvc.FunctionalTests/AntiforgeryMiddlewareTest.cs new file mode 100644 index 000000000000..9de73734ac5a --- /dev/null +++ b/src/Mvc/test/Mvc.FunctionalTests/AntiforgeryMiddlewareTest.cs @@ -0,0 +1,296 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Globalization; +using System.Net; +using System.Net.Http; +using System.Reflection; +using Microsoft.AspNetCore.Antiforgery; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Mvc.ApplicationParts; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests; + +public class AntiforgeryMiddlewareTest +{ + [Fact] + public async Task Works_WithAntiforgeryMetadata_ValidToken() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.AddMvcCore().UseSpecificControllers(typeof(TestController)); + builder.Services.AddAntiforgery(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + app.UseAntiforgery(); + app.MapControllers(); + + await app.StartAsync(); + + var client = app.GetTestClient(); + var antiforgery = app.Services.GetRequiredService(); + var antiforgeryOptions = app.Services.GetRequiredService>(); + var tokens = antiforgery.GetAndStoreTokens(new DefaultHttpContext()); + + var request = new HttpRequestMessage(HttpMethod.Post, "/Test/PostWithRequireAntiforgeryToken"); + request.Headers.Add("Cookie", antiforgeryOptions.Value.Cookie.Name + "=" + tokens.CookieToken); + var nameValueCollection = new List> + { + new("__RequestVerificationToken", tokens.RequestToken), + new("name", "Test task"), + new("isComplete", "false"), + new("dueDate", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + }; + request.Content = new FormUrlEncodedContent(nameValueCollection); + var result = await client.SendAsync(request); + result.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task Works_WithAntiforgeryMetadata_AndFilterAttribute_ValidToken() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.AddMvcCore().UseSpecificControllers(typeof(TestWithBothAttributesController)).AddViews(); + builder.Services.AddAntiforgery(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + app.UseAntiforgery(); + var exception = Assert.Throws(() => app.MapControllers()); + + Assert.Equal("Cannot apply [ValidateAntiForgeryTokenAttribute] and [RequireAntiforgeryTokenAttribute] at the same time.", exception.Message); + } + + [Fact] + public async Task Works_WithAntiforgeryMetadata_InvalidToken() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.AddMvcCore().UseSpecificControllers(typeof(TestController)).AddViews(); + builder.Services.AddAntiforgery(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + app.UseAntiforgery(); + app.MapControllers(); + + await app.StartAsync(); + + var client = app.GetTestClient(); + + var request = new HttpRequestMessage(HttpMethod.Post, "/Test/PostWithRequireAntiforgeryToken"); + var nameValueCollection = new List> + { + new("name", "Test task"), + new("isComplete", "false"), + new("dueDate", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + }; + request.Content = new FormUrlEncodedContent(nameValueCollection); + var result = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode); + } + + [Fact] + public async Task Works_WithAntiforgeryMetadata_ValidToken_RequestSizeLimit() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.AddMvcCore().UseSpecificControllers(typeof(TestController)); + builder.Services.AddAntiforgery(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + app.Use((context, next) => + { + context.Features.Set(new FakeHttpMaxRequestBodySizeFeature(5_000_000)); + return next(context); + }); + app.UseRouting(); + app.Use((context, next) => + { + context.Request.Body = new SizeLimitedStream(context.Request.Body, context.Features.Get()?.MaxRequestBodySize); + return next(context); + }); + app.UseAntiforgery(); + app.MapControllers(); + + await app.StartAsync(); + + var client = app.GetTestClient(); + var antiforgery = app.Services.GetRequiredService(); + var antiforgeryOptions = app.Services.GetRequiredService>(); + var tokens = antiforgery.GetAndStoreTokens(new DefaultHttpContext()); + + var request = new HttpRequestMessage(HttpMethod.Post, "/Test/PostWithRequireAntiforgeryTokenAndSizeLimit"); + request.Headers.Add("Cookie", antiforgeryOptions.Value.Cookie.Name + "=" + tokens.CookieToken); + var nameValueCollection = new List> + { + new("__RequestVerificationToken", tokens.RequestToken), + new("name", "Test task"), + new("isComplete", "false"), + new("dueDate", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + }; + request.Content = new FormUrlEncodedContent(nameValueCollection); + var exception = await Assert.ThrowsAsync(async () => await client.SendAsync(request)); + Assert.Equal("The maximum number of bytes have been read.", exception.Message); + } + + [Fact] + public async Task Throws_WithAntiforgeryMetadata_ValidToken_RequestFormLimits() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.AddMvcCore().UseSpecificControllers(typeof(TestController)).AddViews(); + builder.Services.AddAntiforgery(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + app.UseAntiforgery(); + app.MapControllers(); + + await app.StartAsync(); + + var client = app.GetTestClient(); + var antiforgery = app.Services.GetRequiredService(); + var antiforgeryOptions = app.Services.GetRequiredService>(); + var tokens = antiforgery.GetAndStoreTokens(new DefaultHttpContext()); + + var request = new HttpRequestMessage(HttpMethod.Post, "/Test/PostWithRequireAntiforgeryTokenAndFormLimit"); + request.Headers.Add("Cookie", antiforgeryOptions.Value.Cookie.Name + "=" + tokens.CookieToken); + var nameValueCollection = new List> + { + new("__RequestVerificationToken", tokens.RequestToken), + new("name", "Test task"), + new("isComplete", "false"), + new("dueDate", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + }; + request.Content = new FormUrlEncodedContent(nameValueCollection); + var result = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.BadRequest, result.StatusCode); + } + + [Fact] + public async Task Works_WithAntiforgeryMetadata_ValidToken_DisableRequestSizeLimits() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.AddMvcCore().UseSpecificControllers(typeof(TestWithRequestSizeLimitController)); + builder.Services.AddAntiforgery(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + app.UseAntiforgery(); + app.MapControllers(); + + await app.StartAsync(); + + var client = app.GetTestClient(); + var antiforgery = app.Services.GetRequiredService(); + var antiforgeryOptions = app.Services.GetRequiredService>(); + var tokens = antiforgery.GetAndStoreTokens(new DefaultHttpContext()); + + var request = new HttpRequestMessage(HttpMethod.Post, "/TestWithRequestSizeLimit/PostWithRequireAntiforgeryTokenAndDisableSizeLimit"); + request.Headers.Add("Cookie", antiforgeryOptions.Value.Cookie.Name + "=" + tokens.CookieToken); + var nameValueCollection = new List> + { + new("__RequestVerificationToken", tokens.RequestToken), + new("name", "Test task"), + new("isComplete", "false"), + new("dueDate", DateTime.Today.AddDays(1).ToString(CultureInfo.InvariantCulture)), + }; + request.Content = new FormUrlEncodedContent(nameValueCollection); + var result = await client.SendAsync(request); + result.EnsureSuccessStatusCode(); + } + + [Route("[controller]/[action]")] + [ApiController] + public class TestController : ControllerBase + { + [HttpPost] + [RequireAntiforgeryToken] + public ActionResult PostWithRequireAntiforgeryToken([FromForm] Todo todo) + => new OkObjectResult(todo); + + [HttpPost] + [RequireAntiforgeryToken] + [RequestSizeLimit(4)] + public ActionResult PostWithRequireAntiforgeryTokenAndSizeLimit([FromForm] Todo todo) + => new OkObjectResult(todo); + + [HttpPost] + [RequireAntiforgeryToken] + [RequestFormLimits(ValueCountLimit = 2)] + public ActionResult PostWithRequireAntiforgeryTokenAndFormLimit([FromForm] Todo todo) + => new OkObjectResult(todo); + } + + [Route("[controller]/[action]")] + [ApiController] + public class TestWithBothAttributesController : ControllerBase + { + [HttpPost] + [RequireAntiforgeryToken] + [ValidateAntiForgeryToken] + public ActionResult PostWithRequireAntiforgeryTokenAndFilterAttribute([FromForm] Todo todo) + => new OkObjectResult(todo); + } + + [Route("[controller]/[action]")] + [ApiController] + [RequestSizeLimit(4)] + public class TestWithRequestSizeLimitController : ControllerBase + { + [HttpPost] + [RequireAntiforgeryToken] + public ActionResult PostWithRequireAntiforgeryTokenAndDisableSizeLimit([FromForm] Todo todo) + => new OkObjectResult(todo); + } + + public class Todo + { + public string Name { get; set; } = string.Empty; + public bool IsCompleted { get; set; } = false; + public DateTime DueDate { get; set; } = DateTime.Now.Add(TimeSpan.FromDays(1)); + } + + public class FakeHttpMaxRequestBodySizeFeature : IHttpMaxRequestBodySizeFeature + { + public FakeHttpMaxRequestBodySizeFeature( + long? maxRequestBodySize = null, + bool isReadOnly = false) + { + MaxRequestBodySize = maxRequestBodySize; + IsReadOnly = isReadOnly; + } + public bool IsReadOnly { get; } + public long? MaxRequestBodySize { get; set; } + } +} + +internal static class MvCoreBuilderExtensions +{ + internal static void UseSpecificControllers( + this ApplicationPartManager partManager, + params Type[] controllerTypes) + { + partManager.FeatureProviders.Add(new TestControllerFeatureProvider()); + partManager.ApplicationParts.Clear(); + partManager.ApplicationParts.Add(new SelectedControllersApplicationParts(controllerTypes)); + } + + internal static IMvcCoreBuilder UseSpecificControllers( + this IMvcCoreBuilder mvcCoreBuilder, + params Type[] controllerTypes) => mvcCoreBuilder + .ConfigureApplicationPartManager(partManager => partManager.UseSpecificControllers(controllerTypes)); +} + +internal class SelectedControllersApplicationParts(Type[] types) : ApplicationPart, IApplicationPartTypeProvider +{ + public override string Name { get; } = string.Empty; + + public IEnumerable Types { get; } = types.Select(x => x.GetTypeInfo()).ToArray(); +} + +internal class TestControllerFeatureProvider : ControllerFeatureProvider +{ + // Default controller feature provider doesn't support nested controller classes + // so we override that here + protected override bool IsController(TypeInfo typeInfo) => true; +} diff --git a/src/Mvc/test/Mvc.FunctionalTests/Microsoft.AspNetCore.Mvc.FunctionalTests.csproj b/src/Mvc/test/Mvc.FunctionalTests/Microsoft.AspNetCore.Mvc.FunctionalTests.csproj index ef1661548dab..436507d6a9d5 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/Microsoft.AspNetCore.Mvc.FunctionalTests.csproj +++ b/src/Mvc/test/Mvc.FunctionalTests/Microsoft.AspNetCore.Mvc.FunctionalTests.csproj @@ -8,6 +8,7 @@ + @@ -28,6 +29,9 @@ + + + From de7b841a653bec58a82f8a9dc50de29f9715ee8d Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 8 Aug 2023 14:38:22 -0700 Subject: [PATCH 2/2] Address feedback from review --- .../EndpointRoutingShortCircuitBenchmark.cs | 3 +++ .../Routing/src/EndpointRoutingMiddleware.cs | 6 ++++-- ...ndpointRoutingMiddlewareFormOptionsTest.cs | 19 ++++++------------- .../EndpointRoutingMiddlewareTest.cs | 1 + .../test/UnitTests/RoutingMetricsTests.cs | 2 ++ 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/Http/Routing/perf/Microbenchmarks/EndpointRoutingShortCircuitBenchmark.cs b/src/Http/Routing/perf/Microbenchmarks/EndpointRoutingShortCircuitBenchmark.cs index 2adc33b73084..66fefa98ad27 100644 --- a/src/Http/Routing/perf/Microbenchmarks/EndpointRoutingShortCircuitBenchmark.cs +++ b/src/Http/Routing/perf/Microbenchmarks/EndpointRoutingShortCircuitBenchmark.cs @@ -5,6 +5,7 @@ using BenchmarkDotNet.Attributes; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.ShortCircuit; using Microsoft.AspNetCore.Testing; @@ -32,6 +33,7 @@ public void Setup() new BenchmarkEndpointDataSource(), new DiagnosticListener("benchmark"), Options.Create(new RouteOptions()), + Options.Create(new FormOptions()), routingMetrics, context => Task.CompletedTask); @@ -44,6 +46,7 @@ public void Setup() new BenchmarkEndpointDataSource(), new DiagnosticListener("benchmark"), Options.Create(new RouteOptions()), + Options.Create(new FormOptions()), routingMetrics, context => Task.CompletedTask); } diff --git a/src/Http/Routing/src/EndpointRoutingMiddleware.cs b/src/Http/Routing/src/EndpointRoutingMiddleware.cs index bad134387628..abf193a77182 100644 --- a/src/Http/Routing/src/EndpointRoutingMiddleware.cs +++ b/src/Http/Routing/src/EndpointRoutingMiddleware.cs @@ -13,7 +13,6 @@ using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.ShortCircuit; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -30,6 +29,7 @@ internal sealed partial class EndpointRoutingMiddleware private readonly RoutingMetrics _metrics; private readonly RequestDelegate _next; private readonly RouteOptions _routeOptions; + private readonly FormOptions _formOptions; private Task? _initializationTask; public EndpointRoutingMiddleware( @@ -39,6 +39,7 @@ public EndpointRoutingMiddleware( EndpointDataSource rootCompositeEndpointDataSource, DiagnosticListener diagnosticListener, IOptions routeOptions, + IOptions formOptions, RoutingMetrics metrics, RequestDelegate next) { @@ -50,6 +51,7 @@ public EndpointRoutingMiddleware( _metrics = metrics; _next = next ?? throw new ArgumentNullException(nameof(next)); _routeOptions = routeOptions.Value; + _formOptions = formOptions.Value; // rootCompositeEndpointDataSource is a constructor parameter only so it always gets disposed by DI. This ensures that any // disposable EndpointDataSources also get disposed. _endpointDataSource is a component of rootCompositeEndpointDataSource. @@ -345,7 +347,7 @@ private void SetFormOptions(HttpContext context, Endpoint endpoint) // Request form has not been read yet, so set the limits if (formFeature == null || formFeature is { Form: null }) { - var baseFormOptions = context.RequestServices.GetService>()?.Value ?? new FormOptions(); + var baseFormOptions = _formOptions; var finalFormOptionsMetadata = new FormOptionsMetadata(); var formOptionsMetadatas = endpoint.Metadata .GetOrderedMetadata(); diff --git a/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareFormOptionsTest.cs b/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareFormOptionsTest.cs index 2407eadf0a8b..d26a291edefb 100644 --- a/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareFormOptionsTest.cs +++ b/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareFormOptionsTest.cs @@ -131,17 +131,7 @@ public async Task SupportsMergingSettingsFromMetadataAndServices() var sink = new TestSink(TestSink.EnableWithTypeName); var loggerFactory = new TestLoggerFactory(sink, enabled: true); var logger = new Logger(loggerFactory); - - var serviceCollection = new ServiceCollection(); - serviceCollection.Configure(options => - { - options.BufferBody = true; - options.ValueLengthLimit = 45; - }); - var httpContext = new DefaultHttpContext - { - RequestServices = serviceCollection.BuildServiceProvider() - }; + var httpContext = CreateHttpContext(); var formOptionsMetadata = new FormOptionsMetadata(bufferBody: false, valueCountLimit: 54); var middleware = CreateMiddleware( @@ -149,7 +139,8 @@ public async Task SupportsMergingSettingsFromMetadataAndServices() matcherFactory: new TestMatcherFactory(isHandled: true, setEndpointCallback: c => { c.SetEndpoint(new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(formOptionsMetadata), "myapp")); - })); + }), + formOptions: new FormOptions { BufferBody = true, ValueLengthLimit = 45}); // Act await middleware.Invoke(httpContext); @@ -209,7 +200,8 @@ private EndpointRoutingMiddleware CreateMiddleware( ILogger logger = null, MatcherFactory matcherFactory = null, DiagnosticListener listener = null, - RequestDelegate next = null) + RequestDelegate next = null, + FormOptions formOptions = null) { next ??= c => Task.CompletedTask; logger ??= new Logger(NullLoggerFactory.Instance); @@ -224,6 +216,7 @@ private EndpointRoutingMiddleware CreateMiddleware( new DefaultEndpointDataSource(), listener, Options.Create(new RouteOptions()), + Options.Create(formOptions ?? new FormOptions()), metrics, next); diff --git a/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareTest.cs b/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareTest.cs index b244e77f12e6..d056d7fdc168 100644 --- a/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareTest.cs +++ b/src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareTest.cs @@ -449,6 +449,7 @@ private EndpointRoutingMiddleware CreateMiddleware( new DefaultEndpointDataSource(), listener, Options.Create(new RouteOptions()), + Options.Create(new FormOptions()), metrics, next); diff --git a/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs b/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs index b4ea027bf4b0..fa83c7f3bc43 100644 --- a/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs +++ b/src/Http/Routing/test/UnitTests/RoutingMetricsTests.cs @@ -5,6 +5,7 @@ using System.Diagnostics.Metrics; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Routing.TestObjects; @@ -166,6 +167,7 @@ private EndpointRoutingMiddleware CreateMiddleware( new DefaultEndpointDataSource(), listener, Options.Create(new RouteOptions()), + Options.Create(new FormOptions()), metrics, next);