Skip to content

Commit

Permalink
[TypeName] Nested types should respect MaxNode count (dotnet#106334)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamsitnik committed Sep 9, 2024
1 parent d109cff commit 8f9f431
Show file tree
Hide file tree
Showing 7 changed files with 233 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,13 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection.Metadata;
using System.Text;

namespace System.Reflection.Metadata
{
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,24 +301,28 @@ public string Name
/// </remarks>
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private TypeNameParser(ReadOnlySpan<char> name, bool throwOnError, TypeNameParse
{
if (throwOnError)
{
if (parser._parseOptions.IsMaxDepthExceeded(recursiveDepth))
if (IsMaxDepthExceeded(parser._parseOptions, recursiveDepth))
{
ThrowInvalidOperation_MaxNodesExceeded(parser._parseOptions.MaxNodes);
}
Expand All @@ -61,19 +61,21 @@ private TypeNameParser(ReadOnlySpan<char> 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<int>? 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;
}
Expand Down Expand Up @@ -147,14 +149,24 @@ private TypeNameParser(ReadOnlySpan<char> 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
capturedBeforeProcessing = _inputString;
// 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;
}
Expand Down Expand Up @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ internal static bool IsBeginningOfGenericArgs(ref ReadOnlySpan<char> span, out b
return false;
}

internal static bool TryGetTypeNameInfo(ref ReadOnlySpan<char> input, ref List<int>? nestedNameLengths, out int totalLength)
internal static bool TryGetTypeNameInfo(TypeNameParseOptions options, ref ReadOnlySpan<char> input,
ref List<int>? nestedNameLengths, ref int recursiveDepth, out int totalLength)
{
bool isNestedType;
totalLength = 0;
Expand Down Expand Up @@ -248,6 +249,11 @@ internal static bool TryGetTypeNameInfo(ref ReadOnlySpan<char> input, ref List<i
#endif
if (isNestedType)
{
if (!TryDive(options, ref recursiveDepth))
{
return false;
}

(nestedNameLengths ??= new()).Add(length);
totalLength += 1; // skip the '+' sign in next search
}
Expand Down Expand Up @@ -389,6 +395,19 @@ internal static void ThrowInvalidOperation_HasToBeArrayClass()
#endif
}

internal static bool IsMaxDepthExceeded(TypeNameParseOptions options, int depth)
#if SYSTEM_PRIVATE_CORELIB
=> 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,5 @@ public int MaxNodes
_maxNodes = value;
}
}

internal bool IsMaxDepthExceeded(int depth) => depth >= _maxNodes;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,17 @@ public void TryGetTypeNameInfoGetsAllTheInfo(string input, bool expectedResult,
{
List<int>? nestedNameLengths = null;
ReadOnlySpan<char> 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);

if (expectedResult)
{
Assert.Equal(expectedNestedNameLengths, nestedNameLengths?.ToArray());
Assert.Equal(expectedTotalLength, totalLength);
Assert.Equal(expectedNestedNameLengths?.Length ?? 0, recursiveDepth);
}
}

Expand Down
Loading

0 comments on commit 8f9f431

Please sign in to comment.