From a2574fd9de65862b8a0cabb8aa62d566ddb3fbab Mon Sep 17 00:00:00 2001 From: David Fowler Date: Tue, 8 Jan 2019 18:38:42 -0800 Subject: [PATCH 1/2] Don't allocate the FormFeature eagerly - Expose FormOptions on DefaultHttpContext - Use those options on DefaultHttpContext when the FormFeature is initialized --- src/Http/Http/src/DefaultHttpContext.cs | 2 ++ src/Http/Http/src/Features/FormFeature.cs | 4 +--- src/Http/Http/src/Features/FormOptions.cs | 4 +++- src/Http/Http/src/HttpContextFactory.cs | 5 ++--- src/Http/Http/src/IHttpContextContainer.cs | 2 +- .../Http/src/Internal/DefaultHttpRequest.cs | 2 +- .../Http/test/Features/FormFeatureTests.cs | 22 +++++++++++++++++-- .../Core/src/Internal/Http/HttpProtocol.cs | 2 +- 8 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/Http/Http/src/DefaultHttpContext.cs b/src/Http/Http/src/DefaultHttpContext.cs index 7a03c7a3acd6..6ecaa809f1ec 100644 --- a/src/Http/Http/src/DefaultHttpContext.cs +++ b/src/Http/Http/src/DefaultHttpContext.cs @@ -62,6 +62,8 @@ public void Uninitialize() _websockets?.Uninitialize(); } + public FormOptions FormOptions { get; set; } + private IItemsFeature ItemsFeature => _features.Fetch(ref _features.Cache.Items, _newItemsFeature); diff --git a/src/Http/Http/src/Features/FormFeature.cs b/src/Http/Http/src/Features/FormFeature.cs index d93234d08b36..796e996187dc 100644 --- a/src/Http/Http/src/Features/FormFeature.cs +++ b/src/Http/Http/src/Features/FormFeature.cs @@ -15,8 +15,6 @@ namespace Microsoft.AspNetCore.Http.Features { public class FormFeature : IFormFeature { - private static readonly FormOptions DefaultFormOptions = new FormOptions(); - private readonly HttpRequest _request; private readonly FormOptions _options; private Task _parsedFormTask; @@ -32,7 +30,7 @@ public FormFeature(IFormCollection form) Form = form; } public FormFeature(HttpRequest request) - : this(request, DefaultFormOptions) + : this(request, FormOptions.Default) { } diff --git a/src/Http/Http/src/Features/FormOptions.cs b/src/Http/Http/src/Features/FormOptions.cs index 17e521b21574..bd31f5cd8069 100644 --- a/src/Http/Http/src/Features/FormOptions.cs +++ b/src/Http/Http/src/Features/FormOptions.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.IO; @@ -8,6 +8,8 @@ namespace Microsoft.AspNetCore.Http.Features { public class FormOptions { + public static readonly FormOptions Default = new FormOptions(); + public const int DefaultMemoryBufferThreshold = 1024 * 64; public const int DefaultBufferBodyLengthLimit = 1024 * 1024 * 128; public const int DefaultMultipartBoundaryLengthLimit = 128; diff --git a/src/Http/Http/src/HttpContextFactory.cs b/src/Http/Http/src/HttpContextFactory.cs index 68f55b797ec7..cc2f796d6bf1 100644 --- a/src/Http/Http/src/HttpContextFactory.cs +++ b/src/Http/Http/src/HttpContextFactory.cs @@ -42,13 +42,12 @@ public HttpContext Create(IFeatureCollection featureCollection) _httpContextAccessor.HttpContext = httpContext; } - var formFeature = new FormFeature(httpContext.Request, _formOptions); - featureCollection.Set(formFeature); + httpContext.FormOptions = _formOptions; return httpContext; } - private static HttpContext CreateHttpContext(IFeatureCollection featureCollection) + private static DefaultHttpContext CreateHttpContext(IFeatureCollection featureCollection) { if (featureCollection is IHttpContextContainer container) { diff --git a/src/Http/Http/src/IHttpContextContainer.cs b/src/Http/Http/src/IHttpContextContainer.cs index f77b36ec1de9..ff63e34f459e 100644 --- a/src/Http/Http/src/IHttpContextContainer.cs +++ b/src/Http/Http/src/IHttpContextContainer.cs @@ -6,6 +6,6 @@ namespace Microsoft.AspNetCore.Http { public interface IHttpContextContainer { - HttpContext HttpContext { get; } + DefaultHttpContext HttpContext { get; } } } diff --git a/src/Http/Http/src/Internal/DefaultHttpRequest.cs b/src/Http/Http/src/Internal/DefaultHttpRequest.cs index cd052d875e46..55112b687e5d 100644 --- a/src/Http/Http/src/Internal/DefaultHttpRequest.cs +++ b/src/Http/Http/src/Internal/DefaultHttpRequest.cs @@ -17,7 +17,7 @@ public sealed class DefaultHttpRequest : HttpRequest // Lambdas hoisted to static readonly fields to improve inlining https://github.com/dotnet/roslyn/issues/13624 private readonly static Func _nullRequestFeature = f => null; private readonly static Func _newQueryFeature = f => new QueryFeature(f); - private readonly static Func _newFormFeature = r => new FormFeature(r); + private readonly static Func _newFormFeature = r => new FormFeature(r, r._context.FormOptions ?? FormOptions.Default); private readonly static Func _newRequestCookiesFeature = f => new RequestCookiesFeature(f); private readonly static Func _newRouteValuesFeature = f => new RouteValuesFeature(); private readonly static Func _newRequestBodyPipeFeature = context => new RequestBodyPipeFeature(context); diff --git a/src/Http/Http/test/Features/FormFeatureTests.cs b/src/Http/Http/test/Features/FormFeatureTests.cs index cfa8b0215b19..8a63421c7511 100644 --- a/src/Http/Http/test/Features/FormFeatureTests.cs +++ b/src/Http/Http/test/Features/FormFeatureTests.cs @@ -29,6 +29,24 @@ public async Task ReadFormAsync_0ContentLength_ReturnsEmptyForm() Assert.Same(FormCollection.Empty, formCollection); } + [Fact] + public async Task FormFeatureReadsOptionsFromDefaultHttpContext() + { + var context = new DefaultHttpContext(); + context.Request.ContentType = "application/x-www-form-urlencoded; charset=utf-8"; + context.FormOptions = new FormOptions + { + ValueCountLimit = 1 + }; + + var formContent = Encoding.UTF8.GetBytes("foo=bar&baz=2"); + context.Request.Body = new NonSeekableReadStream(formContent); + + var exception = await Assert.ThrowsAsync(() => context.Request.ReadFormAsync()); + + Assert.Equal("Form value count limit 1 exceeded.", exception.Message); + } + [Theory] [InlineData(true)] [InlineData(false)] @@ -391,7 +409,7 @@ public async Task ReadFormAsync_ValueCountLimitExceeded_Throw(bool bufferRequest IFormFeature formFeature = new FormFeature(context.Request, new FormOptions() { BufferBody = bufferRequest, ValueCountLimit = 2 }); context.Features.Set(formFeature); - var exception = await Assert.ThrowsAsync (() => context.Request.ReadFormAsync()); + var exception = await Assert.ThrowsAsync(() => context.Request.ReadFormAsync()); Assert.Equal("Form value count limit 2 exceeded.", exception.Message); } @@ -416,7 +434,7 @@ public async Task ReadFormAsync_ValueCountLimitExceededWithFiles_Throw(bool buff IFormFeature formFeature = new FormFeature(context.Request, new FormOptions() { BufferBody = bufferRequest, ValueCountLimit = 2 }); context.Features.Set(formFeature); - var exception = await Assert.ThrowsAsync (() => context.Request.ReadFormAsync()); + var exception = await Assert.ThrowsAsync(() => context.Request.ReadFormAsync()); Assert.Equal("Form value count limit 2 exceeded.", exception.Message); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index a3a8b72eaa6a..9d8dad0304c2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -277,7 +277,7 @@ public CancellationToken RequestAborted protected HttpResponseHeaders HttpResponseHeaders { get; } = new HttpResponseHeaders(); - HttpContext IHttpContextContainer.HttpContext + DefaultHttpContext IHttpContextContainer.HttpContext { get { From 7df5d60aadc132274ad5a2ee979efc79ad6251ee Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 9 Jan 2019 12:28:42 -0800 Subject: [PATCH 2/2] Made the default FormOptions internal since they are mutable --- src/Http/Http/src/Features/FormOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http/src/Features/FormOptions.cs b/src/Http/Http/src/Features/FormOptions.cs index bd31f5cd8069..414341a8a1a4 100644 --- a/src/Http/Http/src/Features/FormOptions.cs +++ b/src/Http/Http/src/Features/FormOptions.cs @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Http.Features { public class FormOptions { - public static readonly FormOptions Default = new FormOptions(); + internal static readonly FormOptions Default = new FormOptions(); public const int DefaultMemoryBufferThreshold = 1024 * 64; public const int DefaultBufferBodyLengthLimit = 1024 * 1024 * 128;