Skip to content

Commit

Permalink
Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
captainsafia committed Sep 8, 2023
1 parent bd0082e commit 6b3209d
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@

namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;

internal sealed class FileConverter<T>(HttpContext? httpContext) : FormDataConverter<T>, ISingleValueConverter
internal sealed class FileConverter<T> : FormDataConverter<T>, ISingleValueConverter
{
[RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
internal override bool TryRead(ref FormDataReader reader, Type type, FormDataMapperOptions options, out T? result, out bool found)
{
if (httpContext == null)
if (reader.FormFileCollection == null)
{
result = default;
found = false;
Expand All @@ -21,12 +21,12 @@ internal override bool TryRead(ref FormDataReader reader, Type type, FormDataMap

if (typeof(T) == typeof(IFormFileCollection))
{
result = (T)httpContext.Request.Form.Files;
result = (T)reader.FormFileCollection;
found = true;
return true;
}

var formFileCollection = httpContext.Request.Form.Files;
var formFileCollection = reader.FormFileCollection;
if (formFileCollection.Count == 0)
{
result = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;

internal sealed class FileConverterFactory(IHttpContextAccessor httpContextAccessor) : IFormDataConverterFactory
internal sealed class FileConverterFactory : IFormDataConverterFactory
{
[RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
Expand All @@ -16,7 +16,7 @@ internal sealed class FileConverterFactory(IHttpContextAccessor httpContextAcces
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
public FormDataConverter CreateConverter(Type type, FormDataMapperOptions options)
{
return Activator.CreateInstance(typeof(FileConverter<>).MakeGenericType(type), httpContextAccessor.HttpContext) as FormDataConverter ??
return Activator.CreateInstance(typeof(FileConverter<>).MakeGenericType(type)) as FormDataConverter ??
throw new InvalidOperationException($"Unable to create converter for '{type.FullName}'.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Collections.Concurrent;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.WebUtilities;

namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;
Expand All @@ -19,28 +18,14 @@ public FormDataMapperOptions()
{
_converters = new(WellKnownConverters.Converters);
_factories.Add(new ParsableConverterFactory());
_factories.Add(new FileConverterFactory());
_factories.Add(new EnumConverterFactory());
_factories.Add(new NullableConverterFactory());
_factories.Add(new DictionaryConverterFactory());
_factories.Add(new CollectionConverterFactory());
_factories.Add(new ComplexTypeConverterFactory(this));
}

[RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
internal FormDataMapperOptions(IHttpContextAccessor httpContextAccessor)
{
// We don't use the base constructor here since the ordering of the factories is important.
_converters = new(WellKnownConverters.Converters);
_factories.Add(new ParsableConverterFactory());
_factories.Add(new EnumConverterFactory());
_factories.Add(new FileConverterFactory(httpContextAccessor));
_factories.Add(new NullableConverterFactory());
_factories.Add(new DictionaryConverterFactory());
_factories.Add(new CollectionConverterFactory());
_factories.Add(new ComplexTypeConverterFactory(this));
}

// Not configurable for now, this is the max number of elements we will bind. This is important for
// security reasons, as we don't want to bind a huge collection and cause perf issues.
// Some examples of this are:
Expand Down
11 changes: 10 additions & 1 deletion src/Components/Endpoints/src/FormMapping/FormDataReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Runtime.CompilerServices;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Primitives;

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

public FormDataReader(IReadOnlyDictionary<FormKey, StringValues> formCollection, CultureInfo culture, Memory<char> buffer, IFormFileCollection formFileCollection)
: this(formCollection, culture, buffer)
{
FormFileCollection = formFileCollection;
}

internal ReadOnlyMemory<char> CurrentPrefix => _currentPrefixBuffer;

public IFormatProvider Culture { get; internal set; }
public IFormatProvider Culture { get; }

public IFormFileCollection? FormFileCollection { get; internal set; }

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.Http;
namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;

internal static class WellKnownConverters
Expand Down Expand Up @@ -37,7 +38,9 @@ static WellKnownConverters()
{ typeof(DateTimeOffset), new ParsableConverter<DateTimeOffset>() },
{ typeof(TimeSpan), new ParsableConverter<TimeSpan>() },
{ typeof(TimeOnly), new ParsableConverter<TimeOnly>() },
{ typeof(Guid), new ParsableConverter<Guid>() }
{ typeof(Guid), new ParsableConverter<Guid>() },
{ typeof(IFormFileCollection), new FileConverter<IFormFileCollection>() },
{ typeof(IFormFile), new FileConverter<IFormFile>() }
};

converters.Add(typeof(char?), new NullableConverter<char>((FormDataConverter<char>)converters[typeof(char)]));
Expand Down
52 changes: 50 additions & 2 deletions src/Components/Endpoints/test/Binding/FormDataMapperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Runtime.Serialization;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Primitives;

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

private FormDataReader CreateFormDataReader(Dictionary<string, StringValues> collection, CultureInfo invariantCulture)
private FormDataReader CreateFormDataReader(Dictionary<string, StringValues> collection, CultureInfo invariantCulture, IFormFileCollection formFileCollection = null)
{
var dictionary = new Dictionary<FormKey, StringValues>(collection.Count);
foreach (var kvp in collection)
{
dictionary.Add(new FormKey(kvp.Key.AsMemory()), kvp.Value);
}
return new FormDataReader(dictionary, CultureInfo.InvariantCulture, new char[2048]);
return formFileCollection is null
? new FormDataReader(dictionary, CultureInfo.InvariantCulture, new char[2048])
: new FormDataReader(dictionary, CultureInfo.InvariantCulture, new char[2048], formFileCollection);
}

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

[Fact]
public void CanDeserialize_ComplexType_CanSerializerFormFile()
{
// Arrange
var expected = new FormFile(Stream.Null, 0, 10, "file", "file.txt");
var formFileCollection = new FormFileCollection { expected };
var data = new Dictionary<string, StringValues>();
var reader = CreateFormDataReader(data, CultureInfo.InvariantCulture, formFileCollection);
var errors = new List<FormDataMappingError>();
reader.ErrorHandler = (key, message, attemptedValue) =>
{
errors.Add(new FormDataMappingError(key, message, attemptedValue));
};
reader.PushPrefix("file");
var options = new FormDataMapperOptions();

// Act
var result = CallDeserialize(reader, options, typeof(IFormFile));

// Assert
Assert.Equal(expected, result);
}

[Fact]
public void CanDeserialize_ComplexType_CanSerializerFormFileCollection()
{
// Arrange
var expected = new FormFileCollection { new FormFile(Stream.Null, 0, 10, "file", "file.txt") };
var data = new Dictionary<string, StringValues>();
var reader = CreateFormDataReader(data, CultureInfo.InvariantCulture, expected);
var errors = new List<FormDataMappingError>();
reader.ErrorHandler = (key, message, attemptedValue) =>
{
errors.Add(new FormDataMappingError(key, message, attemptedValue));
};
reader.PushPrefix("file");
var options = new FormDataMapperOptions();

// Act
var result = CallDeserialize(reader, options, typeof(IFormFileCollection));

// Assert
Assert.Equal(expected, result);
}

[Fact]
public void RecursiveTypes_Comparer_SortsValues_Correctly()
{
Expand Down
11 changes: 5 additions & 6 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public static partial class RequestDelegateFactory
private static readonly MemberExpression FilterContextHttpContextStatusCodeExpr = Expression.Property(FilterContextHttpContextResponseExpr, typeof(HttpResponse).GetProperty(nameof(HttpResponse.StatusCode))!);
private static readonly ParameterExpression InvokedFilterContextExpr = Expression.Parameter(typeof(EndpointFilterInvocationContext), "filterContext");

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

var factoryContext = new RequestDelegateFactoryContext
{
Expand Down Expand Up @@ -2077,13 +2075,14 @@ private static Expression BindComplexParameterFromFormItem(

// ProcessForm(context.Request.Form, form_dict, form_buffer);
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));
// name_reader = new FormDataReader(form_dict, CultureInfo.InvariantCulture, form_buffer.AsMemory(0, formDataMapperOptions.MaxKeyBufferSize), httpContext.Request.Form.Files);
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)),
FormFilesExpr));
// name_reader.MaxRecursionDepth = formDataMapperOptions.MaxRecursionDepth;
var setMaxRecursionDepthExpr = Expression.Assign(
Expression.Property(formReader, nameof(FormDataReader.MaxRecursionDepth)),
Expand Down
3 changes: 3 additions & 0 deletions src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ internal sealed class RequestDelegateFactoryContext

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

// Used to invoke TryResolveFormAsync once per handler so that we can
// avoid the blocking code-path that occurs when `httpContext.Request.Form`
// is called.
public bool ReadForm { get; set; }
public bool ReadFormFile { get; set; }
public ParameterInfo? FirstFormRequestBodyParameter { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,21 +293,32 @@ public async Task SupportsFormFileSourcesInDto()
new FormFile(Stream.Null, 0, 10, "file", "file.txt"),
};
httpContext.Request.Form = new FormCollection(new() { { "Description", "A test file" } }, formFiles);
var serviceCollection = new ServiceCollection();
serviceCollection.TryAddSingleton<IHttpContextAccessor>(new HttpContextAccessor(httpContext));
var options = new RequestDelegateFactoryOptions
{
ServiceProvider = serviceCollection.BuildServiceProvider()
};

var factoryResult = RequestDelegateFactory.Create(TestAction, options);
var factoryResult = RequestDelegateFactory.Create(TestAction);
var requestDelegate = factoryResult.RequestDelegate;

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

[Fact]
public async Task SupportsNullableFormFileSourcesInDto()
{
NullableFormFileDto capturedArgument = default;
void TestAction([FromForm] NullableFormFileDto args) { capturedArgument = args; };
var httpContext = CreateHttpContext();
var formFiles = new FormFileCollection();
httpContext.Request.Form = new FormCollection(new() { { "Description", "A test file" } }, formFiles);

var factoryResult = RequestDelegateFactory.Create(TestAction);
var requestDelegate = factoryResult.RequestDelegate;

await requestDelegate(httpContext);
Assert.Equal("A test file", capturedArgument.Description);
Assert.Null(capturedArgument.File);
}

[Fact]
public async Task SupportsFormFileCollectionSourcesInDto()
{
Expand All @@ -319,14 +330,8 @@ public async Task SupportsFormFileCollectionSourcesInDto()
new FormFile(Stream.Null, 0, 10, "file", "file.txt"),
};
httpContext.Request.Form = new FormCollection(new() { { "Description", "A test file" } }, formFiles);
var serviceCollection = new ServiceCollection();
serviceCollection.TryAddSingleton<IHttpContextAccessor>(new HttpContextAccessor(httpContext));
var options = new RequestDelegateFactoryOptions
{
ServiceProvider = serviceCollection.BuildServiceProvider()
};

var factoryResult = RequestDelegateFactory.Create(TestAction, options);
var factoryResult = RequestDelegateFactory.Create(TestAction);
var requestDelegate = factoryResult.RequestDelegate;

await requestDelegate(httpContext);
Expand All @@ -341,26 +346,23 @@ private class Employee
public string Name { get; set; }
public Employee Manager { get; set; }
}
#nullable enable

private class FormFileDto
{
public string Description { get; set; }
public IFormFile File { get; set; }
public string Description { get; set; } = String.Empty;
public required IFormFile File { get; set; }
}

private class FormFileCollectionDto
private class NullableFormFileDto
{
public string Description { get; set; }
public IFormFileCollection FileCollection { get; set; }
public string? Description { get; set; }
public IFormFile? File { get; set; }
}

private class HttpContextAccessor(HttpContext httpContext) : IHttpContextAccessor
private class FormFileCollectionDto
{

public HttpContext HttpContext
{
get;
set;
} = httpContext;
public string Description { get; set; } = string.Empty;
public required IFormFileCollection FileCollection { get; set; }
}
}

0 comments on commit 6b3209d

Please sign in to comment.