Resolve FormOptionsMetadata to FormOptions when reading form (#50166)

* Resolve FormOptionsMetadata to FormOptions when reading form

* React to peer review

* Pass FormOptionsMetadata as ref struct

Author: captainsafia

src/Http/Http/src/Features/FormFeature.cs

@@ -4,6 +4,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Text;
 using Microsoft.AspNetCore.Antiforgery;
+using Microsoft.AspNetCore.Http.Metadata;
 using Microsoft.AspNetCore.WebUtilities;
 using Microsoft.Extensions.Primitives;
 using Microsoft.Net.Http.Headers;
@@ -16,7 +17,8 @@ namespace Microsoft.AspNetCore.Http.Features;
 public class FormFeature : IFormFeature
 {
     private readonly HttpRequest _request;
-    private readonly FormOptions _options;
+    private readonly Endpoint? _endpoint;
+    private FormOptions _options;
     private Task<IFormCollection>? _parsedFormTask;
     private IFormCollection? _form;
 
@@ -48,12 +50,18 @@ public FormFeature(HttpRequest request)
     /// <param name="request">The <see cref="HttpRequest"/>.</param>
     /// <param name="options">The <see cref="FormOptions"/>.</param>
     public FormFeature(HttpRequest request, FormOptions options)
+        : this(request, options, null)
+    {
+    }
+
+    internal FormFeature(HttpRequest request, FormOptions options, Endpoint? endpoint)
     {
         ArgumentNullException.ThrowIfNull(request);
         ArgumentNullException.ThrowIfNull(options);
 
         _request = request;
         _options = options;
+        _endpoint = endpoint;
     }
 
     // Internal for testing.
@@ -144,6 +152,7 @@ public Task<IFormCollection> ReadFormAsync(CancellationToken cancellationToken)
     private async Task<IFormCollection> InnerReadFormAsync(CancellationToken cancellationToken)
     {
         HandleUncheckedAntiforgeryValidationFeature();
+        _options = _endpoint is null ? _options : GetFormOptionsFromMetadata(_options, _endpoint);
 
         if (!HasFormContentType)
         {
@@ -345,4 +354,21 @@ private static string GetBoundary(MediaTypeHeaderValue contentType, int lengthLi
         }
         return boundary.ToString();
     }
+
+    private static FormOptions GetFormOptionsFromMetadata(FormOptions baseFormOptions, Endpoint endpoint)
+    {
+        var formOptionsMetadatas = endpoint.Metadata
+            .GetOrderedMetadata<IFormOptionsMetadata>();
+        var metadataCount = formOptionsMetadatas.Count;
+        if (metadataCount == 0)
+        {
+            return baseFormOptions;
+        }
+        var finalFormOptionsMetadata = new MutableFormOptionsMetadata(formOptionsMetadatas[metadataCount - 1]);
+        for (int i = metadataCount - 2; i >= 0; i--)
+        {
+            formOptionsMetadatas[i].MergeWith(ref finalFormOptionsMetadata);
+        }
+        return finalFormOptionsMetadata.ResolveFormOptions(baseFormOptions);
+    }
 }

src/Http/Http/src/Internal/DefaultHttpRequest.cs

@@ -15,7 +15,7 @@ internal sealed class DefaultHttpRequest : HttpRequest
     // Lambdas hoisted to static readonly fields to improve inlining https://github.com/dotnet/roslyn/issues/13624
     private static readonly Func<IFeatureCollection, IHttpRequestFeature?> _nullRequestFeature = f => null;
     private static readonly Func<IFeatureCollection, IQueryFeature?> _newQueryFeature = f => new QueryFeature(f);
-    private static readonly Func<DefaultHttpRequest, IFormFeature> _newFormFeature = r => new FormFeature(r, r._context.FormOptions ?? FormOptions.Default);
+    private static readonly Func<DefaultHttpRequest, IFormFeature> _newFormFeature = r => new FormFeature(r, r._context.FormOptions ?? FormOptions.Default, r._context.GetEndpoint());
     private static readonly Func<IFeatureCollection, IRequestCookiesFeature> _newRequestCookiesFeature = f => new RequestCookiesFeature(f);
     private static readonly Func<IFeatureCollection, IRouteValuesFeature> _newRouteValuesFeature = f => new RouteValuesFeature();
     private static readonly Func<HttpContext, IRequestBodyPipeFeature> _newRequestBodyPipeFeature = context => new RequestBodyPipeFeature(context);

src/Http/Http/src/Internal/FormOptionsMetadataExtensions.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.
+
+using Microsoft.AspNetCore.Http.Metadata;
+
+namespace Microsoft.AspNetCore.Http.Features;
+
+internal static class FormOptionsMetadataExtensions
+{
+    public static void MergeWith(
+        this IFormOptionsMetadata formOptionsMetadata,
+        ref MutableFormOptionsMetadata otherFormOptionsMetadata)
+    {
+        otherFormOptionsMetadata.BufferBody ??= formOptionsMetadata.BufferBody;
+        otherFormOptionsMetadata.MemoryBufferThreshold ??= formOptionsMetadata.MemoryBufferThreshold;
+        otherFormOptionsMetadata.BufferBodyLengthLimit ??= formOptionsMetadata.BufferBodyLengthLimit;
+        otherFormOptionsMetadata.ValueCountLimit ??= formOptionsMetadata.ValueCountLimit;
+        otherFormOptionsMetadata.KeyLengthLimit ??= formOptionsMetadata.KeyLengthLimit;
+        otherFormOptionsMetadata.ValueLengthLimit ??= formOptionsMetadata.ValueLengthLimit;
+        otherFormOptionsMetadata.MultipartBoundaryLengthLimit ??= formOptionsMetadata.MultipartBoundaryLengthLimit;
+        otherFormOptionsMetadata.MultipartHeadersCountLimit ??= formOptionsMetadata.MultipartHeadersCountLimit;
+        otherFormOptionsMetadata.MultipartHeadersLengthLimit ??= formOptionsMetadata.MultipartHeadersLengthLimit;
+        otherFormOptionsMetadata.MultipartBodyLengthLimit ??= formOptionsMetadata.MultipartBodyLengthLimit;
+    }
+}

src/Http/Http/src/Internal/MutableFormOptionsMetadata.cs

@@ -0,0 +1,33 @@
+// 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.Http.Features;
+
+internal struct MutableFormOptionsMetadata(IFormOptionsMetadata formOptionsMetadata) : IFormOptionsMetadata
+{
+    internal FormOptions ResolveFormOptions(FormOptions baseFormOptions) => new FormOptions
+    {
+        BufferBody = BufferBody ?? baseFormOptions.BufferBody,
+        MemoryBufferThreshold = MemoryBufferThreshold ?? baseFormOptions.MemoryBufferThreshold,
+        BufferBodyLengthLimit = BufferBodyLengthLimit ?? baseFormOptions.BufferBodyLengthLimit,
+        ValueCountLimit = ValueCountLimit ??  baseFormOptions.ValueCountLimit,
+        KeyLengthLimit = KeyLengthLimit ?? baseFormOptions.KeyLengthLimit,
+        ValueLengthLimit = ValueLengthLimit ?? baseFormOptions.ValueLengthLimit,
+        MultipartBoundaryLengthLimit = MultipartBoundaryLengthLimit ?? baseFormOptions.MultipartBoundaryLengthLimit,
+        MultipartHeadersCountLimit = MultipartHeadersCountLimit ?? baseFormOptions.MultipartHeadersCountLimit,
+        MultipartHeadersLengthLimit = MultipartHeadersLengthLimit ?? baseFormOptions.MultipartHeadersLengthLimit,
+        MultipartBodyLengthLimit = MultipartBodyLengthLimit ?? baseFormOptions.MultipartBodyLengthLimit
+    };
+
+    public bool? BufferBody { get; set; } = formOptionsMetadata.BufferBody;
+    public int? MemoryBufferThreshold { get; set; } = formOptionsMetadata.MemoryBufferThreshold;
+    public long? BufferBodyLengthLimit { get; set; } = formOptionsMetadata.BufferBodyLengthLimit;
+    public int? ValueCountLimit { get; set; } = formOptionsMetadata.ValueCountLimit;
+    public int? KeyLengthLimit { get; set; } = formOptionsMetadata.KeyLengthLimit;
+    public int? ValueLengthLimit { get; set; } = formOptionsMetadata.ValueLengthLimit;
+    public int? MultipartBoundaryLengthLimit { get; set; } = formOptionsMetadata.MultipartBoundaryLengthLimit;
+    public int? MultipartHeadersCountLimit { get; set; } = formOptionsMetadata.MultipartHeadersCountLimit;
+    public int? MultipartHeadersLengthLimit { get; set; } = formOptionsMetadata.MultipartHeadersLengthLimit;
+    public long? MultipartBodyLengthLimit { get; set; } = formOptionsMetadata.MultipartBodyLengthLimit;
+}

src/Http/Http/src/Microsoft.AspNetCore.Http.csproj

@@ -12,9 +12,9 @@
   </PropertyGroup>
 
   <ItemGroup>
-	<Compile Include="$(SharedSourceRoot)CancellationTokenSourcePool.cs" />
-    <Compile Include="$(SharedSourceRoot)CopyOnWriteDictionary\*.cs" LinkBase="Shared"/>
-    <Compile Include="$(SharedSourceRoot)ValueTaskExtensions\**\*.cs" LinkBase="Shared"/>
+    <Compile Include="$(SharedSourceRoot)CancellationTokenSourcePool.cs" />
+    <Compile Include="$(SharedSourceRoot)CopyOnWriteDictionary\*.cs" LinkBase="Shared" />
+    <Compile Include="$(SharedSourceRoot)ValueTaskExtensions\**\*.cs" LinkBase="Shared" />
     <Compile Include="..\..\Shared\StreamCopyOperationInternal.cs" Link="Internal\StreamCopyOperationInternal.cs" />
     <Compile Include="..\..\Shared\CookieHeaderParserShared.cs" Link="Internal\CookieHeaderParserShared.cs" />
     <Compile Include="$(SharedSourceRoot)HttpRuleParser.cs" LinkBase="Shared" />

src/Http/Routing/perf/Microbenchmarks/EndpointRoutingShortCircuitBenchmark.cs

@@ -33,7 +33,6 @@ public void Setup()
             new BenchmarkEndpointDataSource(),
             new DiagnosticListener("benchmark"),
             Options.Create(new RouteOptions()),
-            Options.Create(new FormOptions()),
             routingMetrics,
             context => Task.CompletedTask);
 
@@ -46,7 +45,6 @@ public void Setup()
             new BenchmarkEndpointDataSource(),
             new DiagnosticListener("benchmark"),
             Options.Create(new RouteOptions()),
-            Options.Create(new FormOptions()),
             routingMetrics,
             context => Task.CompletedTask);
     }

src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs

@@ -145,7 +145,7 @@ public static TBuilder WithOrder<TBuilder>(this TBuilder builder, int order) whe
             {
                 throw new InvalidOperationException("This endpoint does not support Order.");
             }
-        });      
+        });
         return builder;
     }
 

src/Http/Routing/src/EndpointRoutingMiddleware.cs

@@ -29,7 +29,6 @@ internal sealed partial class EndpointRoutingMiddleware
     private readonly RoutingMetrics _metrics;
     private readonly RequestDelegate _next;
     private readonly RouteOptions _routeOptions;
-    private readonly FormOptions _formOptions;
     private Task<Matcher>? _initializationTask;
 
     public EndpointRoutingMiddleware(
@@ -39,7 +38,6 @@ public EndpointRoutingMiddleware(
         EndpointDataSource rootCompositeEndpointDataSource,
         DiagnosticListener diagnosticListener,
         IOptions<RouteOptions> routeOptions,
-        IOptions<FormOptions> formOptions,
         RoutingMetrics metrics,
         RequestDelegate next)
     {
@@ -51,7 +49,6 @@ 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.
@@ -139,14 +136,6 @@ 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 if the
-            // request being processed is a form request. Note: we do a manual check for the
-            // form content-types here to avoid prematurely accessing the form feature.
-            if (HttpExtensions.IsValidHttpMethodForForm(httpContext.Request.Method) &&
-                HttpExtensions.IsValidContentTypeForForm(httpContext.Request.ContentType))
-            {
-                SetFormOptions(httpContext, endpoint);
-            }
 
             var shortCircuitMetadata = endpoint.Metadata.GetMetadata<ShortCircuitMetadata>();
             if (shortCircuitMetadata is not null)
@@ -345,45 +334,6 @@ private void SetMaxRequestBodySize(HttpContext context)
         }
     }
 
-    private void SetFormOptions(HttpContext context, Endpoint endpoint)
-    {
-        var features = context.Features;
-        var formFeature = features.Get<IFormFeature>();
-
-        // Request form has not been read yet, so set the limits
-        if (formFeature == null || formFeature is { Form: null })
-        {
-            var baseFormOptions = _formOptions;
-            var finalFormOptionsMetadata = new FormOptionsMetadata();
-            var formOptionsMetadatas = endpoint.Metadata
-                .GetOrderedMetadata<IFormOptionsMetadata>();
-            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<IFormFeature>(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)
@@ -427,11 +377,5 @@ 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);
     }
 }