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

Jump to bottom

Initial Form-binding support #44653

Merged
merged 9 commits into from
Nov 8, 2022
Merged

Initial Form-binding support #44653

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
dd6fabb
Adding initial Form-support
brunolins16 Oct 19, 2022
833ce38
clean up
brunolins16 Oct 19, 2022
c931b8e
PR feedback
brunolins16 Oct 31, 2022
65745d3
PR feedback
brunolins16 Oct 31, 2022
dadca52
Fix unit test
brunolins16 Nov 1, 2022
5c892fe
Adding Form Accepts Metadata later
brunolins16 Nov 1, 2022
455735e
clean up
brunolins16 Nov 1, 2022
a8c18b3
Fix warnings
brunolins16 Nov 1, 2022
1b4a884
Adding a test for InferMetadata
brunolins16 Nov 3, 2022
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
100 changes: 81 additions & 19 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public static partial class RequestDelegateFactory
private static readonly PropertyInfo RouteValuesIndexerProperty = typeof(RouteValueDictionary).GetProperty("Item")!;
private static readonly PropertyInfo HeaderIndexerProperty = typeof(IHeaderDictionary).GetProperty("Item")!;
private static readonly PropertyInfo FormFilesIndexerProperty = typeof(IFormFileCollection).GetProperty("Item")!;
private static readonly PropertyInfo FormIndexerProperty = typeof(IFormCollection).GetProperty("Item")!;

private static readonly MethodInfo JsonResultWriteResponseAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(WriteJsonResponse), BindingFlags.NonPublic | BindingFlags.Static)!;

Expand Down Expand Up @@ -110,6 +111,7 @@ public static partial class RequestDelegateFactory

private static readonly string[] DefaultAcceptsAndProducesContentType = new[] { JsonConstants.JsonContentType };
private static readonly string[] FormFileContentType = new[] { "multipart/form-data" };
private static readonly string[] FormContentType = new[] { "multipart/form-data", "application/x-www-form-urlencoded" };
private static readonly string[] PlaintextContentType = new[] { "text/plain" };

/// <summary>
Expand Down Expand Up @@ -710,13 +712,22 @@ private static Expression CreateArgument(ParameterInfo parameter, RequestDelegat

return BindParameterFromFormFiles(parameter, factoryContext);
}
else if (parameter.ParameterType != typeof(IFormFile))
else if (parameter.ParameterType == typeof(IFormFile))
{
throw new NotSupportedException(
$"{nameof(IFromFormMetadata)} is only supported for parameters of type {nameof(IFormFileCollection)} and {nameof(IFormFile)}.");
return BindParameterFromFormFile(parameter, formAttribute.Name ?? parameter.Name, factoryContext, RequestDelegateFactoryConstants.FormFileAttribute);
}
else if (parameter.ParameterType == typeof(IFormCollection))
{
if (!string.IsNullOrEmpty(formAttribute.Name))
{
throw new NotSupportedException(
$"Assigning a value to the {nameof(IFromFormMetadata)}.{nameof(IFromFormMetadata.Name)} property is not supported for parameters of type {nameof(IFormCollection)}.");

return BindParameterFromFormFile(parameter, formAttribute.Name ?? parameter.Name, factoryContext, RequestDelegateFactoryConstants.FormFileAttribute);
}
return BindParameterFromFormCollection(parameter, factoryContext);
}

return BindParameterFromFormItem(parameter, formAttribute.Name ?? parameter.Name, factoryContext);
}
else if (parameter.CustomAttributes.Any(a => typeof(IFromServiceMetadata).IsAssignableFrom(a.AttributeType)))
{
Expand Down Expand Up @@ -753,6 +764,10 @@ private static Expression CreateArgument(ParameterInfo parameter, RequestDelegat
{
return RequestAbortedExpr;
}
else if (parameter.ParameterType == typeof(IFormCollection))
{
return BindParameterFromFormCollection(parameter, factoryContext);
}
else if (parameter.ParameterType == typeof(IFormFileCollection))
{
return BindParameterFromFormFiles(parameter, factoryContext);
Expand Down Expand Up @@ -1820,26 +1835,71 @@ private static void AddInferredAcceptsMetadata(RequestDelegateFactoryContext fac
factoryContext.EndpointBuilder.Metadata.Add(new AcceptsMetadata(type, factoryContext.AllowEmptyRequestBody, contentTypes));
}

private static Expression BindParameterFromFormFiles(
private static Expression BindParameterFromFormCollection(
ParameterInfo parameter,
RequestDelegateFactoryContext factoryContext)
{
if (factoryContext.FirstFormRequestBodyParameter is null)
// Do not duplicate the metadata if there are multiple form parameters
if (!factoryContext.ReadForm)
{
factoryContext.FirstFormRequestBodyParameter = parameter;
AddInferredAcceptsMetadata(factoryContext, parameter.ParameterType, FormContentType);
}

factoryContext.FirstFormRequestBodyParameter ??= parameter;
factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.FormFileParameter);
factoryContext.ReadForm = true;

return BindParameterFromExpression(
parameter,
FormExpr,
factoryContext,
"body");
}

private static Expression BindParameterFromFormItem(
ParameterInfo parameter,
string key,
RequestDelegateFactoryContext factoryContext)
{
var valueExpression = GetValueFromProperty(FormExpr, FormIndexerProperty, key, GetExpressionType(parameter.ParameterType));

// Do not duplicate the metadata if there are multiple form parameters
if (!factoryContext.ReadForm)
{
AddInferredAcceptsMetadata(factoryContext, parameter.ParameterType, FormContentType);
}

factoryContext.FirstFormRequestBodyParameter ??= parameter;
factoryContext.TrackedParameters.Add(key, RequestDelegateFactoryConstants.FormAttribute);
factoryContext.ReadForm = true;

return BindParameterFromValue(
parameter,
valueExpression,
factoryContext,
"form");
}

private static Expression BindParameterFromFormFiles(
ParameterInfo parameter,
RequestDelegateFactoryContext factoryContext)
{
// Do not duplicate the metadata if there are multiple form file parameters
if (!factoryContext.ReadFormFile)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not very familiar with how forms work, but is there any way to have this code path run and register just "multipart/form-data", then have one of the other paths run which would normally add "application/x-www-form-urlencoded" as well, but now they won't because we set ReadForm below.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure if I get your question correctly but, while might be possible send file using application/x-www-form-urlencoded (i am not very familiar 🤷‍♂️ ) . The FormFeature only read files from a multipart/form-data request.

aspnetcore/src/Http/Http/src/Features/FormFeature.cs

Line 171 in 8dc4325

else if (HasMultipartFormContentType(contentType))

My approach here is to keep not adding duplicated metadata, however, since we don't know the parameter ordering and we process them sequentially, in case we found a IFormFile or IFormFileCollection an additional is added, for the first parameter only, with multipart/form-data and since the last metadata is the most relevant it should basically this one been used, make sense?

{
AddInferredAcceptsMetadata(factoryContext, parameter.ParameterType, FormFileContentType);
}

factoryContext.FirstFormRequestBodyParameter ??= parameter;
factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.FormFileParameter);
factoryContext.ReadForm = true;
factoryContext.ReadFormFile = true;
brunolins16 marked this conversation as resolved.
Show resolved Hide resolved

return BindParameterFromExpression(parameter, FormFilesExpr, factoryContext, "body");
return BindParameterFromExpression(
parameter,
FormFilesExpr,
factoryContext,
"body");
}

private static Expression BindParameterFromFormFile(
Expand All @@ -1848,24 +1908,24 @@ private static Expression BindParameterFromFormFile(
RequestDelegateFactoryContext factoryContext,
string trackedParameterSource)
{
if (factoryContext.FirstFormRequestBodyParameter is null)
{
factoryContext.FirstFormRequestBodyParameter = parameter;
}

factoryContext.TrackedParameters.Add(key, trackedParameterSource);
var valueExpression = GetValueFromProperty(FormFilesExpr, FormFilesIndexerProperty, key, typeof(IFormFile));

// Do not duplicate the metadata if there are multiple form parameters
if (!factoryContext.ReadForm)
// Do not duplicate the metadata if there are multiple form file parameters
if (!factoryContext.ReadFormFile)
{
AddInferredAcceptsMetadata(factoryContext, parameter.ParameterType, FormFileContentType);
}

factoryContext.FirstFormRequestBodyParameter ??= parameter;
factoryContext.TrackedParameters.Add(key, trackedParameterSource);
factoryContext.ReadForm = true;
factoryContext.ReadFormFile = true;

var valueExpression = GetValueFromProperty(FormFilesExpr, FormFilesIndexerProperty, key, typeof(IFormFile));

return BindParameterFromExpression(parameter, valueExpression, factoryContext, "form file");
return BindParameterFromExpression(
parameter,
valueExpression,
factoryContext,
"form file");
Copy link
Member

Choose a reason for hiding this comment

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

Nit: make this a constant?

}

private static Expression BindParameterFromBody(ParameterInfo parameter, bool allowEmpty, RequestDelegateFactoryContext factoryContext)
Expand Down Expand Up @@ -2210,12 +2270,14 @@ private static class RequestDelegateFactoryConstants
public const string BodyAttribute = "Body (Attribute)";
public const string ServiceAttribute = "Service (Attribute)";
public const string FormFileAttribute = "Form File (Attribute)";
public const string FormAttribute = "Form (Attribute)";
public const string RouteParameter = "Route (Inferred)";
public const string QueryStringParameter = "Query String (Inferred)";
public const string ServiceParameter = "Services (Inferred)";
public const string BodyParameter = "Body (Inferred)";
public const string RouteOrQueryStringParameter = "Route or Query String (Inferred)";
public const string FormFileParameter = "Form File (Inferred)";
public const string FormCollectionParameter = "Form Collection (Inferred)";
brunolins16 marked this conversation as resolved.
Show resolved Hide resolved
public const string PropertyAsParameter = "As Parameter (Attribute)";
}

Expand Down
1 change: 1 addition & 0 deletions src/Http/Http.Extensions/src/RequestDelegateFactoryContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ internal sealed class RequestDelegateFactoryContext
public NullabilityInfoContext NullabilityContext { get; } = new();

public bool ReadForm { get; set; }
public bool ReadFormFile { get; set; }
public ParameterInfo? FirstFormRequestBodyParameter { get; set; }
// Properties for constructing and managing filters
public List<Expression> ContextArgAccess { get; } = new();
Expand Down
Loading