Skip to content

Commit 6b3209d

Browse files
captainsafiacaptainsafia
committed
Feedback
1 parent bd0082e commit 6b3209d

File tree

9 files changed

+107
-58
lines changed

9 files changed

+107
-58
lines changed

‎src/Components/Endpoints/src/FormMapping/Converters/FileConverter.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66

77
namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;
88

9-
internal sealed class FileConverter<T>(HttpContext? httpContext) : FormDataConverter<T>, ISingleValueConverter
9+
internal sealed class FileConverter<T> : FormDataConverter<T>, ISingleValueConverter
1010
{
1111
[RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
1212
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
1313
internal override bool TryRead(ref FormDataReader reader, Type type, FormDataMapperOptions options, out T? result, out bool found)
1414
{
15-
if (httpContext == null)
15+
if (reader.FormFileCollection == null)
1616
{
1717
result = default;
1818
found = false;
@@ -21,12 +21,12 @@ internal override bool TryRead(ref FormDataReader reader, Type type, FormDataMap
2121

2222
if (typeof(T) == typeof(IFormFileCollection))
2323
{
24-
result = (T)httpContext.Request.Form.Files;
24+
result = (T)reader.FormFileCollection;
2525
found = true;
2626
return true;
2727
}
2828

29-
var formFileCollection = httpContext.Request.Form.Files;
29+
var formFileCollection = reader.FormFileCollection;
3030
if (formFileCollection.Count == 0)
3131
{
3232
result = default;

‎src/Components/Endpoints/src/FormMapping/Factories/FileConverterFactory.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;
88

9-
internal sealed class FileConverterFactory(IHttpContextAccessor httpContextAccessor) : IFormDataConverterFactory
9+
internal sealed class FileConverterFactory : IFormDataConverterFactory
1010
{
1111
[RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
1212
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
@@ -16,7 +16,7 @@ internal sealed class FileConverterFactory(IHttpContextAccessor httpContextAcces
1616
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
1717
public FormDataConverter CreateConverter(Type type, FormDataMapperOptions options)
1818
{
19-
return Activator.CreateInstance(typeof(FileConverter<>).MakeGenericType(type), httpContextAccessor.HttpContext) as FormDataConverter ??
19+
return Activator.CreateInstance(typeof(FileConverter<>).MakeGenericType(type)) as FormDataConverter ??
2020
throw new InvalidOperationException($"Unable to create converter for '{type.FullName}'.");
2121
}
2222
}

‎src/Components/Endpoints/src/FormMapping/FormDataMapperOptions.cs‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System.Collections.Concurrent;
55
using System.Diagnostics.CodeAnalysis;
6-
using Microsoft.AspNetCore.Http;
76
using Microsoft.AspNetCore.WebUtilities;
87

98
namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;
@@ -19,28 +18,14 @@ public FormDataMapperOptions()
1918
{
2019
_converters = new(WellKnownConverters.Converters);
2120
_factories.Add(new ParsableConverterFactory());
21+
_factories.Add(new FileConverterFactory());
2222
_factories.Add(new EnumConverterFactory());
2323
_factories.Add(new NullableConverterFactory());
2424
_factories.Add(new DictionaryConverterFactory());
2525
_factories.Add(new CollectionConverterFactory());
2626
_factories.Add(new ComplexTypeConverterFactory(this));
2727
}
2828

29-
[RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
30-
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
31-
internal FormDataMapperOptions(IHttpContextAccessor httpContextAccessor)
32-
{
33-
// We don't use the base constructor here since the ordering of the factories is important.
34-
_converters = new(WellKnownConverters.Converters);
35-
_factories.Add(new ParsableConverterFactory());
36-
_factories.Add(new EnumConverterFactory());
37-
_factories.Add(new FileConverterFactory(httpContextAccessor));
38-
_factories.Add(new NullableConverterFactory());
39-
_factories.Add(new DictionaryConverterFactory());
40-
_factories.Add(new CollectionConverterFactory());
41-
_factories.Add(new ComplexTypeConverterFactory(this));
42-
}
43-
4429
// Not configurable for now, this is the max number of elements we will bind. This is important for
4530
// security reasons, as we don't want to bind a huge collection and cause perf issues.
4631
// Some examples of this are:

‎src/Components/Endpoints/src/FormMapping/FormDataReader.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Diagnostics.CodeAnalysis;
77
using System.Globalization;
88
using System.Runtime.CompilerServices;
9+
using Microsoft.AspNetCore.Http;
910
using Microsoft.Extensions.Primitives;
1011

1112
namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;
@@ -32,9 +33,17 @@ public FormDataReader(IReadOnlyDictionary<FormKey, StringValues> formCollection,
3233
_prefixBuffer = buffer;
3334
}
3435

36+
public FormDataReader(IReadOnlyDictionary<FormKey, StringValues> formCollection, CultureInfo culture, Memory<char> buffer, IFormFileCollection formFileCollection)
37+
: this(formCollection, culture, buffer)
38+
{
39+
FormFileCollection = formFileCollection;
40+
}
41+
3542
internal ReadOnlyMemory<char> CurrentPrefix => _currentPrefixBuffer;
3643

37-
public IFormatProvider Culture { get; internal set; }
44+
public IFormatProvider Culture { get; }
45+
46+
public IFormFileCollection? FormFileCollection { get; internal set; }
3847

3948
public int MaxRecursionDepth { get; set; } = 64;
4049

‎src/Components/Endpoints/src/FormMapping/WellKnownConverters.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using Microsoft.AspNetCore.Http;
45
namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;
56

67
internal static class WellKnownConverters
@@ -37,7 +38,9 @@ static WellKnownConverters()
3738
{ typeof(DateTimeOffset), new ParsableConverter<DateTimeOffset>() },
3839
{ typeof(TimeSpan), new ParsableConverter<TimeSpan>() },
3940
{ typeof(TimeOnly), new ParsableConverter<TimeOnly>() },
40-
{ typeof(Guid), new ParsableConverter<Guid>() }
41+
{ typeof(Guid), new ParsableConverter<Guid>() },
42+
{ typeof(IFormFileCollection), new FileConverter<IFormFileCollection>() },
43+
{ typeof(IFormFile), new FileConverter<IFormFile>() }
4144
};
4245

4346
converters.Add(typeof(char?), new NullableConverter<char>((FormDataConverter<char>)converters[typeof(char)]));

‎src/Components/Endpoints/test/Binding/FormDataMapperTests.cs‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using System.Diagnostics.CodeAnalysis;
1010
using System.Globalization;
1111
using System.Runtime.Serialization;
12+
using Microsoft.AspNetCore.Http;
1213
using Microsoft.Extensions.Primitives;
1314

1415
namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;
@@ -72,14 +73,16 @@ public void CanDeserialize_NullableEnumTypes(string value, Colors expected)
7273
Assert.Equal(expected, result);
7374
}
7475

75-
private FormDataReader CreateFormDataReader(Dictionary<string, StringValues> collection, CultureInfo invariantCulture)
76+
private FormDataReader CreateFormDataReader(Dictionary<string, StringValues> collection, CultureInfo invariantCulture, IFormFileCollection formFileCollection = null)
7677
{
7778
var dictionary = new Dictionary<FormKey, StringValues>(collection.Count);
7879
foreach (var kvp in collection)
7980
{
8081
dictionary.Add(new FormKey(kvp.Key.AsMemory()), kvp.Value);
8182
}
82-
return new FormDataReader(dictionary, CultureInfo.InvariantCulture, new char[2048]);
83+
return formFileCollection is null
84+
? new FormDataReader(dictionary, CultureInfo.InvariantCulture, new char[2048])
85+
: new FormDataReader(dictionary, CultureInfo.InvariantCulture, new char[2048], formFileCollection);
8386
}
8487

8588
[Theory]
@@ -1683,6 +1686,51 @@ public void CanDeserialize_ComplexType_ThrowsFromConstructor()
16831686
Assert.Equal("Value cannot be null. (Parameter 'key')", constructorError.Message.ToString(CultureInfo.InvariantCulture));
16841687
}
16851688

1689+
[Fact]
1690+
public void CanDeserialize_ComplexType_CanSerializerFormFile()
1691+
{
1692+
// Arrange
1693+
var expected = new FormFile(Stream.Null, 0, 10, "file", "file.txt");
1694+
var formFileCollection = new FormFileCollection { expected };
1695+
var data = new Dictionary<string, StringValues>();
1696+
var reader = CreateFormDataReader(data, CultureInfo.InvariantCulture, formFileCollection);
1697+
var errors = new List<FormDataMappingError>();
1698+
reader.ErrorHandler = (key, message, attemptedValue) =>
1699+
{
1700+
errors.Add(new FormDataMappingError(key, message, attemptedValue));
1701+
};
1702+
reader.PushPrefix("file");
1703+
var options = new FormDataMapperOptions();
1704+
1705+
// Act
1706+
var result = CallDeserialize(reader, options, typeof(IFormFile));
1707+
1708+
// Assert
1709+
Assert.Equal(expected, result);
1710+
}
1711+
1712+
[Fact]
1713+
public void CanDeserialize_ComplexType_CanSerializerFormFileCollection()
1714+
{
1715+
// Arrange
1716+
var expected = new FormFileCollection { new FormFile(Stream.Null, 0, 10, "file", "file.txt") };
1717+
var data = new Dictionary<string, StringValues>();
1718+
var reader = CreateFormDataReader(data, CultureInfo.InvariantCulture, expected);
1719+
var errors = new List<FormDataMappingError>();
1720+
reader.ErrorHandler = (key, message, attemptedValue) =>
1721+
{
1722+
errors.Add(new FormDataMappingError(key, message, attemptedValue));
1723+
};
1724+
reader.PushPrefix("file");
1725+
var options = new FormDataMapperOptions();
1726+
1727+
// Act
1728+
var result = CallDeserialize(reader, options, typeof(IFormFileCollection));
1729+
1730+
// Assert
1731+
Assert.Equal(expected, result);
1732+
}
1733+
16861734
[Fact]
16871735
public void RecursiveTypes_Comparer_SortsValues_Correctly()
16881736
{

‎src/Http/Http.Extensions/src/RequestDelegateFactory.cs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public static partial class RequestDelegateFactory
120120
private static readonly MemberExpression FilterContextHttpContextStatusCodeExpr = Expression.Property(FilterContextHttpContextResponseExpr, typeof(HttpResponse).GetProperty(nameof(HttpResponse.StatusCode))!);
121121
private static readonly ParameterExpression InvokedFilterContextExpr = Expression.Parameter(typeof(EndpointFilterInvocationContext), "filterContext");
122122

123-
private static readonly ConstructorInfo FormDataReaderConstructor = typeof(FormDataReader).GetConstructor(new[] { typeof(IReadOnlyDictionary<FormKey, StringValues>), typeof(CultureInfo), typeof(Memory<char>) })!;
123+
private static readonly ConstructorInfo FormDataReaderConstructor = typeof(FormDataReader).GetConstructor(new[] { typeof(IReadOnlyDictionary<FormKey, StringValues>), typeof(CultureInfo), typeof(Memory<char>), typeof(IFormFileCollection) })!;
124124
private static readonly MethodInfo ProcessFormMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ProcessForm), BindingFlags.Static | BindingFlags.NonPublic)!;
125125
private static readonly MethodInfo FormDataMapperMapMethod = typeof(FormDataMapper).GetMethod(nameof(FormDataMapper.Map))!;
126126
private static readonly MethodInfo AsMemoryMethod = new Func<char[]?, int, int, Memory<char>>(MemoryExtensions.AsMemory).Method;
@@ -276,9 +276,7 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat
276276
var serviceProvider = options?.ServiceProvider ?? options?.EndpointBuilder?.ApplicationServices ?? EmptyServiceProvider.Instance;
277277
var endpointBuilder = options?.EndpointBuilder ?? new RdfEndpointBuilder(serviceProvider);
278278
var jsonSerializerOptions = serviceProvider.GetService<IOptions<JsonOptions>>()?.Value.SerializerOptions ?? JsonOptions.DefaultSerializerOptions;
279-
var formDataMapperOptions = serviceProvider.GetService<IHttpContextAccessor>() is {} httpContextAccessor
280-
? new FormDataMapperOptions(httpContextAccessor)
281-
: new FormDataMapperOptions();
279+
var formDataMapperOptions = new FormDataMapperOptions();;
282280

283281
var factoryContext = new RequestDelegateFactoryContext
284282
{
@@ -2077,13 +2075,14 @@ private static Expression BindComplexParameterFromFormItem(
20772075

20782076
// ProcessForm(context.Request.Form, form_dict, form_buffer);
20792077
var processFormExpr = Expression.Call(ProcessFormMethod, FormExpr, Expression.Constant(formDataMapperOptions.MaxKeyBufferSize), formDict, formBuffer);
2080-
// name_reader = new FormDataReader(form_dict, CultureInfo.InvariantCulture, form_buffer.AsMemory(0, formDataMapperOptions.MaxKeyBufferSize));
2078+
// name_reader = new FormDataReader(form_dict, CultureInfo.InvariantCulture, form_buffer.AsMemory(0, formDataMapperOptions.MaxKeyBufferSize), httpContext.Request.Form.Files);
20812079
var initializeReaderExpr = Expression.Assign(
20822080
formReader,
20832081
Expression.New(FormDataReaderConstructor,
20842082
formDict,
20852083
Expression.Constant(CultureInfo.InvariantCulture),
2086-
Expression.Call(AsMemoryMethod, formBuffer, Expression.Constant(0), Expression.Constant(formDataMapperOptions.MaxKeyBufferSize))));
2084+
Expression.Call(AsMemoryMethod, formBuffer, Expression.Constant(0), Expression.Constant(formDataMapperOptions.MaxKeyBufferSize)),
2085+
FormFilesExpr));
20872086
// name_reader.MaxRecursionDepth = formDataMapperOptions.MaxRecursionDepth;
20882087
var setMaxRecursionDepthExpr = Expression.Assign(
20892088
Expression.Property(formReader, nameof(FormDataReader.MaxRecursionDepth)),

‎src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ internal sealed class RequestDelegateFactoryContext
4646

4747
public NullabilityInfoContext NullabilityContext { get; } = new();
4848

49+
// Used to invoke TryResolveFormAsync once per handler so that we can
50+
// avoid the blocking code-path that occurs when `httpContext.Request.Form`
51+
// is called.
4952
public bool ReadForm { get; set; }
5053
public bool ReadFormFile { get; set; }
5154
public ParameterInfo? FirstFormRequestBodyParameter { get; set; }

‎src/Http/Http.Extensions/test/RequestDelegateFactoryTests.FormMapping.cs‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -293,21 +293,32 @@ public async Task SupportsFormFileSourcesInDto()
293293
new FormFile(Stream.Null, 0, 10, "file", "file.txt"),
294294
};
295295
httpContext.Request.Form = new FormCollection(new() { { "Description", "A test file" } }, formFiles);
296-
var serviceCollection = new ServiceCollection();
297-
serviceCollection.TryAddSingleton<IHttpContextAccessor>(new HttpContextAccessor(httpContext));
298-
var options = new RequestDelegateFactoryOptions
299-
{
300-
ServiceProvider = serviceCollection.BuildServiceProvider()
301-
};
302296

303-
var factoryResult = RequestDelegateFactory.Create(TestAction, options);
297+
var factoryResult = RequestDelegateFactory.Create(TestAction);
304298
var requestDelegate = factoryResult.RequestDelegate;
305299

306300
await requestDelegate(httpContext);
307301
Assert.Equal("A test file", capturedArgument.Description);
308302
Assert.Equal(formFiles["file"], capturedArgument.File);
309303
}
310304

305+
[Fact]
306+
public async Task SupportsNullableFormFileSourcesInDto()
307+
{
308+
NullableFormFileDto capturedArgument = default;
309+
void TestAction([FromForm] NullableFormFileDto args) { capturedArgument = args; };
310+
var httpContext = CreateHttpContext();
311+
var formFiles = new FormFileCollection();
312+
httpContext.Request.Form = new FormCollection(new() { { "Description", "A test file" } }, formFiles);
313+
314+
var factoryResult = RequestDelegateFactory.Create(TestAction);
315+
var requestDelegate = factoryResult.RequestDelegate;
316+
317+
await requestDelegate(httpContext);
318+
Assert.Equal("A test file", capturedArgument.Description);
319+
Assert.Null(capturedArgument.File);
320+
}
321+
311322
[Fact]
312323
public async Task SupportsFormFileCollectionSourcesInDto()
313324
{
@@ -319,14 +330,8 @@ public async Task SupportsFormFileCollectionSourcesInDto()
319330
new FormFile(Stream.Null, 0, 10, "file", "file.txt"),
320331
};
321332
httpContext.Request.Form = new FormCollection(new() { { "Description", "A test file" } }, formFiles);
322-
var serviceCollection = new ServiceCollection();
323-
serviceCollection.TryAddSingleton<IHttpContextAccessor>(new HttpContextAccessor(httpContext));
324-
var options = new RequestDelegateFactoryOptions
325-
{
326-
ServiceProvider = serviceCollection.BuildServiceProvider()
327-
};
328333

329-
var factoryResult = RequestDelegateFactory.Create(TestAction, options);
334+
var factoryResult = RequestDelegateFactory.Create(TestAction);
330335
var requestDelegate = factoryResult.RequestDelegate;
331336

332337
await requestDelegate(httpContext);
@@ -341,26 +346,23 @@ private class Employee
341346
public string Name { get; set; }
342347
public Employee Manager { get; set; }
343348
}
349+
#nullable enable
344350

345351
private class FormFileDto
346352
{
347-
public string Description { get; set; }
348-
public IFormFile File { get; set; }
353+
public string Description { get; set; } = String.Empty;
354+
public required IFormFile File { get; set; }
349355
}
350356

351-
private class FormFileCollectionDto
357+
private class NullableFormFileDto
352358
{
353-
public string Description { get; set; }
354-
public IFormFileCollection FileCollection { get; set; }
359+
public string? Description { get; set; }
360+
public IFormFile? File { get; set; }
355361
}
356362

357-
private class HttpContextAccessor(HttpContext httpContext) : IHttpContextAccessor
363+
private class FormFileCollectionDto
358364
{
359-
360-
public HttpContext HttpContext
361-
{
362-
get;
363-
set;
364-
} = httpContext;
365+
public string Description { get; set; } = string.Empty;
366+
public required IFormFileCollection FileCollection { get; set; }
365367
}
366368
}

0 commit comments

Comments
 (0)