diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs index 9e36505b390eac..04ab8e7ee08598 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs @@ -4,7 +4,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection.Metadata; -using System.Text; namespace System.Reflection.Metadata { @@ -12,8 +11,6 @@ internal struct TypeNameParseOptions { public TypeNameParseOptions() { } #pragma warning disable CA1822 // Mark members as static - // CoreLib does not enforce any limits - public bool IsMaxDepthExceeded(int _) => false; public int MaxNodes { get diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs index 8920e5a31c39fb..22ae86f08e6b53 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeName.cs @@ -301,24 +301,28 @@ public string Name /// public int GetNodeCount() { + // This method uses checked arithmetic to avoid silent overflows. + // It's impossible to parse a TypeName with NodeCount > int.MaxValue + // (TypeNameParseOptions.MaxNodes is an int), but it's possible + // to create such names with the Make* APIs. int result = 1; - if (IsNested) + if (IsArray || IsPointer || IsByRef) { - result += DeclaringType.GetNodeCount(); + result = checked(result + GetElementType().GetNodeCount()); } else if (IsConstructedGenericType) { - result++; - } - else if (IsArray || IsPointer || IsByRef) - { - result += GetElementType().GetNodeCount(); - } + result = checked(result + GetGenericTypeDefinition().GetNodeCount()); - foreach (TypeName genericArgument in GetGenericArguments()) + foreach (TypeName genericArgument in GetGenericArguments()) + { + result = checked(result + genericArgument.GetNodeCount()); + } + } + else if (IsNested) { - result += genericArgument.GetNodeCount(); + result = checked(result + DeclaringType.GetNodeCount()); } return result; diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParser.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParser.cs index e5acbf2acc44b5..97294c8014f2b7 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParser.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParser.cs @@ -49,7 +49,7 @@ private TypeNameParser(ReadOnlySpan name, bool throwOnError, TypeNameParse { if (throwOnError) { - if (parser._parseOptions.IsMaxDepthExceeded(recursiveDepth)) + if (IsMaxDepthExceeded(parser._parseOptions, recursiveDepth)) { ThrowInvalidOperation_MaxNodesExceeded(parser._parseOptions.MaxNodes); } @@ -61,19 +61,21 @@ private TypeNameParser(ReadOnlySpan name, bool throwOnError, TypeNameParse return null; } + Debug.Assert(parsedName.GetNodeCount() == recursiveDepth, $"Node count mismatch for '{typeName.ToString()}'"); + return parsedName; } // this method should return null instead of throwing, so the caller can get errorIndex and include it in error msg private TypeName? ParseNextTypeName(bool allowFullyQualifiedName, ref int recursiveDepth) { - if (!TryDive(ref recursiveDepth)) + if (!TryDive(_parseOptions, ref recursiveDepth)) { return null; } List? nestedNameLengths = null; - if (!TryGetTypeNameInfo(ref _inputString, ref nestedNameLengths, out int fullTypeNameLength)) + if (!TryGetTypeNameInfo(_parseOptions, ref _inputString, ref nestedNameLengths, ref recursiveDepth, out int fullTypeNameLength)) { return null; } @@ -147,6 +149,16 @@ private TypeNameParser(ReadOnlySpan name, bool throwOnError, TypeNameParse { _inputString = capturedBeforeProcessing; } + else + { + // Every constructed generic type needs the generic type definition. + if (!TryDive(_parseOptions, ref recursiveDepth)) + { + return null; + } + // If that generic type is a nested type, we don't increase the recursiveDepth any further, + // as generic type definition uses exactly the same declaring type as the constructed generic type. + } int previousDecorator = default; // capture the current state so we can reprocess it again once we know the AssemblyName @@ -154,7 +166,7 @@ private TypeNameParser(ReadOnlySpan name, bool throwOnError, TypeNameParse // iterate over the decorators to ensure there are no illegal combinations while (TryParseNextDecorator(ref _inputString, out int parsedDecorator)) { - if (!TryDive(ref recursiveDepth)) + if (!TryDive(_parseOptions, ref recursiveDepth)) { return null; } @@ -245,15 +257,5 @@ private bool TryParseAssemblyName(ref AssemblyNameInfo? assemblyName) return declaringType; } - - private bool TryDive(ref int depth) - { - if (_parseOptions.IsMaxDepthExceeded(depth)) - { - return false; - } - depth++; - return true; - } } } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs index b299a225673940..93859262eed99a 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs @@ -220,7 +220,8 @@ internal static bool IsBeginningOfGenericArgs(ref ReadOnlySpan span, out b return false; } - internal static bool TryGetTypeNameInfo(ref ReadOnlySpan input, ref List? nestedNameLengths, out int totalLength) + internal static bool TryGetTypeNameInfo(TypeNameParseOptions options, ref ReadOnlySpan input, + ref List? nestedNameLengths, ref int recursiveDepth, out int totalLength) { bool isNestedType; totalLength = 0; @@ -248,6 +249,11 @@ internal static bool TryGetTypeNameInfo(ref ReadOnlySpan input, ref List false; // CoreLib does not enforce any limits +#else + => depth > options.MaxNodes; +#endif + + internal static bool TryDive(TypeNameParseOptions options, ref int depth) + { + depth++; + return !IsMaxDepthExceeded(options, depth); + } + #if SYSTEM_REFLECTION_METADATA [DoesNotReturn] internal static void ThrowInvalidOperation_NotSimpleName(string fullName) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserOptions.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserOptions.cs index 551876e73147c9..b7420c40aa9eab 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserOptions.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserOptions.cs @@ -27,7 +27,5 @@ public int MaxNodes _maxNodes = value; } } - - internal bool IsMaxDepthExceeded(int depth) => depth >= _maxNodes; } } diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs index b9d23a2be15244..01ec45b50cc3e1 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameParserHelpersTests.cs @@ -122,7 +122,9 @@ public void TryGetTypeNameInfoGetsAllTheInfo(string input, bool expectedResult, { List? nestedNameLengths = null; ReadOnlySpan span = input.AsSpan(); - bool result = TypeNameParserHelpers.TryGetTypeNameInfo(ref span, ref nestedNameLengths, out int totalLength); + TypeNameParseOptions options = new(); + int recursiveDepth = 0; + bool result = TypeNameParserHelpers.TryGetTypeNameInfo(options, ref span, ref nestedNameLengths, ref recursiveDepth, out int totalLength); Assert.Equal(expectedResult, result); @@ -130,6 +132,7 @@ public void TryGetTypeNameInfoGetsAllTheInfo(string input, bool expectedResult, { Assert.Equal(expectedNestedNameLengths, nestedNameLengths?.ToArray()); Assert.Equal(expectedTotalLength, totalLength); + Assert.Equal(expectedNestedNameLengths?.Length ?? 0, recursiveDepth); } } diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs index 893c10b9125799..dcf46d54d2d958 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNameTests.cs @@ -121,119 +121,207 @@ static void Verify(Type type, AssemblyName expectedAssemblyName, TypeName parsed } } - [Theory] - [InlineData(10, "*")] // pointer to pointer - [InlineData(10, "[]")] // array of arrays - [InlineData(100, "*")] - [InlineData(100, "[]")] - public void MaxNodesIsRespected_TooManyDecorators(int maxDepth, string decorator) + private static void EnsureMaxNodesIsRespected(string typeName, int expectedNodeCount, Action validate) { - TypeNameParseOptions options = new() + // Specified MaxNodes is equal the actual node count + TypeNameParseOptions equal = new() { - MaxNodes = maxDepth + MaxNodes = expectedNodeCount }; - string notTooMany = $"System.Int32{string.Join("", Enumerable.Repeat(decorator, maxDepth - 1))}"; - string tooMany = $"System.Int32{string.Join("", Enumerable.Repeat(decorator, maxDepth))}"; + TypeName parsed = TypeName.Parse(typeName.AsSpan(), equal); + Assert.Equal(expectedNodeCount, parsed.GetNodeCount()); + validate(parsed); + Assert.True(TypeName.TryParse(typeName.AsSpan(), out parsed, equal)); + Assert.Equal(expectedNodeCount, parsed.GetNodeCount()); + validate(parsed); + + // Specified MaxNodes is less than the actual node count + TypeNameParseOptions less = new() + { + MaxNodes = expectedNodeCount - 1 + }; - Assert.Throws(() => TypeName.Parse(tooMany.AsSpan(), options)); - Assert.False(TypeName.TryParse(tooMany.AsSpan(), out _, options)); + Assert.Throws(() => TypeName.Parse(typeName.AsSpan(), less)); + Assert.False(TypeName.TryParse(typeName.AsSpan(), out _, less)); - TypeName parsed = TypeName.Parse(notTooMany.AsSpan(), options); - ValidateElementType(maxDepth, parsed, decorator); + // Specified MaxNodes is more than the actual node count + TypeNameParseOptions more = new() + { + MaxNodes = expectedNodeCount + 1 + }; - Assert.True(TypeName.TryParse(notTooMany.AsSpan(), out parsed, options)); - ValidateElementType(maxDepth, parsed, decorator); + parsed = TypeName.Parse(typeName.AsSpan(), more); + Assert.Equal(expectedNodeCount, parsed.GetNodeCount()); + validate(parsed); + Assert.True(TypeName.TryParse(typeName.AsSpan(), out parsed, more)); + Assert.Equal(expectedNodeCount, parsed.GetNodeCount()); + validate(parsed); + } - static void ValidateElementType(int maxDepth, TypeName parsed, string decorator) + [Theory] + [InlineData("*", 10)] // pointer to pointer + [InlineData("[]", 10)] // array of arrays + [InlineData("*", 100)] + [InlineData("[]", 100)] + public void MaxNodesIsRespected_Decorators(string decorator, int decoratorsCount) + { + string name = $"System.Int32{string.Join("", Enumerable.Repeat(decorator, decoratorsCount))}"; + + // Expected node count: + // - +1 for the simple type name (System.Int32) + // - +1 for every decorator + int expectedNodeCount = 1 + decoratorsCount; + + EnsureMaxNodesIsRespected(name, expectedNodeCount, parsed => { - for (int i = 0; i < maxDepth - 1; i++) + for (int i = 0; i < decoratorsCount; i++) { + Assert.Equal(expectedNodeCount - i, parsed.GetNodeCount()); + Assert.Equal(decorator == "*", parsed.IsPointer); Assert.Equal(decorator == "[]", parsed.IsSZArray); Assert.False(parsed.IsConstructedGenericType); + Assert.False(parsed.IsSimple); parsed = parsed.GetElementType(); } - } + }); } [Theory] [InlineData(10)] [InlineData(100)] - public void MaxNodesIsRespected_TooDeepGenerics(int maxDepth) + public void MaxNodesIsRespected_GenericsOfGenerics(int depth) { - TypeNameParseOptions options = new() + // MakeGenericType is not used here, as it crashes for larger depths + string coreLibName = typeof(object).Assembly.FullName; + string name = typeof(int).AssemblyQualifiedName!; + for (int i = 0; i < depth; i++) { - MaxNodes = maxDepth - }; - - string tooDeep = GetName(maxDepth); - string notTooDeep = GetName(maxDepth - 1); - - Assert.Throws(() => TypeName.Parse(tooDeep.AsSpan(), options)); - Assert.False(TypeName.TryParse(tooDeep.AsSpan(), out _, options)); - - TypeName parsed = TypeName.Parse(notTooDeep.AsSpan(), options); - Validate(maxDepth, parsed); - - Assert.True(TypeName.TryParse(notTooDeep.AsSpan(), out parsed, options)); - Validate(maxDepth, parsed); - - static string GetName(int depth) - { - // MakeGenericType is not used here, as it crashes for larger depths - string coreLibName = typeof(object).Assembly.FullName; - string fullName = typeof(int).AssemblyQualifiedName!; - for (int i = 0; i < depth; i++) - { - fullName = $"System.Collections.Generic.List`1[[{fullName}]], {coreLibName}"; - } - return fullName; + name = $"System.Collections.Generic.List`1[[{name}]], {coreLibName}"; } - static void Validate(int maxDepth, TypeName parsed) + // Expected node count: + // - +1 for the simple type name (System.Int32) + // - +2 * depth (+1 for the generic type definition and +1 for the constructed generic type) + int expectedNodeCount = 1 + 2 * depth; + + EnsureMaxNodesIsRespected(name, expectedNodeCount, parsed => { - for (int i = 0; i < maxDepth - 1; i++) + for (int i = 0; i < depth - 1; i++) { Assert.True(parsed.IsConstructedGenericType); parsed = parsed.GetGenericArguments()[0]; } - } + }); } [Theory] [InlineData(10)] [InlineData(100)] - public void MaxNodesIsRespected_TooManyGenericArguments(int maxDepth) + public void MaxNodesIsRespected_FlatGenericArguments(int genericArgsCount) { - TypeNameParseOptions options = new() + string name = $"Some.GenericType`{genericArgsCount}[{string.Join(",", Enumerable.Repeat("System.Int32", genericArgsCount))}]"; + + // Expected node count: + // - +1 for the constructed generic type itself + // - +1 * genericArgsCount (all arguments are simple) + // - +1 for generic type definition + int expectedNodeCount = 1 + genericArgsCount + 1; + + EnsureMaxNodesIsRespected(name, expectedNodeCount, parsed => { - MaxNodes = maxDepth - }; + Assert.True(parsed.IsConstructedGenericType); + ImmutableArray genericArgs = parsed.GetGenericArguments(); + Assert.Equal(genericArgsCount, genericArgs.Length); + Assert.All(genericArgs, arg => Assert.False(arg.IsConstructedGenericType)); + Assert.All(genericArgs, arg => Assert.Equal(1, arg.GetNodeCount())); + Assert.Equal(1, parsed.GetGenericTypeDefinition().GetNodeCount()); + Assert.Equal(expectedNodeCount, parsed.GetNodeCount()); + }); + } - string tooMany = GetName(maxDepth); - string notTooMany = GetName(maxDepth - 1); + [Theory] + [InlineData(10)] + [InlineData(100)] + public void MaxNodesIsRespected_NestedNames(int nestingDepth) + { + string name = $"Namespace.NotNestedType+{string.Join("+", Enumerable.Repeat("NestedType", nestingDepth))}"; - Assert.Throws(() => TypeName.Parse(tooMany.AsSpan(), options)); - Assert.False(TypeName.TryParse(tooMany.AsSpan(), out _, options)); + // Expected node count: + // - +1 for the nested type name itself + // - +1 * nestingDepth + int expectedNodeCount = 1 + nestingDepth; - TypeName parsed = TypeName.Parse(notTooMany.AsSpan(), options); - Validate(parsed, maxDepth); + EnsureMaxNodesIsRespected(name, expectedNodeCount, parsed => + { + Assert.True(parsed.IsNested); + Assert.Equal(expectedNodeCount, parsed.GetNodeCount()); - Assert.True(TypeName.TryParse(notTooMany.AsSpan(), out parsed, options)); - Validate(parsed, maxDepth); + int depth = nestingDepth; + while (parsed.IsNested) + { + parsed = parsed.DeclaringType; + Assert.Equal(depth, parsed.GetNodeCount()); + depth--; + } - static string GetName(int depth) - => $"Some.GenericType`{depth}[{string.Join(",", Enumerable.Repeat("System.Int32", depth))}]"; + Assert.False(parsed.IsNested); + Assert.Equal(1, parsed.GetNodeCount()); + Assert.Equal(0, depth); + }); + } - static void Validate(TypeName parsed, int maxDepth) + [Theory] + [InlineData("System.Int32*", 2)] // a pointer to a simple type + [InlineData("System.Int32[]*", 3)] // a pointer to an array of a simple type + [InlineData("System.Declaring+Nested", 2)] // a nested and declaring types + [InlineData("System.Declaring+Nested*", 3)] // a pointer to a nested type, which has the declaring type + // "Namespace.Declaring+NestedGeneric`2[A, B]" requires 5 TypeName instances: + // - constructed generic type: Namespace.Declaring+NestedGeneric`2[A, B] (+1) + // - declaring type: Namespace.Declaring (+1) + // - generic type definition: Namespace.Declaring+NestedGeneric`2 (+1) + // - declaring type is the same as of the constructed generic type (+0) + // - generic arguments: + // - generic arguments: + // - simple: A (+1) + // - simple: B (+1) + [InlineData("Namespace.Declaring+NestedGeneric`2[A, B]", 5)] + // A pointer to the above + [InlineData("Namespace.Declaring+NestedGeneric`2[A, B]*", 6)] + // A pointer to the array of the above + [InlineData("Namespace.Declaring+NestedGeneric`2[A, B][,]*", 7)] + // "Namespace.Declaring+NestedGeneric`1[AnotherGeneric`1[A]]" requires 6 TypeName instances: + // - constructed generic type: Namespace.Declaring+NestedGeneric`1[AnotherGeneric`1[A]] (+1) + // - declaring type: Namespace.Declaring (+1) + // - generic type definition: Namespace.Declaring+NestedGeneric`1 (+1) + // - declaring type is the same as of the constructed generic type (+0) + // - generic arguments: + // - constructed generic type: AnotherGeneric`1[A] (+1) + // - generic type definition: AnotherGeneric`1 (+1) + // - generic arguments: + // - simple: A (+1) + [InlineData("Namespace.Declaring+NestedGeneric`1[AnotherGeneric`1[A]]", 6)] + public void MaxNodesIsRespected(string typeName, int expectedNodeCount) + { + TypeNameParseOptions tooMany = new() { - Assert.True(parsed.IsConstructedGenericType); - ImmutableArray genericArgs = parsed.GetGenericArguments(); - Assert.Equal(maxDepth - 1, genericArgs.Length); - Assert.All(genericArgs, arg => Assert.False(arg.IsConstructedGenericType)); - } + MaxNodes = expectedNodeCount - 1 + }; + TypeNameParseOptions notTooMany = new() + { + MaxNodes = expectedNodeCount + }; + + Assert.Throws(() => TypeName.Parse(typeName.AsSpan(), tooMany)); + Assert.False(TypeName.TryParse(typeName.AsSpan(), out _, tooMany)); + + TypeName parsed = TypeName.Parse(typeName.AsSpan(), notTooMany); + Assert.Equal(expectedNodeCount, parsed.GetNodeCount()); + + Assert.True(TypeName.TryParse(typeName.AsSpan(), out parsed, notTooMany)); + Assert.Equal(expectedNodeCount, parsed.GetNodeCount()); } public static IEnumerable GenericArgumentsAreSupported_Arguments() @@ -471,10 +559,10 @@ public static IEnumerable GetAdditionalConstructedTypeData() [InlineData(typeof(int[,][]), 3)] [InlineData(typeof(Nullable<>), 1)] // open generic type treated as elemental [InlineData(typeof(NestedNonGeneric_0), 2)] // declaring and nested - [InlineData(typeof(NestedGeneric_0), 3)] // declaring, nested and generic arg + [InlineData(typeof(NestedGeneric_0), 4)] // declaring, nested, generic arg and generic type definition [InlineData(typeof(NestedNonGeneric_0.NestedNonGeneric_1), 3)] // declaring, nested 0 and nested 1 // TypeNameTests+NestedGeneric_0`1+NestedGeneric_1`2[[Int32],[String],[Boolean]] (simplified for brevity) - [InlineData(typeof(NestedGeneric_0.NestedGeneric_1), 6)] // declaring, nested 0 and nested 1 and 3 generic args + [InlineData(typeof(NestedGeneric_0.NestedGeneric_1), 7)] // declaring, nested 0 and nested 1 and 3 generic args and generic type definition [MemberData(nameof(GetAdditionalConstructedTypeData))] public void GetNodeCountReturnsExpectedValue(Type type, int expected) { @@ -493,6 +581,26 @@ public void GetNodeCountReturnsExpectedValue(Type type, int expected) } } + [OuterLoop("It takes a lot of time")] + [Fact] + public void GetNodeCountThrowsForInt32Overflow() + { + TypeName genericType = TypeName.Parse("Generic".AsSpan()); + TypeName genericArg = TypeName.Parse("Arg".AsSpan()); + // Don't allocate Array.MaxLength array as it may make this test flaky (frequent OOMs). + ImmutableArray.Builder genericArgs = ImmutableArray.CreateBuilder(initialCapacity: (int)Math.Sqrt(int.MaxValue)); + for (int i = 0; i < genericArgs.Capacity; ++i) + { + genericArgs.Add(genericArg); + } + TypeName constructedGenericType = genericType.MakeGenericTypeName(genericArgs.MoveToImmutable()); + // Just repeat the same reference to a large closed generic type multiple times. + // It's going to give us large node count without allocating too much. + TypeName largeNodeCount = genericType.MakeGenericTypeName(Enumerable.Repeat(constructedGenericType, (int)Math.Sqrt(int.MaxValue)).ToImmutableArray()); + + Assert.Throws(() => largeNodeCount.GetNodeCount()); + } + [Fact] public void MakeGenericTypeName() {