Skip to content

Commit 25488c5

Browse files
davidfowldavidfowl
authored
Added support for arrays of types with TryParse, StringValues and string[] for query strings and headers (#39809)
* Added support for string[], StringValues and T[] of types with a TryParse method - Added inferred query support for methods where the body inference is disabled. - Added a test
1 parent 123d9b5 commit 25488c5

6 files changed

+419
-41
lines changed

‎src/Http/Http.Extensions/src/RequestDelegateFactory.cs

+137-22
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public static partial class RequestDelegateFactory
3939
private static readonly MethodInfo ResultWriteResponseAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteResultWriteResponse), BindingFlags.NonPublic | BindingFlags.Static)!;
4040
private static readonly MethodInfo StringResultWriteResponseAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteWriteStringResponseAsync), BindingFlags.NonPublic | BindingFlags.Static)!;
4141
private static readonly MethodInfo JsonResultWriteResponseAsyncMethod = GetMethodInfo<Func<HttpResponse, object, Task>>((response, value) => HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, default));
42+
private static readonly MethodInfo StringIsNullOrEmptyMethod = typeof(string).GetMethod(nameof(string.IsNullOrEmpty), BindingFlags.Static | BindingFlags.Public)!;
4243

4344
private static readonly MethodInfo LogParameterBindingFailedMethod = GetMethodInfo<Action<HttpContext, string, string, string, bool>>((httpContext, parameterType, parameterName, sourceValue, shouldThrow) =>
4445
Log.ParameterBindingFailed(httpContext, parameterType, parameterName, sourceValue, shouldThrow));
@@ -71,6 +72,8 @@ public static partial class RequestDelegateFactory
7172
private static readonly ParameterExpression TempSourceStringExpr = ParameterBindingMethodCache.TempSourceStringExpr;
7273
private static readonly BinaryExpression TempSourceStringNotNullExpr = Expression.NotEqual(TempSourceStringExpr, Expression.Constant(null));
7374
private static readonly BinaryExpression TempSourceStringNullExpr = Expression.Equal(TempSourceStringExpr, Expression.Constant(null));
75+
private static readonly UnaryExpression TempSourceStringIsNotNullOrEmptyExpr = Expression.Not(Expression.Call(StringIsNullOrEmptyMethod, TempSourceStringExpr));
76+
7477
private static readonly string[] DefaultAcceptsContentType = new[] { "application/json" };
7578
private static readonly string[] FormFileContentType = new[] { "multipart/form-data" };
7679

@@ -202,6 +205,7 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory
202205
var errorMessage = BuildErrorMessageForInferredBodyParameter(factoryContext);
203206
throw new InvalidOperationException(errorMessage);
204207
}
208+
205209
if (factoryContext.JsonRequestBodyParameter is not null &&
206210
factoryContext.FirstFormRequestBodyParameter is not null)
207211
{
@@ -317,7 +321,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext
317321
{
318322
return BindParameterFromBindAsync(parameter, factoryContext);
319323
}
320-
else if (parameter.ParameterType == typeof(string) || ParameterBindingMethodCache.HasTryParseMethod(parameter))
324+
else if (parameter.ParameterType == typeof(string) || ParameterBindingMethodCache.HasTryParseMethod(parameter.ParameterType))
321325
{
322326
// 1. We bind from route values only, if route parameters are non-null and the parameter name is in that set.
323327
// 2. We bind from query only, if route parameters are non-null and the parameter name is NOT in that set.
@@ -342,6 +346,16 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext
342346
factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteOrQueryStringParameter);
343347
return BindParameterFromRouteValueOrQueryString(parameter, parameter.Name, factoryContext);
344348
}
349+
else if (factoryContext.DisableInferredFromBody && (
350+
(parameter.ParameterType.IsArray && ParameterBindingMethodCache.HasTryParseMethod(parameter.ParameterType.GetElementType()!)) ||
351+
parameter.ParameterType == typeof(string[]) ||
352+
parameter.ParameterType == typeof(StringValues)))
353+
{
354+
// We only infer parameter types if you have an array of TryParsables/string[]/StringValues, and DisableInferredFromBody is true
355+
356+
factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.QueryStringParameter);
357+
return BindParameterFromProperty(parameter, QueryExpr, parameter.Name, factoryContext, "query string");
358+
}
345359
else
346360
{
347361
if (factoryContext.ServiceProviderIsService is IServiceProviderIsService serviceProviderIsService)
@@ -884,22 +898,24 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres
884898
var parameterNameConstant = Expression.Constant(parameter.Name);
885899
var sourceConstant = Expression.Constant(source);
886900

887-
if (parameter.ParameterType == typeof(string))
901+
if (parameter.ParameterType == typeof(string) || parameter.ParameterType == typeof(string[]) || parameter.ParameterType == typeof(StringValues))
888902
{
889903
return BindParameterFromExpression(parameter, valueExpression, factoryContext, source);
890904
}
891905

892906
factoryContext.UsingTempSourceString = true;
893907

894-
var underlyingNullableType = Nullable.GetUnderlyingType(parameter.ParameterType);
908+
var targetParseType = parameter.ParameterType.IsArray ? parameter.ParameterType.GetElementType()! : parameter.ParameterType;
909+
910+
var underlyingNullableType = Nullable.GetUnderlyingType(targetParseType);
895911
var isNotNullable = underlyingNullableType is null;
896912

897-
var nonNullableParameterType = underlyingNullableType ?? parameter.ParameterType;
913+
var nonNullableParameterType = underlyingNullableType ?? targetParseType;
898914
var tryParseMethodCall = ParameterBindingMethodCache.FindTryParseMethod(nonNullableParameterType);
899915

900916
if (tryParseMethodCall is null)
901917
{
902-
var typeName = TypeNameHelper.GetTypeDisplayName(parameter.ParameterType, fullName: false);
918+
var typeName = TypeNameHelper.GetTypeDisplayName(targetParseType, fullName: false);
903919
throw new InvalidOperationException($"No public static bool {typeName}.TryParse(string, out {typeName}) method found for {parameter.Name}.");
904920
}
905921

@@ -940,8 +956,32 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres
940956
// param2_local = 42;
941957
// }
942958

959+
// string[]? values = httpContext.Request.Query["param1"].ToArray();
960+
// int[] param_local = values.Length > 0 ? new int[values.Length] : Array.Empty<int>();
961+
962+
// if (values != null)
963+
// {
964+
// int index = 0;
965+
// while (index < values.Length)
966+
// {
967+
// tempSourceString = values[i];
968+
// if (int.TryParse(tempSourceString, out var parsedValue))
969+
// {
970+
// param_local[i] = parsedValue;
971+
// }
972+
// else
973+
// {
974+
// wasParamCheckFailure = true;
975+
// Log.ParameterBindingFailed(httpContext, "Int32[]", "param1", tempSourceString);
976+
// break;
977+
// }
978+
//
979+
// index++
980+
// }
981+
// }
982+
943983
// If the parameter is nullable, create a "parsedValue" local to TryParse into since we cannot use the parameter directly.
944-
var parsedValue = isNotNullable ? argument : Expression.Variable(nonNullableParameterType, "parsedValue");
984+
var parsedValue = Expression.Variable(nonNullableParameterType, "parsedValue");
945985

946986
var failBlock = Expression.Block(
947987
Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)),
@@ -970,33 +1010,104 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres
9701010
)
9711011
);
9721012

1013+
var index = Expression.Variable(typeof(int), "index");
1014+
9731015
// If the parameter is nullable, we need to assign the "parsedValue" local to the nullable parameter on success.
974-
Expression tryParseExpression = isNotNullable ?
975-
Expression.IfThen(Expression.Not(tryParseCall), failBlock) :
976-
Expression.Block(new[] { parsedValue },
1016+
var tryParseExpression = Expression.Block(new[] { parsedValue },
9771017
Expression.IfThenElse(tryParseCall,
978-
Expression.Assign(argument, Expression.Convert(parsedValue, parameter.ParameterType)),
1018+
Expression.Assign(parameter.ParameterType.IsArray ? Expression.ArrayAccess(argument, index) : argument, Expression.Convert(parsedValue, targetParseType)),
9791019
failBlock));
9801020

981-
var ifNotNullTryParse = !parameter.HasDefaultValue ?
982-
Expression.IfThen(TempSourceStringNotNullExpr, tryParseExpression) :
983-
Expression.IfThenElse(TempSourceStringNotNullExpr,
984-
tryParseExpression,
985-
Expression.Assign(argument, Expression.Constant(parameter.DefaultValue)));
1021+
var ifNotNullTryParse = !parameter.HasDefaultValue
1022+
? Expression.IfThen(TempSourceStringNotNullExpr, tryParseExpression)
1023+
: Expression.IfThenElse(TempSourceStringNotNullExpr, tryParseExpression,
1024+
Expression.Assign(argument,
1025+
Expression.Constant(parameter.DefaultValue)));
1026+
1027+
var loopExit = Expression.Label();
1028+
1029+
// REVIEW: We can reuse this like we reuse temp source string
1030+
var stringArrayExpr = parameter.ParameterType.IsArray ? Expression.Variable(typeof(string[]), "tempStringArray") : null;
1031+
var elementTypeNullabilityInfo = parameter.ParameterType.IsArray ? factoryContext.NullabilityContext.Create(parameter)?.ElementType : null;
1032+
1033+
// Determine optionality of the element type of the array
1034+
var elementTypeOptional = !isNotNullable || (elementTypeNullabilityInfo?.ReadState != NullabilityState.NotNull);
1035+
1036+
// The loop that populates the resulting array values
1037+
var arrayLoop = parameter.ParameterType.IsArray ? Expression.Block(
1038+
// param_local = new int[values.Length];
1039+
Expression.Assign(argument, Expression.NewArrayBounds(parameter.ParameterType.GetElementType()!, Expression.ArrayLength(stringArrayExpr!))),
1040+
// index = 0
1041+
Expression.Assign(index, Expression.Constant(0)),
1042+
// while (index < values.Length)
1043+
Expression.Loop(
1044+
Expression.Block(
1045+
Expression.IfThenElse(
1046+
Expression.LessThan(index, Expression.ArrayLength(stringArrayExpr!)),
1047+
// tempSourceString = values[index];
1048+
Expression.Block(
1049+
Expression.Assign(TempSourceStringExpr, Expression.ArrayIndex(stringArrayExpr!, index)),
1050+
elementTypeOptional ? Expression.IfThen(TempSourceStringIsNotNullOrEmptyExpr, tryParseExpression)
1051+
: tryParseExpression
1052+
),
1053+
// else break
1054+
Expression.Break(loopExit)
1055+
),
1056+
// index++
1057+
Expression.PostIncrementAssign(index)
1058+
)
1059+
, loopExit)
1060+
) : null;
1061+
1062+
var fullParamCheckBlock = (parameter.ParameterType.IsArray, isOptional) switch
1063+
{
1064+
// (isArray: true, optional: true)
1065+
(true, true) =>
1066+
1067+
Expression.Block(
1068+
new[] { index, stringArrayExpr! },
1069+
// values = httpContext.Request.Query["id"];
1070+
Expression.Assign(stringArrayExpr!, valueExpression),
1071+
Expression.IfThen(
1072+
Expression.NotEqual(stringArrayExpr!, Expression.Constant(null)),
1073+
arrayLoop!
1074+
)
1075+
),
1076+
1077+
// (isArray: true, optional: false)
1078+
(true, false) =>
1079+
1080+
Expression.Block(
1081+
new[] { index, stringArrayExpr! },
1082+
// values = httpContext.Request.Query["id"];
1083+
Expression.Assign(stringArrayExpr!, valueExpression),
1084+
Expression.IfThenElse(
1085+
Expression.NotEqual(stringArrayExpr!, Expression.Constant(null)),
1086+
arrayLoop!,
1087+
failBlock
1088+
)
1089+
),
9861090

987-
var fullParamCheckBlock = !isOptional
988-
? Expression.Block(
1091+
// (isArray: false, optional: false)
1092+
(false, false) =>
1093+
1094+
Expression.Block(
9891095
// tempSourceString = httpContext.RequestValue["id"];
9901096
Expression.Assign(TempSourceStringExpr, valueExpression),
9911097
// if (tempSourceString == null) { ... } only produced when parameter is required
9921098
checkRequiredParaseableParameterBlock,
9931099
// if (tempSourceString != null) { ... }
994-
ifNotNullTryParse)
995-
: Expression.Block(
1100+
ifNotNullTryParse),
1101+
1102+
// (isArray: false, optional: true)
1103+
(false, true) =>
1104+
1105+
Expression.Block(
9961106
// tempSourceString = httpContext.RequestValue["id"];
9971107
Expression.Assign(TempSourceStringExpr, valueExpression),
9981108
// if (tempSourceString != null) { ... }
999-
ifNotNullTryParse);
1109+
ifNotNullTryParse)
1110+
};
10001111

10011112
factoryContext.ExtraLocals.Add(argument);
10021113
factoryContext.ParamCheckExpressions.Add(fullParamCheckBlock);
@@ -1065,7 +1176,12 @@ private static Expression BindParameterFromExpression(
10651176
}
10661177

10671178
private static Expression BindParameterFromProperty(ParameterInfo parameter, MemberExpression property, string key, FactoryContext factoryContext, string source) =>
1068-
BindParameterFromValue(parameter, GetValueFromProperty(property, key), factoryContext, source);
1179+
BindParameterFromValue(parameter, GetValueFromProperty(property, key, GetExpressionType(parameter.ParameterType)), factoryContext, source);
1180+
1181+
private static Type? GetExpressionType(Type type) =>
1182+
type.IsArray ? typeof(string[]) :
1183+
type == typeof(StringValues) ? typeof(StringValues) :
1184+
null;
10691185

10701186
private static Expression BindParameterFromRouteValueOrQueryString(ParameterInfo parameter, string key, FactoryContext factoryContext)
10711187
{
@@ -1077,7 +1193,6 @@ private static Expression BindParameterFromRouteValueOrQueryString(ParameterInfo
10771193
private static Expression BindParameterFromBindAsync(ParameterInfo parameter, FactoryContext factoryContext)
10781194
{
10791195
// We reference the boundValues array by parameter index here
1080-
var nullability = factoryContext.NullabilityContext.Create(parameter);
10811196
var isOptional = IsOptionalParameter(parameter, factoryContext);
10821197

10831198
// Get the BindAsync method for the type.

‎src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public static IEnumerable<object[]> TryParseStringParameterInfoData
144144
[MemberData(nameof(TryParseStringParameterInfoData))]
145145
public void HasTryParseStringMethod_ReturnsTrueWhenMethodExists(ParameterInfo parameterInfo)
146146
{
147-
Assert.True(new ParameterBindingMethodCache().HasTryParseMethod(parameterInfo));
147+
Assert.True(new ParameterBindingMethodCache().HasTryParseMethod(parameterInfo.ParameterType));
148148
}
149149

150150
[Fact]

0 commit comments

Comments
 (0)