From c9cbe6872901d4d6fd79873686bb89d15ac191c0 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 15 May 2024 20:03:07 +0200 Subject: [PATCH 01/17] Fixes --- .../src/RequestDelegateFactory.cs | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 5ab4ccc690a8..94773c199e0d 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -276,7 +276,7 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat var serviceProvider = options?.ServiceProvider ?? options?.EndpointBuilder?.ApplicationServices ?? EmptyServiceProvider.Instance; var endpointBuilder = options?.EndpointBuilder ?? new RdfEndpointBuilder(serviceProvider); var jsonSerializerOptions = serviceProvider.GetService>()?.Value.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; - var formDataMapperOptions = new FormDataMapperOptions();; + var formDataMapperOptions = new FormDataMapperOptions(); ; var factoryContext = new RequestDelegateFactoryContext { @@ -719,7 +719,25 @@ private static Expression CreateArgument(ParameterInfo parameter, RequestDelegat else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } headerAttribute) { factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.HeaderAttribute); - return BindParameterFromProperty(parameter, HeadersExpr, HeaderIndexerProperty, headerAttribute.Name ?? parameter.Name, factoryContext, "header"); + + var headerName = headerAttribute.Name ?? parameter.Name; + var headerValuesExpr = Expression.Property(HeadersExpr, HeaderIndexerProperty, Expression.Constant(headerName)); + var stringValueExpr = Expression.Call(headerValuesExpr, "ToString", Type.EmptyTypes); + + Expression boundValueExpr; + + if (parameter.ParameterType.IsArray || typeof(IEnumerable).IsAssignableFrom(parameter.ParameterType)) + { + var valuesExpr = Expression.Call(stringValueExpr, "Split", Type.EmptyTypes, Expression.Constant(new[] { ',' }, typeof(char[]))); + var trimmedValuesExpr = Expression.Call(typeof(RequestDelegateFactory), nameof(TrimValues), Type.EmptyTypes, valuesExpr); + boundValueExpr = Expression.Convert(trimmedValuesExpr, parameter.ParameterType); + } + else + { + boundValueExpr = Expression.Convert(stringValueExpr, parameter.ParameterType); + } + + return boundValueExpr; } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } bodyAttribute) { @@ -758,7 +776,6 @@ private static Expression CreateArgument(ParameterInfo parameter, RequestDelegat { throw new NotSupportedException( $"Assigning a value to the {nameof(IFromFormMetadata)}.{nameof(IFromFormMetadata.Name)} property is not supported for parameters of type {nameof(IFormCollection)}."); - } return BindParameterFromFormCollection(parameter, factoryContext); } @@ -2846,6 +2863,15 @@ private static void FormatTrackedParameters(RequestDelegateFactoryContext factor } } + private static string[] TrimValues(string[] values) + { + for (var i = 0; i < values.Length; i++) + { + values[i] = values[i].Trim(); + } + return values; + } + private sealed class RdfEndpointBuilder : EndpointBuilder { public RdfEndpointBuilder(IServiceProvider applicationServices) @@ -2864,4 +2890,5 @@ private sealed class EmptyServiceProvider : IServiceProvider public static EmptyServiceProvider Instance { get; } = new EmptyServiceProvider(); public object? GetService(Type serviceType) => null; } + } From b5a2e38ff7d26d13b6dabbb90d0dc64e88041e0e Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Wed, 15 May 2024 20:29:38 +0200 Subject: [PATCH 02/17] Removing extra ';' --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 94773c199e0d..f7ff1d98aeb2 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -276,7 +276,7 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat var serviceProvider = options?.ServiceProvider ?? options?.EndpointBuilder?.ApplicationServices ?? EmptyServiceProvider.Instance; var endpointBuilder = options?.EndpointBuilder ?? new RdfEndpointBuilder(serviceProvider); var jsonSerializerOptions = serviceProvider.GetService>()?.Value.SerializerOptions ?? JsonOptions.DefaultSerializerOptions; - var formDataMapperOptions = new FormDataMapperOptions(); ; + var formDataMapperOptions = new FormDataMapperOptions(); var factoryContext = new RequestDelegateFactoryContext { From 6158c14738364bae2fc8c32cc9191ee8c4d66f1a Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 20 May 2024 12:49:37 +0200 Subject: [PATCH 03/17] Ensuring single header values and comma seperated header values are treated correctly with tests --- .../test/RequestDelegateFactoryTests.cs | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 452eeb6c0674..32a4b2429144 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -3150,6 +3150,51 @@ static void TestAction([AsParameters] ParameterListRequiredNullableStringFromDif Assert.Null(httpContext.Items["RequiredHeaderParam"]); } + [Fact] + public async Task RequestDelegate_ShouldHandleSingleHeaderValue() + { + // Arrange + var httpContext = CreateHttpContext(); + httpContext.Request.Headers["TestHeader"] = "HeaderValue"; + + var resultFactory = RequestDelegateFactory.Create((HttpContext httpContext, [FromHeader(Name = "TestHeader")] string headerValue) => + { + httpContext.Items["headerValue"] = headerValue; + }); + + var requestDelegate = resultFactory.RequestDelegate; + + // Act + await requestDelegate(httpContext); + + // Assert + Assert.Equal("HeaderValue", httpContext.Items["headerValue"]); + } + + [Fact] + public async Task RequestDelegate_ShouldHandleCommaSeparatedHeaderValues() + { + // Arrange + var httpContext = CreateHttpContext(); + httpContext.Request.Headers["TestHeader"] = "Value1,Value2,Value3"; + + var resultFactory = RequestDelegateFactory.Create((HttpContext httpContext, [FromHeader(Name = "TestHeader")] string[] headerValues) => + { + httpContext.Items["headerValues"] = headerValues; + }); + + var requestDelegate = resultFactory.RequestDelegate; + + // Act + await requestDelegate(httpContext); + + // Assert + Assert.True(httpContext.Items.ContainsKey("headerValues")); + var headerValues = httpContext.Items["headerValues"] as string[]; + Assert.NotNull(headerValues); + Assert.Equal(new[] { "Value1", "Value2", "Value3" }, headerValues); + } + #nullable disable private class ParameterListMixedRequiredStringsFromDifferentSources { From 63dcea3e808e38cb64395e90b09864c165bfc080 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 20 May 2024 14:56:09 +0200 Subject: [PATCH 04/17] Test ammendments --- src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 32a4b2429144..fafd7e9c19ef 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -3168,6 +3168,7 @@ public async Task RequestDelegate_ShouldHandleSingleHeaderValue() await requestDelegate(httpContext); // Assert + Assert.True(httpContext.Items.ContainsKey("headerValue")); Assert.Equal("HeaderValue", httpContext.Items["headerValue"]); } From 8e5f7860d20e03fff6c9f1c924f79577b9d05b73 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 20 May 2024 14:57:35 +0200 Subject: [PATCH 05/17] Adapting BindParameterFromExpression to handle nessisary cases --- .../src/RequestDelegateFactory.cs | 115 +++++++++++------- 1 file changed, 70 insertions(+), 45 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index f7ff1d98aeb2..522c97a427fc 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -719,25 +719,7 @@ private static Expression CreateArgument(ParameterInfo parameter, RequestDelegat else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } headerAttribute) { factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.HeaderAttribute); - - var headerName = headerAttribute.Name ?? parameter.Name; - var headerValuesExpr = Expression.Property(HeadersExpr, HeaderIndexerProperty, Expression.Constant(headerName)); - var stringValueExpr = Expression.Call(headerValuesExpr, "ToString", Type.EmptyTypes); - - Expression boundValueExpr; - - if (parameter.ParameterType.IsArray || typeof(IEnumerable).IsAssignableFrom(parameter.ParameterType)) - { - var valuesExpr = Expression.Call(stringValueExpr, "Split", Type.EmptyTypes, Expression.Constant(new[] { ',' }, typeof(char[]))); - var trimmedValuesExpr = Expression.Call(typeof(RequestDelegateFactory), nameof(TrimValues), Type.EmptyTypes, valuesExpr); - boundValueExpr = Expression.Convert(trimmedValuesExpr, parameter.ParameterType); - } - else - { - boundValueExpr = Expression.Convert(stringValueExpr, parameter.ParameterType); - } - - return boundValueExpr; + return BindParameterFromProperty(parameter, HeadersExpr, HeaderIndexerProperty, headerAttribute.Name ?? parameter.Name, factoryContext, "header"); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } bodyAttribute) { @@ -1645,6 +1627,7 @@ private static Expression BindParameterFromKeyedService(ParameterInfo parameter, private static Expression BindParameterFromValue(ParameterInfo parameter, Expression valueExpression, RequestDelegateFactoryContext factoryContext, string source) { + if (parameter.ParameterType == typeof(string) || parameter.ParameterType == typeof(string[]) || parameter.ParameterType == typeof(StringValues) || parameter.ParameterType == typeof(StringValues?)) { @@ -1871,10 +1854,10 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres } private static Expression BindParameterFromExpression( - ParameterInfo parameter, - Expression valueExpression, - RequestDelegateFactoryContext factoryContext, - string source) + ParameterInfo parameter, + Expression valueExpression, + RequestDelegateFactoryContext factoryContext, + string source) { var nullability = factoryContext.NullabilityContext.Create(parameter); var isOptional = IsOptionalParameter(parameter, factoryContext); @@ -1885,16 +1868,62 @@ private static Expression BindParameterFromExpression( var parameterNameConstant = Expression.Constant(parameter.Name); var sourceConstant = Expression.Constant(source); + if (source == "header" && (parameter.ParameterType.IsArray || typeof(IEnumerable).IsAssignableFrom(parameter.ParameterType))) + { + // Convert StringValues to string array + var stringValuesExpr = Expression.Convert(valueExpression, typeof(StringValues)); + var toStringArrayMethod = typeof(StringValues).GetMethod(nameof(StringValues.ToArray)); + var headerValuesArrayExpr = Expression.Call(stringValuesExpr, toStringArrayMethod); + + // Process each string in the array: split and trim + var splitAndTrimMethod = typeof(RequestDelegateFactory).GetMethod(nameof(SplitAndTrim), System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic); + var splitAndTrimExpr = Expression.Call(splitAndTrimMethod, headerValuesArrayExpr); + + // Convert the split and trimmed values to the parameter type + var boundValueExpr = Expression.Convert(splitAndTrimExpr, parameter.ParameterType); + + if (!isOptional) + { + var checkRequiredStringParameterBlock = Expression.Block( + Expression.Assign(argument, boundValueExpr), + Expression.IfThen(Expression.Equal(argument, Expression.Constant(null)), + Expression.Block( + Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)), + Expression.Call(LogRequiredParameterNotProvidedMethod, + HttpContextExpr, parameterTypeNameConstant, parameterNameConstant, sourceConstant, + Expression.Constant(factoryContext.ThrowOnBadRequest)) + ) + ) + ); + + factoryContext.ExtraLocals.Add(argument); + factoryContext.ParamCheckExpressions.Add(checkRequiredStringParameterBlock); + return argument; + } + + // Allow nullable parameters that don't have a default value + if (nullability.ReadState != NullabilityState.NotNull && !parameter.HasDefaultValue) + { + if (parameter.ParameterType == typeof(StringValues?)) + { + return Expression.Block( + Expression.Condition(Expression.Equal(boundValueExpr, Expression.Convert(Expression.Constant(StringValues.Empty), parameter.ParameterType)), + Expression.Convert(Expression.Constant(null), parameter.ParameterType), + boundValueExpr + ) + ); + } + return boundValueExpr; + } + + return Expression.Block( + Expression.Condition(Expression.NotEqual(boundValueExpr, Expression.Constant(null)), + boundValueExpr, + Expression.Convert(Expression.Constant(parameter.DefaultValue), parameter.ParameterType))); + } + if (!isOptional) { - // The following is produced if the parameter is required: - // - // argument = value["param1"]; - // if (argument == null) - // { - // wasParamCheckFailure = true; - // Log.RequiredParameterNotProvided(httpContext, "TypeOfValue", "param1"); - // } var checkRequiredStringParameterBlock = Expression.Block( Expression.Assign(argument, valueExpression), Expression.IfThen(Expression.Equal(argument, Expression.Constant(null)), @@ -1907,35 +1936,25 @@ private static Expression BindParameterFromExpression( ) ); - // NOTE: when StringValues is used as a parameter, value["some_unpresent_parameter"] returns StringValue.Empty, and it's equivalent to (string?)null - factoryContext.ExtraLocals.Add(argument); factoryContext.ParamCheckExpressions.Add(checkRequiredStringParameterBlock); return argument; } - // Allow nullable parameters that don't have a default value if (nullability.ReadState != NullabilityState.NotNull && !parameter.HasDefaultValue) { if (parameter.ParameterType == typeof(StringValues?)) { - // when Nullable is used and the actual value is StringValues.Empty, we should pass in a Nullable return Expression.Block( Expression.Condition(Expression.Equal(valueExpression, Expression.Convert(Expression.Constant(StringValues.Empty), parameter.ParameterType)), - Expression.Convert(Expression.Constant(null), parameter.ParameterType), - valueExpression - ) - ); + Expression.Convert(Expression.Constant(null), parameter.ParameterType), + valueExpression + ) + ); } return valueExpression; } - // The following is produced if the parameter is optional. Note that we convert the - // default value to the target ParameterType to address scenarios where the user is - // is setting null as the default value in a context where nullability is disabled. - // - // param1_local = httpContext.RouteValue["param1"] ?? httpContext.Query["param1"]; - // param1_local != null ? param1_local : Convert(null, Int32) return Expression.Block( Expression.Condition(Expression.NotEqual(valueExpression, Expression.Constant(null)), valueExpression, @@ -2872,6 +2891,12 @@ private static string[] TrimValues(string[] values) return values; } + private static string[] SplitAndTrim(string[] values) + { + var result = values.SelectMany(v => v.Split(',').Select(s => s.Trim())).ToArray(); + return result; + } + private sealed class RdfEndpointBuilder : EndpointBuilder { public RdfEndpointBuilder(IServiceProvider applicationServices) From b52fad2ee6e48e19b6c0e6ebd82e5daf3241f334 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 20 May 2024 15:04:54 +0200 Subject: [PATCH 06/17] Removing trim method --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 522c97a427fc..aaa734639c7e 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -2882,15 +2882,6 @@ private static void FormatTrackedParameters(RequestDelegateFactoryContext factor } } - private static string[] TrimValues(string[] values) - { - for (var i = 0; i < values.Length; i++) - { - values[i] = values[i].Trim(); - } - return values; - } - private static string[] SplitAndTrim(string[] values) { var result = values.SelectMany(v => v.Split(',').Select(s => s.Trim())).ToArray(); From f415785de817eb6ab3c9f7a33cf9eab783e9385e Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 20 May 2024 15:10:21 +0200 Subject: [PATCH 07/17] Adding comments --- .../Http.Extensions/src/RequestDelegateFactory.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index aaa734639c7e..cfe9b8debc27 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1896,6 +1896,8 @@ private static Expression BindParameterFromExpression( ) ); + // NOTE: when StringValues is used as a parameter, value["some_unpresent_parameter"] returns StringValue.Empty, and it's equivalent to (string?)null + factoryContext.ExtraLocals.Add(argument); factoryContext.ParamCheckExpressions.Add(checkRequiredStringParameterBlock); return argument; @@ -1924,6 +1926,7 @@ private static Expression BindParameterFromExpression( if (!isOptional) { + // The following is produced if the parameter is optional. var checkRequiredStringParameterBlock = Expression.Block( Expression.Assign(argument, valueExpression), Expression.IfThen(Expression.Equal(argument, Expression.Constant(null)), @@ -1936,15 +1939,19 @@ private static Expression BindParameterFromExpression( ) ); + // NOTE: when StringValues is used as a parameter, value["some_unpresent_parameter"] returns StringValue.Empty, and it's equivalent to (string?)null + factoryContext.ExtraLocals.Add(argument); factoryContext.ParamCheckExpressions.Add(checkRequiredStringParameterBlock); return argument; } + // Allow nullable parameters that don't have a default value if (nullability.ReadState != NullabilityState.NotNull && !parameter.HasDefaultValue) { if (parameter.ParameterType == typeof(StringValues?)) { + // when Nullable is used and the actual value is StringValues.Empty, we should pass in a Nullable return Expression.Block( Expression.Condition(Expression.Equal(valueExpression, Expression.Convert(Expression.Constant(StringValues.Empty), parameter.ParameterType)), Expression.Convert(Expression.Constant(null), parameter.ParameterType), @@ -1955,6 +1962,9 @@ private static Expression BindParameterFromExpression( return valueExpression; } + // The following is produced if the parameter is optional. Note that we convert the + // default value to the target ParameterType to address scenarios where the user is + // is setting null as the default value in a context where nullability is disabled. return Expression.Block( Expression.Condition(Expression.NotEqual(valueExpression, Expression.Constant(null)), valueExpression, @@ -2906,5 +2916,4 @@ private sealed class EmptyServiceProvider : IServiceProvider public static EmptyServiceProvider Instance { get; } = new EmptyServiceProvider(); public object? GetService(Type serviceType) => null; } - } From aeb3354847ce7ae32162965424750ed7262ab497 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 20 May 2024 15:12:04 +0200 Subject: [PATCH 08/17] Clean up --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index cfe9b8debc27..81736be43bac 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1870,16 +1870,13 @@ private static Expression BindParameterFromExpression( if (source == "header" && (parameter.ParameterType.IsArray || typeof(IEnumerable).IsAssignableFrom(parameter.ParameterType))) { - // Convert StringValues to string array var stringValuesExpr = Expression.Convert(valueExpression, typeof(StringValues)); var toStringArrayMethod = typeof(StringValues).GetMethod(nameof(StringValues.ToArray)); var headerValuesArrayExpr = Expression.Call(stringValuesExpr, toStringArrayMethod); - // Process each string in the array: split and trim var splitAndTrimMethod = typeof(RequestDelegateFactory).GetMethod(nameof(SplitAndTrim), System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic); var splitAndTrimExpr = Expression.Call(splitAndTrimMethod, headerValuesArrayExpr); - // Convert the split and trimmed values to the parameter type var boundValueExpr = Expression.Convert(splitAndTrimExpr, parameter.ParameterType); if (!isOptional) @@ -1897,7 +1894,6 @@ private static Expression BindParameterFromExpression( ); // NOTE: when StringValues is used as a parameter, value["some_unpresent_parameter"] returns StringValue.Empty, and it's equivalent to (string?)null - factoryContext.ExtraLocals.Add(argument); factoryContext.ParamCheckExpressions.Add(checkRequiredStringParameterBlock); return argument; @@ -1939,8 +1935,6 @@ private static Expression BindParameterFromExpression( ) ); - // NOTE: when StringValues is used as a parameter, value["some_unpresent_parameter"] returns StringValue.Empty, and it's equivalent to (string?)null - factoryContext.ExtraLocals.Add(argument); factoryContext.ParamCheckExpressions.Add(checkRequiredStringParameterBlock); return argument; @@ -1962,9 +1956,6 @@ private static Expression BindParameterFromExpression( return valueExpression; } - // The following is produced if the parameter is optional. Note that we convert the - // default value to the target ParameterType to address scenarios where the user is - // is setting null as the default value in a context where nullability is disabled. return Expression.Block( Expression.Condition(Expression.NotEqual(valueExpression, Expression.Constant(null)), valueExpression, From 5b31f5019600b57ca84e246d3d87cc8dbd870f38 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 20 May 2024 16:17:33 +0200 Subject: [PATCH 09/17] Handling possible null occurances --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 81736be43bac..6c83cd9be11a 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1872,11 +1872,20 @@ private static Expression BindParameterFromExpression( { var stringValuesExpr = Expression.Convert(valueExpression, typeof(StringValues)); var toStringArrayMethod = typeof(StringValues).GetMethod(nameof(StringValues.ToArray)); + if (toStringArrayMethod == null) + { + throw new InvalidOperationException("The method 'ToArray' could not be found on 'StringValues'."); + } var headerValuesArrayExpr = Expression.Call(stringValuesExpr, toStringArrayMethod); var splitAndTrimMethod = typeof(RequestDelegateFactory).GetMethod(nameof(SplitAndTrim), System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic); + if (splitAndTrimMethod == null) + { + throw new InvalidOperationException("The method 'SplitAndTrim' could not be found on 'RequestDelegateFactory'."); + } var splitAndTrimExpr = Expression.Call(splitAndTrimMethod, headerValuesArrayExpr); + var boundValueExpr = Expression.Convert(splitAndTrimExpr, parameter.ParameterType); if (!isOptional) From 1e3ec2e3749c6ac2b6728b9255c67fed2208f0fc Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 20 May 2024 18:12:11 +0200 Subject: [PATCH 10/17] Removing extra lines --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 6c83cd9be11a..e1dbc8b425ee 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1885,7 +1885,6 @@ private static Expression BindParameterFromExpression( } var splitAndTrimExpr = Expression.Call(splitAndTrimMethod, headerValuesArrayExpr); - var boundValueExpr = Expression.Convert(splitAndTrimExpr, parameter.ParameterType); if (!isOptional) From 9c3f5758a28f654af20d0c2a0bf0d40d2bdac7b4 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Mon, 20 May 2024 20:23:30 +0200 Subject: [PATCH 11/17] Improving performance and mimizing memory usage and allocations within `SplitAndTrim(string[] values)` --- .../src/RequestDelegateFactory.cs | 36 +++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index e1dbc8b425ee..a93d0f63db75 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -2893,8 +2893,40 @@ private static void FormatTrackedParameters(RequestDelegateFactoryContext factor private static string[] SplitAndTrim(string[] values) { - var result = values.SelectMany(v => v.Split(',').Select(s => s.Trim())).ToArray(); - return result; + if (values == null || values.Length == 0) + { + return []; + } + + var result = new List(); + + foreach (var value in values) + { + if (string.IsNullOrWhiteSpace(value)) + { + continue; + } + + var span = value.AsSpan(); + var start = 0; + + for (var i = 0; i <= span.Length; i++) + { + if (i == span.Length || span[i] == ',') + { + var slice = span.Slice(start, i - start).Trim(); + + if (!slice.IsEmpty) + { + result.Add(new string(slice)); + } + + start = i + 1; + } + } + } + + return [.. result]; } private sealed class RdfEndpointBuilder : EndpointBuilder From c78f1a9ee0faced56417ffcf8f90f7fa6947eb83 Mon Sep 17 00:00:00 2001 From: Matthew Leslie <67387027+MattyLeslie@users.noreply.github.com> Date: Mon, 15 Jul 2024 10:37:44 +0200 Subject: [PATCH 12/17] Update src/Http/Http.Extensions/src/RequestDelegateFactory.cs Co-authored-by: Brennan --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index a93d0f63db75..cd387c179f41 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1871,11 +1871,7 @@ private static Expression BindParameterFromExpression( if (source == "header" && (parameter.ParameterType.IsArray || typeof(IEnumerable).IsAssignableFrom(parameter.ParameterType))) { var stringValuesExpr = Expression.Convert(valueExpression, typeof(StringValues)); - var toStringArrayMethod = typeof(StringValues).GetMethod(nameof(StringValues.ToArray)); - if (toStringArrayMethod == null) - { - throw new InvalidOperationException("The method 'ToArray' could not be found on 'StringValues'."); - } + var toStringArrayMethod = typeof(StringValues).GetMethod(nameof(StringValues.ToArray))!; var headerValuesArrayExpr = Expression.Call(stringValuesExpr, toStringArrayMethod); var splitAndTrimMethod = typeof(RequestDelegateFactory).GetMethod(nameof(SplitAndTrim), System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic); From 99ec84b903aea9d51e277d6b0d08c42e95b829a7 Mon Sep 17 00:00:00 2001 From: Matthew Leslie <67387027+MattyLeslie@users.noreply.github.com> Date: Mon, 15 Jul 2024 10:38:05 +0200 Subject: [PATCH 13/17] Update src/Http/Http.Extensions/src/RequestDelegateFactory.cs Co-authored-by: Brennan --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index cd387c179f41..afd6225559be 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1874,11 +1874,7 @@ private static Expression BindParameterFromExpression( var toStringArrayMethod = typeof(StringValues).GetMethod(nameof(StringValues.ToArray))!; var headerValuesArrayExpr = Expression.Call(stringValuesExpr, toStringArrayMethod); - var splitAndTrimMethod = typeof(RequestDelegateFactory).GetMethod(nameof(SplitAndTrim), System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic); - if (splitAndTrimMethod == null) - { - throw new InvalidOperationException("The method 'SplitAndTrim' could not be found on 'RequestDelegateFactory'."); - } + var splitAndTrimMethod = typeof(RequestDelegateFactory).GetMethod(nameof(SplitAndTrim), System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic)!; var splitAndTrimExpr = Expression.Call(splitAndTrimMethod, headerValuesArrayExpr); var boundValueExpr = Expression.Convert(splitAndTrimExpr, parameter.ParameterType); From 9db0e6fc4f1ca9d4a0b4712c43ca06dd81978b06 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Tue, 16 Jul 2024 12:05:03 +0200 Subject: [PATCH 14/17] Fixing indentations --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index afd6225559be..bd7dd98233cf 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1854,10 +1854,10 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres } private static Expression BindParameterFromExpression( - ParameterInfo parameter, - Expression valueExpression, - RequestDelegateFactoryContext factoryContext, - string source) + ParameterInfo parameter, + Expression valueExpression, + RequestDelegateFactoryContext factoryContext, + string source) { var nullability = factoryContext.NullabilityContext.Create(parameter); var isOptional = IsOptionalParameter(parameter, factoryContext); From 8bcad90bd9124a78e445a3b1d371769412b87f10 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Tue, 16 Jul 2024 12:40:30 +0200 Subject: [PATCH 15/17] Fixing duplicate code and using MemoryAllocations.Split --- .../src/RequestDelegateFactory.cs | 86 ++++++------------- 1 file changed, 27 insertions(+), 59 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index bd7dd98233cf..909b9d053c46 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1854,10 +1854,10 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres } private static Expression BindParameterFromExpression( - ParameterInfo parameter, - Expression valueExpression, - RequestDelegateFactoryContext factoryContext, - string source) + ParameterInfo parameter, + Expression valueExpression, + RequestDelegateFactoryContext factoryContext, + string source) { var nullability = factoryContext.NullabilityContext.Create(parameter); var isOptional = IsOptionalParameter(parameter, factoryContext); @@ -1868,7 +1868,7 @@ private static Expression BindParameterFromExpression( var parameterNameConstant = Expression.Constant(parameter.Name); var sourceConstant = Expression.Constant(source); - if (source == "header" && (parameter.ParameterType.IsArray || typeof(IEnumerable).IsAssignableFrom(parameter.ParameterType))) + if (source == "header" && (parameter.ParameterType == typeof(string[]) || typeof(IEnumerable).IsAssignableFrom(parameter.ParameterType))) { var stringValuesExpr = Expression.Convert(valueExpression, typeof(StringValues)); var toStringArrayMethod = typeof(StringValues).GetMethod(nameof(StringValues.ToArray))!; @@ -1877,52 +1877,19 @@ private static Expression BindParameterFromExpression( var splitAndTrimMethod = typeof(RequestDelegateFactory).GetMethod(nameof(SplitAndTrim), System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic)!; var splitAndTrimExpr = Expression.Call(splitAndTrimMethod, headerValuesArrayExpr); - var boundValueExpr = Expression.Convert(splitAndTrimExpr, parameter.ParameterType); - - if (!isOptional) - { - var checkRequiredStringParameterBlock = Expression.Block( - Expression.Assign(argument, boundValueExpr), - Expression.IfThen(Expression.Equal(argument, Expression.Constant(null)), - Expression.Block( - Expression.Assign(WasParamCheckFailureExpr, Expression.Constant(true)), - Expression.Call(LogRequiredParameterNotProvidedMethod, - HttpContextExpr, parameterTypeNameConstant, parameterNameConstant, sourceConstant, - Expression.Constant(factoryContext.ThrowOnBadRequest)) - ) - ) - ); - - // NOTE: when StringValues is used as a parameter, value["some_unpresent_parameter"] returns StringValue.Empty, and it's equivalent to (string?)null - factoryContext.ExtraLocals.Add(argument); - factoryContext.ParamCheckExpressions.Add(checkRequiredStringParameterBlock); - return argument; - } - - // Allow nullable parameters that don't have a default value - if (nullability.ReadState != NullabilityState.NotNull && !parameter.HasDefaultValue) - { - if (parameter.ParameterType == typeof(StringValues?)) - { - return Expression.Block( - Expression.Condition(Expression.Equal(boundValueExpr, Expression.Convert(Expression.Constant(StringValues.Empty), parameter.ParameterType)), - Expression.Convert(Expression.Constant(null), parameter.ParameterType), - boundValueExpr - ) - ); - } - return boundValueExpr; - } - - return Expression.Block( - Expression.Condition(Expression.NotEqual(boundValueExpr, Expression.Constant(null)), - boundValueExpr, - Expression.Convert(Expression.Constant(parameter.DefaultValue), parameter.ParameterType))); + valueExpression = Expression.Convert(splitAndTrimExpr, parameter.ParameterType); } if (!isOptional) { - // The following is produced if the parameter is optional. + // The following is produced if the parameter is required: + // + // argument = value; + // if (argument == null) + // { + // wasParamCheckFailure = true; + // Log.RequiredParameterNotProvided(httpContext, "TypeOfValue", "parameterName", "source"); + // } var checkRequiredStringParameterBlock = Expression.Block( Expression.Assign(argument, valueExpression), Expression.IfThen(Expression.Equal(argument, Expression.Constant(null)), @@ -1956,6 +1923,12 @@ private static Expression BindParameterFromExpression( return valueExpression; } + // The following is produced if the parameter is optional. Note that we convert the + // default value to the target ParameterType to address scenarios where the user is + // setting null as the default value in a context where nullability is disabled. + // + // param1_local = value; + // param1_local != null ? param1_local : Convert(defaultValue, ParameterType) return Expression.Block( Expression.Condition(Expression.NotEqual(valueExpression, Expression.Constant(null)), valueExpression, @@ -2891,6 +2864,7 @@ private static string[] SplitAndTrim(string[] values) } var result = new List(); + Span parts = stackalloc Range[16]; foreach (var value in values) { @@ -2899,21 +2873,15 @@ private static string[] SplitAndTrim(string[] values) continue; } - var span = value.AsSpan(); - var start = 0; + var valueSpan = value.AsSpan(); + var length = valueSpan.Split(parts, ','); - for (var i = 0; i <= span.Length; i++) + for (int i = 0; i < length; i++) { - if (i == span.Length || span[i] == ',') + var part = valueSpan[parts[i]].Trim(); + if (!part.IsEmpty) { - var slice = span.Slice(start, i - start).Trim(); - - if (!slice.IsEmpty) - { - result.Add(new string(slice)); - } - - start = i + 1; + result.Add(new string(part)); } } } From f81960cda93fcd2479dfc11b36f1b8a00e50f07a Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Tue, 16 Jul 2024 12:44:31 +0200 Subject: [PATCH 16/17] Adding more test cases --- .../test/RequestDelegateFactoryTests.cs | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index fafd7e9c19ef..3fdd9cbe23a9 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -3151,11 +3151,11 @@ static void TestAction([AsParameters] ParameterListRequiredNullableStringFromDif } [Fact] - public async Task RequestDelegate_ShouldHandleSingleHeaderValue() + public async Task RequestDelegate_ShouldHandleCommaSeparatedHeaderValuesAsString() { // Arrange var httpContext = CreateHttpContext(); - httpContext.Request.Headers["TestHeader"] = "HeaderValue"; + httpContext.Request.Headers["TestHeader"] = "Value1,Value2"; var resultFactory = RequestDelegateFactory.Create((HttpContext httpContext, [FromHeader(Name = "TestHeader")] string headerValue) => { @@ -3169,15 +3169,15 @@ public async Task RequestDelegate_ShouldHandleSingleHeaderValue() // Assert Assert.True(httpContext.Items.ContainsKey("headerValue")); - Assert.Equal("HeaderValue", httpContext.Items["headerValue"]); + Assert.Equal("Value1,Value2", httpContext.Items["headerValue"]); } [Fact] - public async Task RequestDelegate_ShouldHandleCommaSeparatedHeaderValues() + public async Task RequestDelegate_ShouldHandleSingleHeaderValueAsStringArray() { // Arrange var httpContext = CreateHttpContext(); - httpContext.Request.Headers["TestHeader"] = "Value1,Value2,Value3"; + httpContext.Request.Headers["TestHeader"] = "Value1"; var resultFactory = RequestDelegateFactory.Create((HttpContext httpContext, [FromHeader(Name = "TestHeader")] string[] headerValues) => { @@ -3193,6 +3193,31 @@ public async Task RequestDelegate_ShouldHandleCommaSeparatedHeaderValues() Assert.True(httpContext.Items.ContainsKey("headerValues")); var headerValues = httpContext.Items["headerValues"] as string[]; Assert.NotNull(headerValues); + Assert.Single(headerValues); + Assert.Equal("Value1", headerValues[0]); + } + + [Fact] + public async Task RequestDelegate_ShouldHandleCommaSeparatedHeaderValuesAsIEnumerable() + { + // Arrange + var httpContext = CreateHttpContext(); + httpContext.Request.Headers["TestHeader"] = "Value1,Value2,Value3"; + + var resultFactory = RequestDelegateFactory.Create((HttpContext httpContext, [FromHeader(Name = "TestHeader")] IEnumerable headerValues) => + { + httpContext.Items["headerValues"] = headerValues; + }); + + var requestDelegate = resultFactory.RequestDelegate; + + // Act + await requestDelegate(httpContext); + + // Assert + Assert.True(httpContext.Items.ContainsKey("headerValues")); + var headerValues = httpContext.Items["headerValues"] as IEnumerable; + Assert.NotNull(headerValues); Assert.Equal(new[] { "Value1", "Value2", "Value3" }, headerValues); } From 7f8bca7b9dd6222458bb26db0ce205c78581da38 Mon Sep 17 00:00:00 2001 From: MattyLeslie Date: Tue, 16 Jul 2024 12:55:04 +0200 Subject: [PATCH 17/17] Updating RequestDelegateGenerator --- .../gen/RequestDelegateGenerator.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs b/src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs index ac8bbdd3a175..33aff42ae6c2 100644 --- a/src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs +++ b/src/Http/Http.Extensions/gen/RequestDelegateGenerator.cs @@ -103,6 +103,8 @@ public void Initialize(IncrementalGeneratorInitializationContext context) endpoint.EmitJsonPreparation(codeWriter); endpoint.EmitRouteOrQueryResolver(codeWriter); endpoint.EmitJsonBodyOrServiceResolver(codeWriter); + EmitHeaderValueParsing(codeWriter); + if (endpoint.NeedsParameterArray) { codeWriter.WriteLine("var parameters = del.Method.GetParameters();"); @@ -288,5 +290,25 @@ public void Initialize(IncrementalGeneratorInitializationContext context) context.AddSource("GeneratedRouteBuilderExtensions.g.cs", code); }); + + } + private static void EmitHeaderValueParsing(CodeWriter codeWriter) + { + codeWriter.WriteLine("var headerParameters = handler.Method.GetParameters().Where(p => p.GetCustomAttribute() != null).ToList();"); + codeWriter.WriteLine("foreach (var param in headerParameters)"); + codeWriter.StartBlock(); + codeWriter.WriteLine("var headerAttribute = param.GetCustomAttribute();"); + codeWriter.WriteLine("var headerName = headerAttribute?.Name ?? param.Name;"); + codeWriter.WriteLine("var headerValue = httpContext.Request.Headers[headerName];"); + codeWriter.WriteLine("if (param.ParameterType == typeof(string[]) || typeof(IEnumerable).IsAssignableFrom(param.ParameterType))"); + codeWriter.StartBlock(); + codeWriter.WriteLine("var parsedValues = headerValue.SelectMany(v => v.Split(',')).Select(v => v.Trim()).ToArray();"); + codeWriter.WriteLine("arguments[param.Position] = param.ParameterType == typeof(string[]) ? parsedValues : (object)parsedValues.AsEnumerable();"); + codeWriter.EndBlock(); + codeWriter.WriteLine("else if (param.ParameterType == typeof(string))"); + codeWriter.StartBlock(); + codeWriter.WriteLine("arguments[param.Position] = headerValue.ToString();"); + codeWriter.EndBlock(); + codeWriter.EndBlock(); } }