diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index 80edc7b2a3fd..57df73b27d27 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.Linq.Expressions; using System.Reflection; +using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Http.Extensions.Tests { @@ -274,6 +275,63 @@ public void FindBindAsyncMethod_FindsNonNullableReturningBindAsyncMethodGivenNul Assert.True(new ParameterBindingMethodCache().HasBindAsyncMethod(parameterInfo)); } + [Theory] + [InlineData(typeof(InvalidVoidReturnTryParseStruct))] + [InlineData(typeof(InvalidVoidReturnTryParseClass))] + [InlineData(typeof(InvalidWrongTypeTryParseStruct))] + [InlineData(typeof(InvalidWrongTypeTryParseClass))] + [InlineData(typeof(InvalidTryParseNullableStruct))] + [InlineData(typeof(InvalidTooFewArgsTryParseStruct))] + [InlineData(typeof(InvalidTooFewArgsTryParseClass))] + [InlineData(typeof(InvalidNonStaticTryParseStruct))] + [InlineData(typeof(InvalidNonStaticTryParseClass))] + public void FindTryParseMethod_ThrowsIfInvalidTryParseOnType(Type type) + { + var ex = Assert.Throws( + () => new ParameterBindingMethodCache().FindTryParseMethod(type)); + Assert.StartsWith($"TryParse method found on {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} with incorrect format. Must be a static method with format", ex.Message); + Assert.Contains($"bool TryParse(string, IFormatProvider, out {TypeNameHelper.GetTypeDisplayName(type, fullName: false)})", ex.Message); + Assert.Contains($"bool TryParse(string, out {TypeNameHelper.GetTypeDisplayName(type, fullName: false)})", ex.Message); + } + + [Theory] + [InlineData(typeof(TryParseClassWithGoodAndBad))] + [InlineData(typeof(TryParseStructWithGoodAndBad))] + public void FindTryParseMethod_IgnoresInvalidTryParseIfGoodOneFound(Type type) + { + var method = new ParameterBindingMethodCache().FindTryParseMethod(type); + Assert.NotNull(method); + } + + [Theory] + [InlineData(typeof(InvalidWrongReturnBindAsyncStruct))] + [InlineData(typeof(InvalidWrongReturnBindAsyncClass))] + [InlineData(typeof(InvalidWrongParamBindAsyncStruct))] + [InlineData(typeof(InvalidWrongParamBindAsyncClass))] + public void FindBindAsyncMethod_ThrowsIfInvalidBindAsyncOnType(Type type) + { + var cache = new ParameterBindingMethodCache(); + var parameter = new MockParameterInfo(type, "anything"); + var ex = Assert.Throws( + () => cache.FindBindAsyncMethod(parameter)); + Assert.StartsWith($"BindAsync method found on {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} with incorrect format. Must be a static method with format", ex.Message); + Assert.Contains($"ValueTask<{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}> BindAsync(HttpContext context, ParameterInfo parameter)", ex.Message); + Assert.Contains($"ValueTask<{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}> BindAsync(HttpContext context)", ex.Message); + Assert.Contains($"ValueTask<{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}?> BindAsync(HttpContext context, ParameterInfo parameter)", ex.Message); + Assert.Contains($"ValueTask<{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}?> BindAsync(HttpContext context)", ex.Message); + } + + [Theory] + [InlineData(typeof(BindAsyncStructWithGoodAndBad))] + [InlineData(typeof(BindAsyncClassWithGoodAndBad))] + public void FindBindAsyncMethod_IgnoresInvalidBindAsyncIfGoodOneFound(Type type) + { + var cache = new ParameterBindingMethodCache(); + var parameter = new MockParameterInfo(type, "anything"); + var (expression, _) = cache.FindBindAsyncMethod(parameter); + Assert.NotNull(expression); + } + enum Choice { One, @@ -292,8 +350,6 @@ private static void NullableReturningBindAsyncStructMethod(NullableReturningBind private static void BindAsyncSingleArgRecordMethod(BindAsyncSingleArgRecord arg) { } private static void BindAsyncSingleArgStructMethod(BindAsyncSingleArgStruct arg) { } - private static void BindAsyncNullableSingleArgStructMethod(BindAsyncSingleArgStruct? arg) { } - private static void NullableReturningBindAsyncSingleArgStructMethod(NullableReturningBindAsyncSingleArgStruct arg) { } private static ParameterInfo GetFirstParameter(Expression> expr) { @@ -331,6 +387,157 @@ public static bool TryParse(string? value, IFormatProvider formatProvider, out T } } + private record struct InvalidVoidReturnTryParseStruct(int Value) + { + public static void TryParse(string? value, IFormatProvider formatProvider, out InvalidVoidReturnTryParseStruct result) + { + if (!int.TryParse(value, NumberStyles.Integer, formatProvider, out var val)) + { + result = default; + return; + } + + result = new InvalidVoidReturnTryParseStruct(val); + return; + } + } + + private record struct InvalidWrongTypeTryParseStruct(int Value) + { + public static bool TryParse(string? value, IFormatProvider formatProvider, out InvalidVoidReturnTryParseStruct result) + { + if (!int.TryParse(value, NumberStyles.Integer, formatProvider, out var val)) + { + result = default; + return false; + } + + result = new InvalidVoidReturnTryParseStruct(val); + return true; + } + } + + private record struct InvalidTryParseNullableStruct(int Value) + { + public static bool TryParse(string? value, IFormatProvider formatProvider, out InvalidTryParseNullableStruct? result) + { + if (!int.TryParse(value, NumberStyles.Integer, formatProvider, out var val)) + { + result = default; + return false; + } + + result = new InvalidTryParseNullableStruct(val); + return true; + } + } + + private record struct InvalidTooFewArgsTryParseStruct(int Value) + { + public static bool TryParse(out InvalidTooFewArgsTryParseStruct result) + { + result = default; + return false; + } + } + + private struct TryParseStructWithGoodAndBad + { + public static bool TryParse(string? value, out TryParseStructWithGoodAndBad result) + { + result = new(); + return false; + } + + public static void TryParse(out TryParseStructWithGoodAndBad result) + { + result = new(); + } + } + + private record struct InvalidNonStaticTryParseStruct(int Value) + { + public bool TryParse(string? value, IFormatProvider formatProvider, out InvalidVoidReturnTryParseStruct result) + { + if (!int.TryParse(value, NumberStyles.Integer, formatProvider, out var val)) + { + result = default; + return false; + } + + result = new InvalidVoidReturnTryParseStruct(val); + return true; + } + } + + private class InvalidVoidReturnTryParseClass + { + public static void TryParse(string? value, IFormatProvider formatProvider, out InvalidVoidReturnTryParseClass result) + { + if (!int.TryParse(value, NumberStyles.Integer, formatProvider, out var val)) + { + result = new(); + return; + } + + result = new(); + } + } + + private class InvalidWrongTypeTryParseClass + { + public static bool TryParse(string? value, IFormatProvider formatProvider, out InvalidVoidReturnTryParseClass result) + { + if (!int.TryParse(value, NumberStyles.Integer, formatProvider, out var val)) + { + result = new(); + return false; + } + + result = new(); + return true; + } + } + + private class InvalidTooFewArgsTryParseClass + { + public static bool TryParse(out InvalidTooFewArgsTryParseClass result) + { + result = new(); + return false; + } + } + + private class TryParseClassWithGoodAndBad + { + public static bool TryParse(string? value, out TryParseClassWithGoodAndBad result) + { + result = new(); + return false; + } + + public static bool TryParse(out TryParseClassWithGoodAndBad result) + { + result = new(); + return false; + } + } + + private class InvalidNonStaticTryParseClass + { + public bool TryParse(string? value, IFormatProvider formatProvider, out InvalidNonStaticTryParseClass result) + { + if (!int.TryParse(value, NumberStyles.Integer, formatProvider, out var val)) + { + result = new(); + return false; + } + + result = new(); + return true; + } + } + private record BindAsyncRecord(int Value) { public static ValueTask BindAsync(HttpContext context, ParameterInfo parameter) @@ -395,9 +602,45 @@ public static ValueTask BindAsync(HttpContext context) } } - private record struct NullableReturningBindAsyncSingleArgStruct(int Value) + private record struct InvalidWrongReturnBindAsyncStruct(int Value) { - public static ValueTask BindAsync(HttpContext context, ParameterInfo parameter) => + public static Task BindAsync(HttpContext context, ParameterInfo parameter) => + throw new NotImplementedException(); + } + + private class InvalidWrongReturnBindAsyncClass + { + public static Task BindAsync(HttpContext context, ParameterInfo parameter) => + throw new NotImplementedException(); + } + + private record struct InvalidWrongParamBindAsyncStruct(int Value) + { + public static ValueTask BindAsync(ParameterInfo parameter) => + throw new NotImplementedException(); + } + + private class InvalidWrongParamBindAsyncClass + { + public static Task BindAsync(ParameterInfo parameter) => + throw new NotImplementedException(); + } + + private record struct BindAsyncStructWithGoodAndBad(int Value) + { + public static ValueTask BindAsync(HttpContext context, ParameterInfo parameter) => + throw new NotImplementedException(); + + public static ValueTask BindAsync(ParameterInfo parameter) => + throw new NotImplementedException(); + } + + private class BindAsyncClassWithGoodAndBad + { + public static ValueTask BindAsync(HttpContext context, ParameterInfo parameter) => + throw new NotImplementedException(); + + public static ValueTask BindAsync(ParameterInfo parameter) => throw new NotImplementedException(); } diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 63ae1fb94641..f98b7183c984 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -1538,6 +1538,51 @@ void TestBothInvalidAction(Todo value1, [FromBody] int value2) { } Assert.Throws(() => RequestDelegateFactory.Create(TestBothInvalidAction)); } + [Fact] + public void BuildRequestDelegateThrowsInvalidOperationExceptionForInvalidTryParse() + { + void TestTryParseStruct(BadTryParseStruct value1) { } + void TestTryParseClass(BadTryParseClass value1) { } + + Assert.Throws(() => RequestDelegateFactory.Create(TestTryParseStruct)); + Assert.Throws(() => RequestDelegateFactory.Create(TestTryParseClass)); + } + + private struct BadTryParseStruct + { + public static void TryParse(string? value, out BadTryParseStruct result) { } + } + + private class BadTryParseClass + { + public static void TryParse(string? value, out BadTryParseClass result) + { + result = new(); + } + } + + [Fact] + public void BuildRequestDelegateThrowsInvalidOperationExceptionForInvalidBindAsync() + { + void TestBindAsyncStruct(BadBindAsyncStruct value1) { } + void TestBindAsyncClass(BadBindAsyncClass value1) { } + + var ex = Assert.Throws(() => RequestDelegateFactory.Create(TestBindAsyncStruct)); + Assert.Throws(() => RequestDelegateFactory.Create(TestBindAsyncClass)); + } + + private struct BadBindAsyncStruct + { + public static Task BindAsync(HttpContext context, ParameterInfo parameter) => + throw new NotImplementedException(); + } + + private class BadBindAsyncClass + { + public static Task BindAsync(HttpContext context, ParameterInfo parameter) => + throw new NotImplementedException(); + } + public static object[][] ExplicitFromServiceActions { get diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index 1d605195c74a..1f7f304188f5 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -9,6 +9,8 @@ using System.Numerics; using System.Reflection; using System.Runtime.CompilerServices; +using System.Text; +using Microsoft.Extensions.Internal; #nullable enable @@ -106,7 +108,7 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) => methodInfo = type.GetMethod("TryParse", BindingFlags.Public | BindingFlags.Static, new[] { typeof(string), typeof(IFormatProvider), type.MakeByRefType() }); - if (methodInfo != null) + if (methodInfo is not null && methodInfo.ReturnType == typeof(bool)) { return (expression) => Expression.Call( methodInfo, @@ -117,11 +119,24 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) => methodInfo = type.GetMethod("TryParse", BindingFlags.Public | BindingFlags.Static, new[] { typeof(string), type.MakeByRefType() }); - if (methodInfo != null) + if (methodInfo is not null && methodInfo.ReturnType == typeof(bool)) { return (expression) => Expression.Call(methodInfo, TempSourceStringExpr, expression); } + if (type.GetMethod("TryParse", BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance) is MethodInfo invalidMethod) + { + var stringBuilder = new StringBuilder(); + stringBuilder.AppendLine(CultureInfo.InvariantCulture, $"TryParse method found on {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} with incorrect format. Must be a static method with format"); + stringBuilder.AppendLine(CultureInfo.InvariantCulture, $"bool TryParse(string, IFormatProvider, out {TypeNameHelper.GetTypeDisplayName(type, fullName: false)})"); + stringBuilder.AppendLine(CultureInfo.InvariantCulture, $"bool TryParse(string, out {TypeNameHelper.GetTypeDisplayName(type, fullName: false)})"); + stringBuilder.AppendLine("but found"); + stringBuilder.Append(invalidMethod.IsStatic ? "static " : "not-static "); + stringBuilder.Append(invalidMethod.ToString()); + + throw new InvalidOperationException(stringBuilder.ToString()); + } + return null; } @@ -192,6 +207,21 @@ public bool HasBindAsyncMethod(ParameterInfo parameter) => } } + if (nonNullableParameterType.GetMethod("BindAsync", BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance) is MethodInfo invalidBindMethod) + { + var stringBuilder = new StringBuilder(); + stringBuilder.AppendLine(CultureInfo.InvariantCulture, $"BindAsync method found on {TypeNameHelper.GetTypeDisplayName(nonNullableParameterType, fullName: false)} with incorrect format. Must be a static method with format"); + stringBuilder.AppendLine(CultureInfo.InvariantCulture, $"ValueTask<{TypeNameHelper.GetTypeDisplayName(nonNullableParameterType, fullName: false)}> BindAsync(HttpContext context, ParameterInfo parameter)"); + stringBuilder.AppendLine(CultureInfo.InvariantCulture, $"ValueTask<{TypeNameHelper.GetTypeDisplayName(nonNullableParameterType, fullName: false)}> BindAsync(HttpContext context)"); + stringBuilder.AppendLine(CultureInfo.InvariantCulture, $"ValueTask<{TypeNameHelper.GetTypeDisplayName(nonNullableParameterType, fullName: false)}?> BindAsync(HttpContext context, ParameterInfo parameter)"); + stringBuilder.AppendLine(CultureInfo.InvariantCulture, $"ValueTask<{TypeNameHelper.GetTypeDisplayName(nonNullableParameterType, fullName: false)}?> BindAsync(HttpContext context)"); + stringBuilder.AppendLine("but found"); + stringBuilder.Append(invalidBindMethod.IsStatic ? "static " : "not-static"); + stringBuilder.Append(invalidBindMethod.ToString()); + + throw new InvalidOperationException(stringBuilder.ToString()); + } + return (null, 0); }