-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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.
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
+559
−16
Merged
Changes from 9 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 e5232bf
Feedback
captainsafia 635e67d
Fix up FormFile integration in Blazor
captainsafia 3a2c2a8
Fix up FileConverter interfaces
captainsafia b8e33a5
Fix build
captainsafia a34d9fe
Fix FormFileCollection initialization
captainsafia 78bf7d8
Revert "Revert "[Blazor] Update selenium versions (#50511)" (#50556)"
captainsafia 41e54ac
Update test for non-enhanced form
captainsafia f219fd4
Revert "Revert "Revert "[Blazor] Update selenium versions (#50511)" (…
captainsafia 6688063
Add support for IReadOnlyList<IBrowserFile>
captainsafia File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
29 changes: 29 additions & 0 deletions
29
src/Components/Endpoints/src/FormMapping/BrowserFileFromFormFile.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||
} | ||
} |
73 changes: 73 additions & 0 deletions
73
src/Components/Endpoints/src/FormMapping/Converters/FileConverter.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
// 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; | ||
} | ||
} | ||
#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()); | ||
if (file != null) | ||
{ | ||
result = (T)file; | ||
found = true; | ||
return true; | ||
} | ||
|
||
result = default; | ||
found = false; | ||
return true; | ||
} | ||
} |
31 changes: 31 additions & 0 deletions
31
src/Components/Endpoints/src/FormMapping/Factories/FileConverterFactory.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
#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}'."); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
|
@@ -37,7 +42,13 @@ 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>() } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Can we also do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure |
||
#endif | ||
}; | ||
|
||
converters.Add(typeof(char?), new NullableConverter<char>((FormDataConverter<char>)converters[typeof(char)])); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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:
IFormFile
This is a sample payload that chrome will send for input and input multiple:
The contents of the files are not shown because the chrome dev tools scraps them. The form that I used is like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is interesting. At the moment, minimal APIs assumes that a top-level
IFormFileCollection
type refers toHttpContext.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 anIReadOnlyList<IFormFile>
type similar to whatIFormFIleCollection.GetFiles
returns.Binding for multiple
IFormFile
s with different names and erroring for multiple files with the same name makes sense though.There was a problem hiding this comment.
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
andinput type=file multiple
in a way that people can either use the inputs directly or rely onInputFile
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.There was a problem hiding this comment.
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:
as types backing the property.
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.