Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support resolving form files in complex form mapping #50537

Merged
merged 10 commits into from
Sep 15, 2023
Merged

Support resolving form files in complex form mapping #50537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
76a6019
Support resolving IFormFile in complex form mapping
captainsafia Aug 31, 2023
e5232bf
Feedback
captainsafia Sep 8, 2023
635e67d
Fix up FormFile integration in Blazor
captainsafia Sep 13, 2023
3a2c2a8
Fix up FileConverter interfaces
captainsafia Sep 14, 2023
b8e33a5
Fix build
captainsafia Sep 14, 2023
a34d9fe
Fix FormFileCollection initialization
captainsafia Sep 14, 2023
78bf7d8
Revert "Revert "[Blazor] Update selenium versions (#50511)" (#50556)"
captainsafia Sep 14, 2023
41e54ac
Update test for non-enhanced form
captainsafia Sep 14, 2023
f219fd4
Revert "Revert "Revert "[Blazor] Update selenium versions (#50511)" (…
captainsafia Sep 14, 2023
6688063
Add support for IReadOnlyList<IBrowserFile>
captainsafia Sep 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/Components/Endpoints/src/FormMapping/BrowserFileFromFormFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// 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.Forms;
using Microsoft.AspNetCore.Http;

namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;

internal sealed class BrowserFileFromFormFile(IFormFile formFile) : IBrowserFile
{
public string Name => formFile.Name;

public DateTimeOffset LastModified => DateTimeOffset.Parse(formFile.Headers.LastModified.ToString(), CultureInfo.InvariantCulture);

public long Size => formFile.Length;

public string ContentType => formFile.ContentType;

public Stream OpenReadStream(long maxAllowedSize = 512000, CancellationToken cancellationToken = default)
{
if (Size > maxAllowedSize)
{
throw new IOException($"Supplied file with size {Size} bytes exceeds the maximum of {maxAllowedSize} bytes.");
}

return formFile.OpenReadStream();
}
}
86 changes: 86 additions & 0 deletions src/Components/Endpoints/src/FormMapping/Converters/FileConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// 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.CodeAnalysis;
#if COMPONENTS
using Microsoft.AspNetCore.Components.Forms;
#endif
using Microsoft.AspNetCore.Http;

namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;

internal sealed class FileConverter<T> : FormDataConverter<T>
{
[RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
internal override bool TryRead(ref FormDataReader reader, Type type, FormDataMapperOptions options, out T? result, out bool found)
{
if (reader.FormFileCollection == null)
{
result = default;
found = false;
return true;
}

#if COMPONENTS
if (typeof(T) == typeof(IBrowserFile))
{
var targetFile = reader.FormFileCollection.GetFile(reader.CurrentPrefix.ToString());
if (targetFile != null)
{
var browserFile = new BrowserFileFromFormFile(targetFile);
result = (T)(IBrowserFile)browserFile;
found = true;
return true;
}
}

if (typeof(T) == typeof(IReadOnlyList<IBrowserFile>))
{
var targetFiles = reader.FormFileCollection.GetFiles(reader.CurrentPrefix.ToString());
var buffer = ReadOnlyCollectionBufferAdapter<IBrowserFile>.CreateBuffer();
for (var i = 0; i < targetFiles.Count; i++)
{
buffer = ReadOnlyCollectionBufferAdapter<IBrowserFile>.Add(ref buffer, new BrowserFileFromFormFile(targetFiles[i]));
}
result = (T)(IReadOnlyList<IBrowserFile>)ReadOnlyCollectionBufferAdapter<IBrowserFile>.ToResult(buffer);
found = true;
return true;
}
#endif

if (typeof(T) == typeof(IReadOnlyList<IFormFile>))
{
result = (T)reader.FormFileCollection.GetFiles(reader.CurrentPrefix.ToString());
found = true;
return true;
}

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

var formFileCollection = reader.FormFileCollection;
if (formFileCollection.Count == 0)
{
result = default;
found = false;
return true;
}

var file = formFileCollection.GetFile(reader.CurrentPrefix.ToString());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took another look, and this might be a bit more nuanced than we think.

For reference, MVC seems to only support binding files as a Top-level property/object, (which is fine, we can decide to do something different here), but I believe we need to be careful and make sure that the following scenarios work:

  • Multiple file inputs binding to a single file 
    <input type="file" name="Resume" />
<input type="file" name="Degree" />
    with a model like 
    public class MyModel
{
    public IFormFile Resume { get; set; }
    public IFormFile Degree { get; set; }
}
  • Multiple file inputs binding to collections of files 
    <input type="file" multiple name="Reports" />
<input type="file" multiple name="OtherDocuments" />
    with a model like 
    public class MyModel
{
    public IFormFileCollection Reports { get; set; }
    public IFormFile OtherDocuments { get; set; }
}
  • We should also probably produce an error if there are many files with the same name and we are trying to bind to an IFormFile

This is a sample payload that chrome will send for input and input multiple:

------WebKitFormBoundaryTxMDCYZlym9qHpwC
Content-Disposition: form-data; name="SampleFile"; filename="MySampleFile.txt"
Content-Type: text/plain


------WebKitFormBoundaryTxMDCYZlym9qHpwC
Content-Disposition: form-data; name="OtherFile"; filename="MyOtherFile.txt"
Content-Type: text/plain


------WebKitFormBoundaryTxMDCYZlym9qHpwC
Content-Disposition: form-data; name="Reports"; filename="MyReports2.txt"
Content-Type: text/plain


------WebKitFormBoundaryTxMDCYZlym9qHpwC
Content-Disposition: form-data; name="Reports"; filename="MyReports.txt"
Content-Type: text/plain


------WebKitFormBoundaryTxMDCYZlym9qHpwC
Content-Disposition: form-data; name="OtherDocuments"; filename="MyOtherDocuments2.txt"
Content-Type: text/plain


------WebKitFormBoundaryTxMDCYZlym9qHpwC
Content-Disposition: form-data; name="OtherDocuments"; filename="MyOtherDocuments.txt"
Content-Type: text/plain


------WebKitFormBoundaryTxMDCYZlym9qHpwC
Content-Disposition: form-data; name="__RequestVerificationToken"

CfDJ8NQYaZebqUlGr0rlYIlnIDg5dKVRjgYAN_qNiT2n1Fw0M3XRiweKM4YcI2rzoBE9BBzidDyoPzG7YumuVUkV5qNUz0GIEpBGlNb2muhD-OoFPhKZoHLXKzQV0StDU0_iVSDXivsQu2RVtcOBPOdA-lg
------WebKitFormBoundaryTxMDCYZlym9qHpwC--

The contents of the files are not shown because the chrome dev tools scraps them. The form that I used is like this:

<form method="post" enctype="multipart/form-data">
    <input type="file" name="SampleFile" />
    <input type="file" name="OtherFile" />
    <input type="file" multiple name="Reports" />
    <input type="file" multiple name="OtherDocuments" />
    <input type="submit" value="Upload" />
</form>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

public IFormFileCollection Reports { get; set; }

This one is interesting. At the moment, minimal APIs assumes that a top-level IFormFileCollection type refers to HttpContext.Request.Form.Files in all occasions. We don't use the parameter name to extract a subset of files by their name. IMO, it probably makes more sense to support that by supporting an IReadOnlyList<IFormFile> type similar to what IFormFIleCollection.GetFiles returns.

Binding for multiple IFormFiles with different names and erroring for multiple files with the same name makes sense though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@captainsafia I'm ok if we keep IFormFileCollection binding the entire set of files independent of the prefix (the type acts as the signal/discriminator) of what you want, and it's a well-defined behavior.

What I am interested in making sure that we do correctly is handling input type=file and input type=file multiple in a way that people can either use the inputs directly or rely on InputFile to author the UI.

We need to get this right because whatever we bake in now, will become a breaking change in the future otherwise. So I think your proposal of having IFormFileCollection bind to the entire thing and have <<(Collection|Dictionary)Type>><IFormFile> bind to the set that match the proposed subset makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've updated the PR to accommodate some of the functionality here. InputFile works with:

  • IBrowserFile
  • IFormFile
  • IReadOnlyList
  • IFormFileCollection

as types backing the property.

Binding for multiple IFormFiles with different names and erroring for multiple files with the same name makes sense though.

I take this back. It appears the current behavior for IFormFileCollection is to return the first file with a given name if they are multiple so I kept this pattern in the implementation.

if (file != null)
{
result = (T)file;
found = true;
return true;
}

result = default;
found = false;
return true;
}
}
31 changes: 31 additions & 0 deletions src/Components/Endpoints/src/FormMapping/Factories/FileConverterFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// 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.CodeAnalysis;
#if COMPONENTS
using Microsoft.AspNetCore.Components.Forms;
#endif
using Microsoft.AspNetCore.Http;

namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;

internal sealed class FileConverterFactory : IFormDataConverterFactory
{
[RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
#if COMPONENTS
public bool CanConvert(Type type, FormDataMapperOptions options) => CanConvertCommon(type) || type == typeof(IBrowserFile) || type == typeof(IReadOnlyList<IBrowserFile>);
#else
public bool CanConvert(Type type, FormDataMapperOptions options) => CanConvertCommon(type);
#endif

private static bool CanConvertCommon(Type type) => type == typeof(IFormFile) || type == typeof(IFormFileCollection) || type == typeof(IReadOnlyList<IFormFile>);

[RequiresDynamicCode(FormMappingHelpers.RequiresDynamicCodeMessage)]
[RequiresUnreferencedCode(FormMappingHelpers.RequiresUnreferencedCodeMessage)]
public FormDataConverter CreateConverter(Type type, FormDataMapperOptions options)
{
return Activator.CreateInstance(typeof(FileConverter<>).MakeGenericType(type)) as FormDataConverter ??
throw new InvalidOperationException($"Unable to create converter for '{type.FullName}'.");
}
}
1 change: 1 addition & 0 deletions src/Components/Endpoints/src/FormMapping/FormDataMapperOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public FormDataMapperOptions(ILoggerFactory loggerFactory)
{
_converters = new(WellKnownConverters.Converters);
_factories.Add(new ParsableConverterFactory());
_factories.Add(new FileConverterFactory());
_factories.Add(new EnumConverterFactory());
_factories.Add(new NullableConverterFactory());
_factories.Add(new DictionaryConverterFactory());
Expand Down
11 changes: 10 additions & 1 deletion src/Components/Endpoints/src/FormMapping/FormDataReader.cs
View file
Open in desktop
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 Down Expand Up @@ -33,9 +34,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
7 changes: 6 additions & 1 deletion src/Components/Endpoints/src/FormMapping/HttpContextFormDataProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Primitives;

namespace Microsoft.AspNetCore.Components.Endpoints;
Expand All @@ -11,15 +12,19 @@ internal sealed class HttpContextFormDataProvider
{
private string? _incomingHandlerName;
private IReadOnlyDictionary<string, StringValues>? _entries;
private IFormFileCollection? _formFiles;

public string? IncomingHandlerName => _incomingHandlerName;

public IReadOnlyDictionary<string, StringValues> Entries => _entries ?? ReadOnlyDictionary<string, StringValues>.Empty;

public void SetFormData(string incomingHandlerName, IReadOnlyDictionary<string, StringValues> form)
public IFormFileCollection FormFiles => _formFiles ?? (IFormFileCollection)FormCollection.Empty;

public void SetFormData(string incomingHandlerName, IReadOnlyDictionary<string, StringValues> form, IFormFileCollection formFiles)
{
_incomingHandlerName = incomingHandlerName;
_entries = form;
_formFiles = formFiles;
}

public bool TryGetIncomingHandlerName([NotNullWhen(true)] out string? incomingHandlerName)
Expand Down
12 changes: 8 additions & 4 deletions src/Components/Endpoints/src/FormMapping/HttpContextFormValueMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Globalization;
using Microsoft.AspNetCore.Components.Endpoints.FormMapping;
using Microsoft.AspNetCore.Components.Forms.Mapping;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;

Expand Down Expand Up @@ -85,7 +86,7 @@ public void Map(FormValueMappingContext context)

var deserializer = _cache.GetOrAdd(context.ValueType, CreateDeserializer);
Debug.Assert(deserializer != null);
deserializer.Deserialize(context, _options, _formData.Entries);
deserializer.Deserialize(context, _options, _formData.Entries, _formData.FormFiles);
}

private FormValueSupplier CreateDeserializer(Type type) =>
Expand All @@ -99,7 +100,8 @@ internal abstract class FormValueSupplier
public abstract void Deserialize(
FormValueMappingContext context,
FormDataMapperOptions options,
IReadOnlyDictionary<string, StringValues> form);
IReadOnlyDictionary<string, StringValues> form,
IFormFileCollection formFiles);
}

internal class FormValueSupplier<T> : FormValueSupplier
Expand All @@ -109,7 +111,8 @@ internal class FormValueSupplier<T> : FormValueSupplier
public override void Deserialize(
FormValueMappingContext context,
FormDataMapperOptions options,
IReadOnlyDictionary<string, StringValues> form)
IReadOnlyDictionary<string, StringValues> form,
IFormFileCollection formFiles)
{
if (form.Count == 0)
{
Expand All @@ -129,7 +132,8 @@ public override void Deserialize(
using var reader = new FormDataReader(
dictionary,
CultureInfo.InvariantCulture,
buffer.AsMemory(0, options.MaxKeyBufferSize))
buffer.AsMemory(0, options.MaxKeyBufferSize),
formFiles)
{
ErrorHandler = context.OnError,
AttachInstanceToErrorsHandler = context.MapErrorToContainer,
Expand Down
7 changes: 7 additions & 0 deletions src/Components/Endpoints/src/FormMapping/Metadata/FormDataMetadataFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ internal partial class FormDataMetadataFactory(List<IFormDataConverterFactory> f
private readonly FormMetadataContext _context = new();
private readonly ParsableConverterFactory _parsableFactory = factories.OfType<ParsableConverterFactory>().Single();
private readonly DictionaryConverterFactory _dictionaryFactory = factories.OfType<DictionaryConverterFactory>().Single();
private readonly FileConverterFactory _fileConverterFactory = factories.OfType<FileConverterFactory>().Single();
private readonly CollectionConverterFactory _collectionFactory = factories.OfType<CollectionConverterFactory>().Single();
private readonly ILogger<FormDataMetadataFactory> _logger = loggerFactory.CreateLogger<FormDataMetadataFactory>();

Expand Down Expand Up @@ -86,6 +87,12 @@ internal partial class FormDataMetadataFactory(List<IFormDataConverterFactory> f
return result;
}

if (_fileConverterFactory.CanConvert(type, options))
{
result.Kind = FormDataTypeKind.File;
return result;
}

if (_dictionaryFactory.CanConvert(type, options))
{
Log.DictionaryType(_logger, type);
Expand Down
1 change: 1 addition & 0 deletions src/Components/Endpoints/src/FormMapping/Metadata/FormDataTypeKind.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping.Metadata;
internal enum FormDataTypeKind
{
Primitive,
File,
Collection,
Dictionary,
Object,
Expand Down
14 changes: 13 additions & 1 deletion src/Components/Endpoints/src/FormMapping/WellKnownConverters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#if COMPONENTS
using Microsoft.AspNetCore.Components.Forms;
#endif
using Microsoft.AspNetCore.Http;

namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;

internal static class WellKnownConverters
Expand Down Expand Up @@ -37,7 +42,14 @@ 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>() },
{ typeof(IReadOnlyList<IFormFile>), new FileConverter<IReadOnlyList<IFormFile>>() },
#if COMPONENTS
{ typeof(IBrowserFile), new FileConverter<IBrowserFile>() },
{ typeof(IReadOnlyList<IBrowserFile>), new FileConverter<IReadOnlyList<IBrowserFile>>() }
#endif
};

converters.Add(typeof(char?), new NullableConverter<char>((FormDataConverter<char>)converters[typeof(char)]));
Expand Down
1 change: 1 addition & 0 deletions src/Components/Endpoints/src/Microsoft.AspNetCore.Components.Endpoints.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<IsPackable>false</IsPackable>
<EmbeddedFilesManifestFileName>Microsoft.Extensions.FileProviders.Embedded.Manifest.xml</EmbeddedFilesManifestFileName>
<Nullable>enable</Nullable>
<DefineConstants>$(DefineConstants);COMPONENTS</DefineConstants>
</PropertyGroup>

<!-- This workaround is required when referencing FileProviders.Embedded as a project -->
Expand Down
2 changes: 1 addition & 1 deletion src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ internal static async Task InitializeStandardComponentServicesAsync(
if (handler != null && form != null)
{
httpContext.RequestServices.GetRequiredService<HttpContextFormDataProvider>()
.SetFormData(handler, new FormCollectionReadOnlyDictionary(form));
.SetFormData(handler, new FormCollectionReadOnlyDictionary(form), form.Files);
}

if (httpContext.RequestServices.GetService<AntiforgeryStateProvider>() is EndpointAntiforgeryStateProvider antiforgery)
Expand Down
Loading