Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public static class RequestDelegateFactory
private static readonly ParameterExpression HttpContextExpr = Expression.Parameter(typeof(HttpContext), "httpContext");
private static readonly ParameterExpression BodyValueExpr = Expression.Parameter(typeof(object), "bodyValue");
private static readonly ParameterExpression WasTryParseFailureExpr = Expression.Variable(typeof(bool), "wasTryParseFailure");
private static readonly ParameterExpression TempSourceStringExpr = Expression.Variable(typeof(string), "tempSourceString");
private static readonly ParameterExpression TempSourceStringExpr = TryParseMethodCache.TempSourceStringExpr;

private static readonly MemberExpression RequestServicesExpr = Expression.Property(HttpContextExpr, nameof(HttpContext.RequestServices));
private static readonly MemberExpression HttpRequestExpr = Expression.Property(HttpContextExpr, nameof(HttpContext.Request));
Expand Down Expand Up @@ -499,9 +499,9 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres
var isNotNullable = underlyingNullableType is null;

var nonNullableParameterType = underlyingNullableType ?? parameter.ParameterType;
var tryParseMethod = TryParseMethodCache.FindTryParseMethod(nonNullableParameterType);
var tryParseMethodCall = TryParseMethodCache.FindTryParseMethod(nonNullableParameterType);

if (tryParseMethod is null)
if (tryParseMethodCall is null)
{
throw new InvalidOperationException($"No public static bool {parameter.ParameterType.Name}.TryParse(string, out {parameter.ParameterType.Name}) method found for {parameter.Name}.");
}
Expand Down Expand Up @@ -556,7 +556,7 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres
Expression.Call(LogParameterBindingFailureMethod,
HttpContextExpr, parameterTypeNameConstant, parameterNameConstant, TempSourceStringExpr));

var tryParseCall = Expression.Call(tryParseMethod, TempSourceStringExpr, parsedValue);
var tryParseCall = tryParseMethodCall(parsedValue);

// If the parameter is nullable, we need to assign the "parsedValue" local to the nullable parameter on success.
Expression tryParseExpression = isNotNullable ?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using System.Text.Json.Serialization;
using System.Threading;
using System.Threading.Tasks;

using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Http.Json;
Expand Down
110 changes: 110 additions & 0 deletions src/Http/Http.Extensions/test/TryParseMethodCacheTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#nullable enable

using System;
using System.Globalization;
using System.Linq.Expressions;

using Xunit;

namespace Microsoft.AspNetCore.Http.Extensions.Tests
{
public class TryParseMethodCacheTests
{
[Theory]
[InlineData(typeof(int))]
[InlineData(typeof(double))]
[InlineData(typeof(float))]
[InlineData(typeof(Half))]
[InlineData(typeof(short))]
[InlineData(typeof(long))]
[InlineData(typeof(IntPtr))]
[InlineData(typeof(sbyte))]
[InlineData(typeof(ushort))]
[InlineData(typeof(uint))]
[InlineData(typeof(ulong))]
public void FindTryParseMethod_ReturnsTheExpectedTryParseMethodWithInvariantCulture(Type @type)
{
var methodFound = TryParseMethodCache.FindTryParseMethod(@type);

Assert.NotNull(methodFound);

var call = methodFound!(Expression.Variable(type, "parsedValue"));
var parameters = call.Method.GetParameters();

Assert.Equal(4, parameters.Length);
Assert.Equal(typeof(string), parameters[0].ParameterType);
Assert.Equal(typeof(NumberStyles), parameters[1].ParameterType);
Assert.Equal(typeof(IFormatProvider), parameters[2].ParameterType);
Assert.True(parameters[3].IsOut);
}

[Theory]
[InlineData(typeof(DateTime))]
[InlineData(typeof(DateOnly))]
[InlineData(typeof(DateTimeOffset))]
[InlineData(typeof(TimeOnly))]
[InlineData(typeof(TimeSpan))]
public void FindTryParseMethod_ReturnsTheExpectedTryParseMethodWithInvariantCultureDateType(Type @type)
{
var methodFound = TryParseMethodCache.FindTryParseMethod(@type);

Assert.NotNull(methodFound);

var call = methodFound!(Expression.Variable(type, "parsedValue"));
var parameters = call.Method.GetParameters();

if (@type == typeof(TimeSpan))
{
Assert.Equal(3, parameters.Length);
Assert.Equal(typeof(string), parameters[0].ParameterType);
Assert.Equal(typeof(IFormatProvider), parameters[1].ParameterType);
Assert.True(parameters[2].IsOut);
}
else
{
Assert.Equal(4, parameters.Length);
Assert.Equal(typeof(string), parameters[0].ParameterType);
Assert.Equal(typeof(IFormatProvider), parameters[1].ParameterType);
Assert.Equal(typeof(DateTimeStyles), parameters[2].ParameterType);
Assert.True(parameters[3].IsOut);
}
}

[Theory]
[InlineData(typeof(TryParsableInvariantRecord))]
public void FindTryParseMethod_ReturnsTheExpectedTryParseMethodWithInvariantCultureCustomType(Type @type)
{
var methodFound = TryParseMethodCache.FindTryParseMethod(@type);

Assert.NotNull(methodFound);

var call = methodFound!(Expression.Variable(type, "parsedValue"));
var parameters = call.Method.GetParameters();

Assert.Equal(3, parameters.Length);
Assert.Equal(typeof(string), parameters[0].ParameterType);
Assert.Equal(typeof(IFormatProvider), parameters[1].ParameterType);
Assert.True(parameters[2].IsOut);
Assert.True(((call.Arguments[1] as ConstantExpression)!.Value as CultureInfo)!.Equals(CultureInfo.InvariantCulture));
}

private record TryParsableInvariantRecord(int value)
{
public static bool TryParse(string? value, IFormatProvider formatProvider, out TryParsableInvariantRecord? result)
{
if (!int.TryParse(value, NumberStyles.Integer, formatProvider, out var val))
{
result = null;
return false;
}

result = new TryParsableInvariantRecord(val);
return true;
}
}

}
}
165 changes: 145 additions & 20 deletions src/Shared/TryParseMethodCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,57 +4,90 @@
using System;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq;
using System.Linq.Expressions;
using System.Numerics;
using System.Reflection;

#nullable enable

namespace Microsoft.AspNetCore.Http
{
internal static class TryParseMethodCache
{
private static readonly MethodInfo EnumTryParseMethod = GetEnumTryParseMethod();

// Since this is shared source, the cache won't be shared between RequestDelegateFactory and the ApiDescriptionProvider sadly :(
private static readonly ConcurrentDictionary<Type, MethodInfo?> Cache = new();
private static readonly ConcurrentDictionary<Type, Func<Expression, MethodCallExpression>?> MethodCallCache = new();
internal static readonly ParameterExpression TempSourceStringExpr = Expression.Variable(typeof(string), "tempSourceString");

public static bool HasTryParseMethod(ParameterInfo parameter)
{
var nonNullableParameterType = Nullable.GetUnderlyingType(parameter.ParameterType) ?? parameter.ParameterType;
return FindTryParseMethod(nonNullableParameterType) is not null;
}

// TODO: Use InvariantCulture where possible? Or is CurrentCulture fine because it's more flexible?
public static MethodInfo? FindTryParseMethod(Type type)
public static Func<Expression, MethodCallExpression>? FindTryParseMethod(Type type)
{
static MethodInfo? Finder(Type type)
static Func<Expression, MethodCallExpression>? Finder(Type type)
{
MethodInfo? methodInfo;

if (type.IsEnum)
{
return EnumTryParseMethod.MakeGenericMethod(type);
methodInfo = EnumTryParseMethod.MakeGenericMethod(type);
if (methodInfo != null)
{
return (expression) => Expression.Call(methodInfo!, TempSourceStringExpr, expression);
}

return null;
}

var staticMethods = type.GetMethods(BindingFlags.Public | BindingFlags.Static);
if (TryGetDateTimeTryParseMethod(type, out methodInfo))
{
return (expression) => Expression.Call(
methodInfo!,
TempSourceStringExpr,
Expression.Constant(CultureInfo.InvariantCulture),
Expression.Constant(DateTimeStyles.None),
expression);
}

foreach (var method in staticMethods)
if (TryGetNumberStylesTryGetMethod(type, out methodInfo, out var numberStyle))
{
if (method.Name != "TryParse" || method.ReturnType != typeof(bool))
{
continue;
}
return (expression) => Expression.Call(
methodInfo!,
TempSourceStringExpr,
Expression.Constant(numberStyle),
Expression.Constant(CultureInfo.InvariantCulture),
expression);
}

var tryParseParameters = method.GetParameters();
methodInfo = type.GetMethod("TryParse", BindingFlags.Public | BindingFlags.Static, new[] { typeof(string), typeof(IFormatProvider), type.MakeByRefType() });
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a test with a custom type that implements this signature and verify it actually gets called with CultureInfo.InvariantCulture?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure ! 🙂


if (tryParseParameters.Length == 2 &&
tryParseParameters[0].ParameterType == typeof(string) &&
tryParseParameters[1].IsOut &&
tryParseParameters[1].ParameterType == type.MakeByRefType())
{
return method;
}
if (methodInfo != null)
{
return (expression) => Expression.Call(
methodInfo,
TempSourceStringExpr,
Expression.Constant(CultureInfo.InvariantCulture),
expression);
}

methodInfo = type.GetMethod("TryParse", BindingFlags.Public | BindingFlags.Static, new[] { typeof(string), type.MakeByRefType() });

if (methodInfo != null)
{
return (expression) => Expression.Call(methodInfo, TempSourceStringExpr, expression);
}

return null;
}

return Cache.GetOrAdd(type, Finder);
return MethodCallCache.GetOrAdd(type, Finder);
}

private static MethodInfo GetEnumTryParseMethod()
Expand All @@ -81,5 +114,97 @@ private static MethodInfo GetEnumTryParseMethod()
Debug.Fail("static bool System.Enum.TryParse<TEnum>(string? value, out TEnum result) not found.");
throw new Exception("static bool System.Enum.TryParse<TEnum>(string? value, out TEnum result) not found.");
}

private static bool TryGetDateTimeTryParseMethod(Type type, [NotNullWhen(true)] out MethodInfo? methodInfo)
{
methodInfo = null;
if (type != typeof(DateTime) && type != typeof(DateOnly) &&
type != typeof(DateTimeOffset) && type != typeof(TimeOnly))
{
return false;
}

var staticTryParseDateMethod = type.GetMethod(
"TryParse",
BindingFlags.Public | BindingFlags.Static,
new[] { typeof(string), typeof(IFormatProvider), typeof(DateTimeStyles), type.MakeByRefType() });

methodInfo = staticTryParseDateMethod;

return methodInfo != null;
}

private static bool TryGetNumberStylesTryGetMethod(Type type, [NotNullWhen(true)] out MethodInfo? method, [NotNullWhen(true)] out NumberStyles? numberStyles)
{
method = null;
numberStyles = null;

if (!UseTryParseWithNumberStyleOption(type))
{
return false;
}

var staticMethods = type.GetMethods(BindingFlags.Public | BindingFlags.Static)
.Where(m => m.Name == "TryParse" && m.ReturnType == typeof(bool))
.OrderByDescending(m => m.GetParameters().Length);

var numberStylesToUse = NumberStyles.Integer;
var methodToUse = default(MethodInfo);

foreach (var methodInfo in staticMethods)
{
var tryParseParameters = methodInfo.GetParameters();

if (tryParseParameters.Length == 4 &&
tryParseParameters[0].ParameterType == typeof(string) &&
tryParseParameters[1].ParameterType == typeof(NumberStyles) &&
tryParseParameters[2].ParameterType == typeof(IFormatProvider) &&
tryParseParameters[3].IsOut &&
tryParseParameters[3].ParameterType == type.MakeByRefType())
{
if (type == typeof(int) || type == typeof(short) || type == typeof(IntPtr) ||
type == typeof(long) || type == typeof(byte) || type == typeof(sbyte) ||
type == typeof(ushort) || type == typeof(uint) || type == typeof(ulong) ||
type == typeof(BigInteger))
{
numberStylesToUse = NumberStyles.Integer;
}

if (type == typeof(double) || type == typeof(float) || type == typeof(Half))
{
numberStylesToUse = NumberStyles.AllowThousands | NumberStyles.Float;
}

if (type == typeof(decimal))
{
numberStylesToUse = NumberStyles.Number;
}

methodToUse = methodInfo!;
break;
}
}

numberStyles = numberStylesToUse!;
method = methodToUse!;

return true;
}

internal static bool UseTryParseWithNumberStyleOption(Type type)
=> type == typeof(int) ||
type == typeof(double) ||
type == typeof(decimal) ||
type == typeof(float) ||
type == typeof(Half) ||
type == typeof(short) ||
type == typeof(long) ||
type == typeof(IntPtr) ||
type == typeof(byte) ||
type == typeof(sbyte) ||
type == typeof(ushort) ||
type == typeof(uint) ||
type == typeof(ulong) ||
type == typeof(BigInteger);
}
}