CSHARP-6040: Fix StackOverflowException in convention CouldApply on self-referencing IEnumerable<T> types (#2013)

Author: papafe

src/MongoDB.Bson/Serialization/Conventions/EnumRepresentationConvention.cs

@@ -90,14 +90,27 @@ IBsonSerializer Reconfigure(IBsonSerializer s)
                 => s.ValueType.IsEnum ? (s as IRepresentationConfigurable)?.WithRepresentation(_representation) : null;
 
             bool CouldApply(Type type)
-                => type.IsEnum ||
-                   type.IsNullableEnum() ||
-                   (type.IsArray && CouldApply(type.GetElementType())) || // This is covered by IEnumerable<T>, but it's a good short-circuit
-                   type.GetInterfaces().Prepend(type).Any(
-                       i =>
-                           i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IEnumerable<>) && CouldApply(i.GetGenericArguments()[0]) || // IEnumerable<T>
-                           i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IDictionary<,>) && (CouldApply(i.GetGenericArguments()[0]) || CouldApply(i.GetGenericArguments()[1])) // IDictionary<TKey, TValue>
-                   );
+            {
+                var visited = new HashSet<Type>();
+                return CouldApplyCore(type);
+
+                bool CouldApplyCore(Type t)
+                {
+                    if (!visited.Add(t))
+                    {
+                        return false;
+                    }
+
+                    return t.IsEnum ||
+                           t.IsNullableEnum() ||
+                           (t.IsArray && CouldApplyCore(t.GetElementType())) || // This is covered by IEnumerable<T>, but it's a good short-circuit
+                           t.GetInterfaces().Prepend(t).Any(
+                               i =>
+                                   i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IEnumerable<>) && CouldApplyCore(i.GetGenericArguments()[0]) || // IEnumerable<T>
+                                   i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IDictionary<,>) && (CouldApplyCore(i.GetGenericArguments()[0]) || CouldApplyCore(i.GetGenericArguments()[1])) // IDictionary<TKey, TValue>
+                           );
+                }
+            }
         }
 
         // private methods

src/MongoDB.Bson/Serialization/Conventions/ObjectSerializerAllowedTypesConvention.cs

@@ -170,14 +170,27 @@ public void Apply(BsonMemberMap memberMap)
             }
 
             bool CouldApply(Type type)
-                => type == typeof(object) ||
-                   (type.IsArray && CouldApply(type.GetElementType())) || // This is covered by IEnumerable<T>, but it's a good short-circuit
-                   IsNonGenericEnumerable(type) ||
-                   type.GetInterfaces().Prepend(type).Any(
-                       i =>
-                           i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IEnumerable<>) && CouldApply(i.GetGenericArguments()[0]) || // IEnumerable<T>
-                           i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IDictionary<,>) && (CouldApply(i.GetGenericArguments()[0]) || CouldApply(i.GetGenericArguments()[1])) // IDictionary<TKey, TValue>
-                   );
+            {
+                var visited = new HashSet<Type>();
+                return CouldApplyCore(type);
+
+                bool CouldApplyCore(Type t)
+                {
+                    if (!visited.Add(t))
+                    {
+                        return false;
+                    }
+
+                    return t == typeof(object) ||
+                           (t.IsArray && CouldApplyCore(t.GetElementType())) || // This is covered by IEnumerable<T>, but it's a good short-circuit
+                           IsNonGenericEnumerable(t) ||
+                           t.GetInterfaces().Prepend(t).Any(
+                               i =>
+                                   i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IEnumerable<>) && CouldApplyCore(i.GetGenericArguments()[0]) || // IEnumerable<T>
+                                   i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IDictionary<,>) && (CouldApplyCore(i.GetGenericArguments()[0]) || CouldApplyCore(i.GetGenericArguments()[1])) // IDictionary<TKey, TValue>
+                           );
+                }
+            }
 
             // A type that implements IEnumerable but not IEnumerable<T> (e.g. ArrayList, Hashtable).
             static bool IsNonGenericEnumerable(Type type)

tests/MongoDB.Bson.Tests/Serialization/Conventions/EnumRepresentationConventionTests.cs

@@ -14,6 +14,7 @@
 */
 
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.Linq.Expressions;
 using FluentAssertions;
@@ -52,6 +53,7 @@ public class C
             public IEnumerable<E> IEnumerableEnum { get; set; }
             public IDictionary<string, E> IDictionaryEnumValue { get; set; }
             public IDictionary<E, string> IDictionaryEnumKey { get; set; }
+            public SelfEnumerable SelfEnumerableProp { get; set; }
             // public Dictionary<E, C> RecursiveDictionary { get; set; } - this is not supported.
         }
 
@@ -60,6 +62,12 @@ public class IC
             public C C { get; set; }
         }
 
+        public class SelfEnumerable : IEnumerable<SelfEnumerable>
+        {
+            public IEnumerator<SelfEnumerable> GetEnumerator() => throw new NotImplementedException();
+            IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+        }
+
         [Theory]
         [InlineData(BsonType.Int32)]
         [InlineData(BsonType.Int64)]
@@ -295,6 +303,17 @@ public void Convention_should_work_with_recursive_type_when_top_level_is_false()
             ConventionRegistry.Remove("enumRecursive");
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void Convention_should_not_stack_overflow_with_self_referencing_enumerable_type(bool topLevelOnly)
+        {
+            var subject = new EnumRepresentationConvention(BsonType.String, topLevelOnly);
+            var memberMap = CreateMemberMap(c => c.SelfEnumerableProp);
+
+            subject.Apply(memberMap);
+        }
+
         [Theory]
         [InlineData((BsonType)0)]
         [InlineData(BsonType.Int32)]

tests/MongoDB.Bson.Tests/Serialization/Conventions/ObjectSerializerAllowedTypesConventionTests.cs

@@ -49,6 +49,7 @@ private class TestClass
             public IDictionary<string, object> IDictionaryStringObjectProp { get; set; }
             public IDictionary<object, string> IDictionaryObjectStringProp { get; set; }
             public ArrayList ArrayListProp { get; set; }
+            public SelfEnumerable SelfEnumerableProp { get; set; }
             // public Dictionary<TestClass, object> RecursivePropDictionary { get; set; } - this is not supported.
         }
 
@@ -57,6 +58,12 @@ private class IC
             public TestClass TestClass { get; set; }
         }
 
+        private class SelfEnumerable : IEnumerable<SelfEnumerable>
+        {
+            public IEnumerator<SelfEnumerable> GetEnumerator() => throw new NotImplementedException();
+            IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+        }
+
         [Fact]
         public void Apply_should_configure_serializer_when_building_with_constructor_single_delegate()
         {
@@ -508,6 +515,15 @@ public void Convention_should_work_with_recursive_type()
             ConventionRegistry.Remove("objectRecursive");
         }
 
+        [Fact]
+        public void Convention_should_not_stack_overflow_with_self_referencing_enumerable_type()
+        {
+            var subject = new ObjectSerializerAllowedTypesConvention();
+            var memberMap = CreateMemberMap(c => c.SelfEnumerableProp);
+
+            subject.Apply(memberMap);
+        }
+
         // private methods
         private static BsonMemberMap CreateMemberMap<TMember>(Expression<Func<TestClass, TMember>> member)
         {