diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs index 0982628197ea6..dc940d2cb0787 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs @@ -552,7 +552,9 @@ private bool IsAssignableTo(ITypeSymbol source, ITypeSymbol dest) return conversion.IsReference && conversion.IsImplicit; } - private bool IsUnsupportedType(ITypeSymbol type) + private HashSet? _visitedTypes = new(SymbolEqualityComparer.Default); + + private bool IsUnsupportedType(ITypeSymbol type, HashSet? visitedTypes = null) { if (type.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T) { @@ -569,25 +571,55 @@ private bool IsUnsupportedType(ITypeSymbol type) return true; } - if (type is IArrayTypeSymbol arrayTypeSymbol) + if (visitedTypes?.Contains(type) is true) { - return arrayTypeSymbol.Rank > 1 || IsUnsupportedType(arrayTypeSymbol.ElementType); + // avoid infinite recursion in nested types like + // public record RecursiveType + // { + // public TreeElement? Tree { get; set; } + // } + // public sealed class TreeElement : Dictionary; + // + // return false for the second call. The type will continue be checked in the first call anyway. + return false; } - if (IsCollection(type)) + IArrayTypeSymbol? arrayTypeSymbol = type as IArrayTypeSymbol; + if (arrayTypeSymbol is null) { - INamedTypeSymbol collectionType = (INamedTypeSymbol)type; - - if (IsCandidateDictionary(collectionType, out ITypeSymbol? keyType, out ITypeSymbol? elementType)) - { - return IsUnsupportedType(keyType) || IsUnsupportedType(elementType); - } - else if (TryGetElementType(collectionType, out elementType)) + if (!IsCollection(type)) { - return IsUnsupportedType(elementType); + return false; } } + if (visitedTypes is null) + { + visitedTypes = _visitedTypes; + visitedTypes.Clear(); + } + + visitedTypes.Add(type); + + if (arrayTypeSymbol is not null) + { + return arrayTypeSymbol.Rank > 1 || IsUnsupportedType(arrayTypeSymbol.ElementType, visitedTypes); + } + + Debug.Assert(IsCollection(type)); + + INamedTypeSymbol collectionType = (INamedTypeSymbol)type; + + if (IsCandidateDictionary(collectionType, out ITypeSymbol? keyType, out ITypeSymbol? elementType)) + { + return IsUnsupportedType(keyType, visitedTypes) || IsUnsupportedType(elementType, visitedTypes); + } + + if (TryGetElementType(collectionType, out elementType)) + { + return IsUnsupportedType(elementType, visitedTypes); + } + return false; } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/BindingHelperInfo.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/BindingHelperInfo.cs index dfa3c5f169048..5cdc40e011517 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/BindingHelperInfo.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/BindingHelperInfo.cs @@ -131,6 +131,9 @@ bool TryRegisterCore() } case DictionarySpec dictionarySpec: { + // Base case to avoid stack overflow for recursive object graphs. + _seenTransitiveTypes.Add(typeRef, true); + bool shouldRegister = _typeIndex.CanBindTo(typeRef) && TryRegisterTransitiveTypesForMethodGen(dictionarySpec.KeyTypeRef) && TryRegisterTransitiveTypesForMethodGen(dictionarySpec.ElementTypeRef) && @@ -145,6 +148,9 @@ bool TryRegisterCore() } case CollectionSpec collectionSpec: { + // Base case to avoid stack overflow for recursive object graphs. + _seenTransitiveTypes.Add(typeRef, true); + if (_typeIndex.GetTypeSpec(collectionSpec.ElementTypeRef) is ComplexTypeSpec) { _namespaces.Add("System.Linq"); @@ -157,8 +163,7 @@ bool TryRegisterCore() { // Base case to avoid stack overflow for recursive object graphs. // Register all object types for gen; we need to throw runtime exceptions in some cases. - bool shouldRegister = true; - _seenTransitiveTypes.Add(typeRef, shouldRegister); + _seenTransitiveTypes.Add(typeRef, true); // List is used in generated code as a temp holder for formatting // an error for config properties that don't map to object properties. diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index 28209c1c5deb7..74de280474104 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs @@ -189,6 +189,15 @@ public string Color } } + public sealed class TreeElement : Dictionary; + + public record TypeWithRecursionThroughCollections + { + public TreeElement? Tree { get; set; } + public TreeElement?[]? Flat { get; set; } + public List? List { get; set; } + } + public record RecordWithArrayParameter(string[] Array); public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index 6ce085d5ec9cd..9e95c80643ed2 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -1544,6 +1544,87 @@ public void CanBindOnParametersAndProperties_RecordWithArrayConstructorParameter Assert.Equal(new string[] { "a", "b", "c" }, options.Array); } + /// + /// Test binding to recursive types using Dictionary or Collections. + /// This ensure no stack overflow will occur during the compilation through the source gen or at runtime. + /// + [Fact] + public void BindToRecursiveTypesTest() + { + string jsonConfig = @"{ + ""Tree"": { + ""Branch1"": { + ""Leaf1"": {}, + ""Leaf2"": {} + }, + ""Branch2"": { + ""Leaf3"": {} + } + }, + ""Flat"": [ + { + ""Element1"": { + ""SubElement1"": {} + } + }, + { + ""Element2"": { + ""SubElement2"": {} + } + }, + { + ""Element3"": {} + } + ], + ""List"": [ + { + ""Item1"": { + ""NestedItem1"": {} + } + }, + { + ""Item2"": {} + }, + ] + }"; + + var configuration = new ConfigurationBuilder() + .AddJsonStream(new System.IO.MemoryStream(System.Text.Encoding.UTF8.GetBytes(jsonConfig))) + .Build(); + + var instance = new TypeWithRecursionThroughCollections(); + configuration.Bind(instance); + + // Validate the dictionary + Assert.NotNull(instance.Tree); + Assert.Equal(2, instance.Tree.Count); + Assert.NotNull(instance.Tree["Branch1"]); + Assert.Equal(2, instance.Tree["Branch1"].Count); + Assert.Equal(["Leaf1", "Leaf2"], instance.Tree["Branch1"].Keys); + Assert.Equal(["Leaf3"], instance.Tree["Branch2"].Keys); + + // Validate the array + Assert.NotNull(instance.Flat); + Assert.Equal(3, instance.Flat.Length); + Assert.Equal(["Element1"], instance.Flat[0].Keys); + Assert.Equal(["Element2"], instance.Flat[1].Keys); + Assert.Equal(["Element3"], instance.Flat[2].Keys); + Assert.Equal(1, instance.Flat[0].Values.Count); + Assert.Equal(["SubElement1"], instance.Flat[0].Values.ToArray()[0].Keys); + Assert.Equal(1, instance.Flat[1].Values.Count); + Assert.Equal(["SubElement2"], instance.Flat[1].Values.ToArray()[0].Keys); + Assert.Equal(1, instance.Flat[2].Values.Count); + + // Validate the List + Assert.NotNull(instance.Flat); + Assert.Equal(2, instance.List.Count); + Assert.Equal(["Item1"], instance.List[0].Keys); + Assert.Equal(["Item2"], instance.List[1].Keys); + Assert.Equal(1, instance.List[0].Values.Count); + Assert.Equal(["NestedItem1"], instance.List[0].Values.ToArray()[0].Keys); + Assert.Equal(1, instance.List[1].Values.Count); + } + [Fact] public void CanBindReadonlyRecordStructOptions() {