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.

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

Pass in IFormFileCollection into minimal hosting delegate #35100

Closed
wants to merge 1 commit into from

Conversation

jchannon
Copy link
Contributor

@jchannon jchannon commented Aug 6, 2021

This adds IFormFileCollection IFormFileCollection into minimal hosting delegate.

Fixes #34303

NOTE: Tests not added as cannot get any IDE to work so happy for someone to take this commit and carry on the work

@ghost ghost added area-runtime community-contribution Indicates that the PR has been added by a community member labels Aug 6, 2021
@davidfowl
Copy link
Member

This adds support for IFormFileCollection but what about IFormFile?

@jchannon
Copy link
Contributor Author

jchannon commented Aug 6, 2021 via email

@@ -55,6 +55,7 @@ public static partial class RequestDelegateFactory
private static readonly MemberExpression HttpResponseExpr = Expression.Property(HttpContextExpr, nameof(HttpContext.Response));
private static readonly MemberExpression RequestAbortedExpr = Expression.Property(HttpContextExpr, nameof(HttpContext.RequestAborted));
private static readonly MemberExpression UserExpr = Expression.Property(HttpContextExpr, nameof(HttpContext.User));
private static readonly MemberExpression FormFileExpr = Expression.Property(HttpContextExpr, nameof(HttpContext.Request.Form.Files));
Copy link
Member

Choose a reason for hiding this comment

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

This isn't how we would do this. Reading the form body should be done asynchronously. We'd probably store state on the FactoryContext that describes this is a form body that needs files. Take a look at BindParameterFromBody and HandleRequestBodyAndCompileRequestDelegate

@davidfowl
Copy link
Member

This change is a bit more complex than what's implemented here:

  • We need to pre-read the form in this case asynchronously if IFormFile or IFormFileCollection is used. This is alot simpler than what we do for JSON bodies as we don't need to pass any state after the read but we do need to pre-read the body in this case.
  • We need to add support to the endpoints API explorer for detecting IFormFile and IFormFileCollection in the request body for Open API support.

@jchannon jchannon closed this Aug 6, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support binding to IFormFile in minimal routing APIs
3 participants