Avoid StackOverflowException when cyclic enumerable Type (dotnet#37818)

kreuwont · May 21, 2019 · 857d1d1 · 857d1d1
1 parent 76d1a05
commit 857d1d1
Show file tree

Hide file tree

Showing 14 changed files with 142 additions and 79 deletions.
diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs
@@ -20,6 +20,11 @@ internal abstract class JsonPropertyInfo
         private static readonly JsonEnumerableConverter s_jsonIEnumerableConstuctibleConverter = new DefaultIEnumerableConstructibleConverter();
         private static readonly JsonEnumerableConverter s_jsonImmutableConverter = new DefaultImmutableConverter();
 
+        private JsonClassInfo _runtimeClassInfo;
+
+        private Type _elementType;
+        private JsonClassInfo _elementClassInfo;
+
         public static readonly JsonPropertyInfo s_missingProperty = new JsonPropertyInfoNotNullable<object, object, object>();
 
         public ClassType ClassType;
@@ -43,8 +48,38 @@ internal abstract class JsonPropertyInfo
         public bool IsPropertyPolicy {get; protected set;}
         public bool IgnoreNullValues { get; private set; }
 
-        // todo: to minimize hashtable lookups, cache JsonClassInfo:
-        //public JsonClassInfo ClassInfo;
+        // Options can be referenced here since all JsonPropertyInfos originate from a JsonClassInfo that is cached on JsonSerializerOptions.
+        protected JsonSerializerOptions Options { get; set; }
+
+        public JsonClassInfo RuntimeClassInfo
+        {
+            get
+            {
+                if (_runtimeClassInfo == null)
+                {
+                    _runtimeClassInfo = Options.GetOrAddClass(RuntimePropertyType);
+                }
+
+                return _runtimeClassInfo;
+            }
+        }
+
+        /// <summary>
+        /// Return the JsonClassInfo for the element type, or null if the the property is not an enumerable or dictionary.
+        /// </summary>
+        public JsonClassInfo ElementClassInfo
+        {
+            get
+            {
+                if (_elementClassInfo == null && _elementType != null)
+                {
+                    Debug.Assert(ClassType == ClassType.Enumerable || ClassType == ClassType.Dictionary);
+                    _elementClassInfo = Options.GetOrAddClass(_elementType);
+                }
+
+                return _elementClassInfo;
+            }
+        }
 
         public virtual void Initialize(
             Type parentClassType,
@@ -59,18 +94,13 @@ public virtual void Initialize(
             RuntimePropertyType = runtimePropertyType;
             PropertyInfo = propertyInfo;
             ClassType = JsonClassInfo.GetClassType(runtimePropertyType);
-            if (elementType != null)
-            {
-                Debug.Assert(ClassType == ClassType.Enumerable || ClassType == ClassType.Dictionary);
-                ElementClassInfo = options.GetOrAddClass(elementType);
-            }
-
+            _elementType = elementType;
+            Options = options;
             IsNullableType = runtimePropertyType.IsGenericType && runtimePropertyType.GetGenericTypeDefinition() == typeof(Nullable<>);
             CanBeNull = IsNullableType || !runtimePropertyType.IsValueType;
         }
 
         public bool CanBeNull { get; private set; }
-        public JsonClassInfo ElementClassInfo { get; private set; }
         public JsonEnumerableConverter EnumerableConverter { get; private set; }
 
         public bool IsNullableType { get; private set; }
@@ -83,14 +113,14 @@ public virtual void Initialize(
 
         public Type RuntimePropertyType { get; private set; }
 
-        public virtual void GetPolicies(JsonSerializerOptions options)
+        public virtual void GetPolicies()
         {
-            DetermineSerializationCapabilities(options);
-            DeterminePropertyName(options);
-            IgnoreNullValues = options.IgnoreNullValues;
+            DetermineSerializationCapabilities();
+            DeterminePropertyName();
+            IgnoreNullValues = Options.IgnoreNullValues;
         }
 
-        private void DeterminePropertyName(JsonSerializerOptions options)
+        private void DeterminePropertyName()
         {
             if (PropertyInfo == null)
             {
@@ -108,9 +138,9 @@ private void DeterminePropertyName(JsonSerializerOptions options)
 
                 NameAsString = name;
             }
-            else if (options.PropertyNamingPolicy != null)
+            else if (Options.PropertyNamingPolicy != null)
             {
-                string name = options.PropertyNamingPolicy.ConvertName(PropertyInfo.Name);
+                string name = Options.PropertyNamingPolicy.ConvertName(PropertyInfo.Name);
                 if (name == null)
                 {
                     ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameNull(ParentClassType, this);
@@ -129,7 +159,7 @@ private void DeterminePropertyName(JsonSerializerOptions options)
             Name = Encoding.UTF8.GetBytes(NameAsString);
 
             // Set the compare name.
-            if (options.PropertyNameCaseInsensitive)
+            if (Options.PropertyNameCaseInsensitive)
             {
                 NameUsedToCompareAsString = NameAsString.ToUpperInvariant();
                 NameUsedToCompare = Encoding.UTF8.GetBytes(NameUsedToCompareAsString);
@@ -173,12 +203,12 @@ private void DeterminePropertyName(JsonSerializerOptions options)
 #endif
         }
 
-        private void DetermineSerializationCapabilities(JsonSerializerOptions options)
+        private void DetermineSerializationCapabilities()
         {
             if (ClassType != ClassType.Enumerable && ClassType != ClassType.Dictionary)
             {
                 // We serialize if there is a getter + not ignoring readonly properties.
-                ShouldSerialize = HasGetter && (HasSetter || !options.IgnoreReadOnlyProperties);
+                ShouldSerialize = HasGetter && (HasSetter || !Options.IgnoreReadOnlyProperties);
 
                 // We deserialize if there is a setter. 
                 ShouldDeserialize = HasSetter;
@@ -233,13 +263,13 @@ private void DetermineSerializationCapabilities(JsonSerializerOptions options)
                             RuntimePropertyType.GetGenericArguments().Length == 1)
                         {
                             EnumerableConverter = s_jsonImmutableConverter;
-                            ((DefaultImmutableConverter)EnumerableConverter).RegisterImmutableCollectionType(RuntimePropertyType, elementType, options);
+                            ((DefaultImmutableConverter)EnumerableConverter).RegisterImmutableCollectionType(RuntimePropertyType, elementType, Options);
                         }
                     }
                 }
                 else
                 {
-                    ShouldSerialize = HasGetter && !options.IgnoreReadOnlyProperties;
+                    ShouldSerialize = HasGetter && !Options.IgnoreReadOnlyProperties;
                 }
             }
         }
@@ -264,8 +294,9 @@ public void CopyRuntimeSettingsTo(JsonPropertyInfo other)
         public static JsonPropertyInfo CreateIgnoredPropertyPlaceholder(PropertyInfo propertyInfo, JsonSerializerOptions options)
         {
             JsonPropertyInfo jsonPropertyInfo = new JsonPropertyInfoNotNullable<int, int, int>();
+            jsonPropertyInfo.Options = options;
             jsonPropertyInfo.PropertyInfo = propertyInfo;
-            jsonPropertyInfo.DeterminePropertyName(options);
+            jsonPropertyInfo.DeterminePropertyName();
 
             Debug.Assert(!jsonPropertyInfo.ShouldDeserialize);
             Debug.Assert(!jsonPropertyInfo.ShouldSerialize);
@@ -288,13 +319,13 @@ public static TAttribute GetAttribute<TAttribute>(PropertyInfo propertyInfo) whe
 
         public abstract Type GetDictionaryConcreteType();
 
-        public abstract void Read(JsonTokenType tokenType, JsonSerializerOptions options, ref ReadStack state, ref Utf8JsonReader reader);
-        public abstract void ReadEnumerable(JsonTokenType tokenType, JsonSerializerOptions options, ref ReadStack state, ref Utf8JsonReader reader);
+        public abstract void Read(JsonTokenType tokenType, ref ReadStack state, ref Utf8JsonReader reader);
+        public abstract void ReadEnumerable(JsonTokenType tokenType, ref ReadStack state, ref Utf8JsonReader reader);
         public abstract void SetValueAsObject(object obj, object value);
 
-        public abstract void Write(JsonSerializerOptions options, ref WriteStackFrame current, Utf8JsonWriter writer);
+        public abstract void Write(ref WriteStackFrame current, Utf8JsonWriter writer);
 
-        public virtual void WriteDictionary(JsonSerializerOptions options, ref WriteStackFrame current, Utf8JsonWriter writer) { }
-        public abstract void WriteEnumerable(JsonSerializerOptions options, ref WriteStackFrame current, Utf8JsonWriter writer);
+        public virtual void WriteDictionary(ref WriteStackFrame current, Utf8JsonWriter writer) { }
+        public abstract void WriteEnumerable(ref WriteStackFrame current, Utf8JsonWriter writer);
     }
 }
diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs
@@ -53,13 +53,13 @@ public override void Initialize(
                 ValueConverter = DefaultConverters<TRuntimeProperty>.s_converter;
             }
 
-            GetPolicies(options);
+            GetPolicies();
         }
 
-        public override void GetPolicies(JsonSerializerOptions options)
+        public override void GetPolicies()
         {
             ValueConverter = DefaultConverters<TRuntimeProperty>.s_converter;
-            base.GetPolicies(options);
+            base.GetPolicies();
         }
 
         public override object GetValueAsObject(object obj)

diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullable.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullable.cs
@@ -14,15 +14,15 @@ internal sealed class JsonPropertyInfoNotNullable<TClass, TDeclaredProperty, TRu
         JsonPropertyInfoCommon<TClass, TDeclaredProperty, TRuntimeProperty>
         where TRuntimeProperty : TDeclaredProperty
     {
-        public override void Read(JsonTokenType tokenType, JsonSerializerOptions options, ref ReadStack state, ref Utf8JsonReader reader)
+        public override void Read(JsonTokenType tokenType, ref ReadStack state, ref Utf8JsonReader reader)
         {
             Debug.Assert(ShouldDeserialize);
 
             if (ElementClassInfo != null)
             {
                 // Forward the setter to the value-based JsonPropertyInfo.
                 JsonPropertyInfo propertyInfo = ElementClassInfo.GetPolicyProperty();
-                propertyInfo.ReadEnumerable(tokenType, options, ref state, ref reader);
+                propertyInfo.ReadEnumerable(tokenType, ref state, ref reader);
             }
             else
             {
@@ -48,7 +48,7 @@ public override void Read(JsonTokenType tokenType, JsonSerializerOptions options
         }
 
         // If this method is changed, also change JsonPropertyInfoNullable.ReadEnumerable and JsonSerializer.ApplyObjectToEnumerable
-        public override void ReadEnumerable(JsonTokenType tokenType, JsonSerializerOptions options, ref ReadStack state, ref Utf8JsonReader reader)
+        public override void ReadEnumerable(JsonTokenType tokenType, ref ReadStack state, ref Utf8JsonReader reader)
         {
             Debug.Assert(ShouldDeserialize);
 
@@ -61,7 +61,7 @@ public override void ReadEnumerable(JsonTokenType tokenType, JsonSerializerOptio
             JsonSerializer.ApplyValueToEnumerable(ref value, ref state, ref reader);
         }
 
-        public override void Write(JsonSerializerOptions options, ref WriteStackFrame current, Utf8JsonWriter writer)
+        public override void Write(ref WriteStackFrame current, Utf8JsonWriter writer)
         {
             Debug.Assert(current.Enumerator == null);
             Debug.Assert(ShouldSerialize);
@@ -98,13 +98,13 @@ public override void Write(JsonSerializerOptions options, ref WriteStackFrame cu
             }
         }
 
-        public override void WriteDictionary(JsonSerializerOptions options, ref WriteStackFrame current, Utf8JsonWriter writer)
+        public override void WriteDictionary(ref WriteStackFrame current, Utf8JsonWriter writer)
         {
             Debug.Assert(ShouldSerialize);
-            JsonSerializer.WriteDictionary(ValueConverter, options, ref current, writer);
+            JsonSerializer.WriteDictionary(ValueConverter, Options, ref current, writer);
         }
 
-        public override void WriteEnumerable(JsonSerializerOptions options, ref WriteStackFrame current, Utf8JsonWriter writer)
+        public override void WriteEnumerable(ref WriteStackFrame current, Utf8JsonWriter writer)
         {
             Debug.Assert(ShouldSerialize);
 

diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNullable.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNullable.cs
@@ -17,7 +17,7 @@ internal sealed class JsonPropertyInfoNullable<TClass, TProperty>
         // should this be cached somewhere else so that it's not populated per TClass as well as TProperty?
         private static readonly Type s_underlyingType = typeof(TProperty);
 
-        public override void Read(JsonTokenType tokenType, JsonSerializerOptions options, ref ReadStack state, ref Utf8JsonReader reader)
+        public override void Read(JsonTokenType tokenType, ref ReadStack state, ref Utf8JsonReader reader)
         {
             Debug.Assert(ElementClassInfo == null);
             Debug.Assert(ShouldDeserialize);
@@ -39,7 +39,7 @@ public override void Read(JsonTokenType tokenType, JsonSerializerOptions options
             ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(RuntimePropertyType, reader, state.PropertyPath);
         }
 
-        public override void ReadEnumerable(JsonTokenType tokenType, JsonSerializerOptions options, ref ReadStack state, ref Utf8JsonReader reader)
+        public override void ReadEnumerable(JsonTokenType tokenType, ref ReadStack state, ref Utf8JsonReader reader)
         {
             Debug.Assert(ShouldDeserialize);
 
@@ -53,15 +53,15 @@ public override void ReadEnumerable(JsonTokenType tokenType, JsonSerializerOptio
             JsonSerializer.ApplyValueToEnumerable(ref nullableValue, ref state, ref reader);
         }
 
-        public override void Write(JsonSerializerOptions options, ref WriteStackFrame current, Utf8JsonWriter writer)
+        public override void Write(ref WriteStackFrame current, Utf8JsonWriter writer)
         {
             Debug.Assert(ShouldSerialize);
 
             if (current.Enumerator != null)
             {
                 // Forward the setter to the value-based JsonPropertyInfo.
                 JsonPropertyInfo propertyInfo = ElementClassInfo.GetPolicyProperty();
-                propertyInfo.WriteEnumerable(options, ref current, writer);
+                propertyInfo.WriteEnumerable(ref current, writer);
             }
             else
             {
@@ -98,7 +98,7 @@ public override void Write(JsonSerializerOptions options, ref WriteStackFrame cu
             }
         }
 
-        public override void WriteEnumerable(JsonSerializerOptions options, ref WriteStackFrame current, Utf8JsonWriter writer)
+        public override void WriteEnumerable(ref WriteStackFrame current, Utf8JsonWriter writer)
         {
             Debug.Assert(ShouldSerialize);
 

diff --git a/...stem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs b/...stem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs
@@ -50,7 +50,7 @@ private static void HandleStartDictionary(JsonSerializerOptions options, ref Utf
             if (jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue) == null)
             {
                 // Create the dictionary.
-                JsonClassInfo dictionaryClassInfo = options.GetOrAddClass(jsonPropertyInfo.RuntimePropertyType);
+                JsonClassInfo dictionaryClassInfo = jsonPropertyInfo.RuntimeClassInfo;
                 IDictionary value = (IDictionary)dictionaryClassInfo.CreateObject();
                 if (value != null)
                 {

diff --git a/...em.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/...em.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs
@@ -101,17 +101,15 @@ private static void ProcessMissingProperty(
             IDictionary extensionData = (IDictionary)jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue);
             if (extensionData == null)
             {
-                Type type = jsonPropertyInfo.DeclaredPropertyType;
-
                 // Create the appropriate dictionary type. We already verified the types.
-                Debug.Assert(type.IsGenericType);
-                Debug.Assert(type.GetGenericArguments().Length == 2);
-                Debug.Assert(type.GetGenericArguments()[0].UnderlyingSystemType == typeof(string));
+                Debug.Assert(jsonPropertyInfo.DeclaredPropertyType.IsGenericType);
+                Debug.Assert(jsonPropertyInfo.DeclaredPropertyType.GetGenericArguments().Length == 2);
+                Debug.Assert(jsonPropertyInfo.DeclaredPropertyType.GetGenericArguments()[0].UnderlyingSystemType == typeof(string));
                 Debug.Assert(
-                    type.GetGenericArguments()[1].UnderlyingSystemType == typeof(object) ||
-                    type.GetGenericArguments()[1].UnderlyingSystemType == typeof(JsonElement));
+                    jsonPropertyInfo.DeclaredPropertyType.GetGenericArguments()[1].UnderlyingSystemType == typeof(object) ||
+                    jsonPropertyInfo.DeclaredPropertyType.GetGenericArguments()[1].UnderlyingSystemType == typeof(JsonElement));
 
-                extensionData = (IDictionary)options.GetOrAddClass(type).CreateObject();
+                extensionData = (IDictionary)jsonPropertyInfo.RuntimeClassInfo.CreateObject();
                 jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, extensionData);
             }
 

diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleValue.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleValue.cs
@@ -25,7 +25,7 @@ private static bool HandleValue(JsonTokenType tokenType, JsonSerializerOptions o
 
             bool lastCall = (!state.Current.IsProcessingEnumerableOrDictionary && state.Current.ReturnValue == null);
 
-            jsonPropertyInfo.Read(tokenType, options, ref state, ref reader);
+            jsonPropertyInfo.Read(tokenType, ref state, ref reader);
             return lastCall;
         }
     }

diff --git a/...tem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleDictionary.cs b/...tem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleDictionary.cs
@@ -52,7 +52,7 @@ private static bool HandleDictionary(
 
                     if (elementClassInfo.ClassType == ClassType.Value)
                     {
-                        elementClassInfo.GetPolicyProperty().WriteDictionary(options, ref state.Current, writer);
+                        elementClassInfo.GetPolicyProperty().WriteDictionary(ref state.Current, writer);
                     }
                     else if (state.Current.Enumerator.Current == null)
                     {

diff --git a/...tem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleEnumerable.cs b/...tem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleEnumerable.cs
@@ -48,7 +48,7 @@ private static bool HandleEnumerable(
 
                 if (elementClassInfo.ClassType == ClassType.Value)
                 {
-                    elementClassInfo.GetPolicyProperty().WriteEnumerable(options, ref state.Current, writer);
+                    elementClassInfo.GetPolicyProperty().WriteEnumerable(ref state.Current, writer);
                 }
                 else if (state.Current.Enumerator.Current == null)
                 {

diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleObject.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleObject.cs
@@ -74,7 +74,7 @@ private static bool HandleObject(
 
             if (jsonPropertyInfo.ClassType == ClassType.Value)
             {
-                jsonPropertyInfo.Write(options, ref state.Current, writer);
+                jsonPropertyInfo.Write(ref state.Current, writer);
                 state.Current.NextProperty();
                 return true;
             }
@@ -116,7 +116,7 @@ private static bool HandleObject(
 
                 state.Current.NextProperty();
 
-                JsonClassInfo nextClassInfo = options.GetOrAddClass(jsonPropertyInfo.RuntimePropertyType);
+                JsonClassInfo nextClassInfo = jsonPropertyInfo.RuntimeClassInfo;
                 state.Push(nextClassInfo, currentValue);
 
                 // Set the PropertyInfo so we can obtain the property name in order to write it.

diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.cs
@@ -31,7 +31,7 @@ private static bool Write(
                         break;
                     case ClassType.Value:
                         Debug.Assert(current.JsonPropertyInfo.ClassType == ClassType.Value);
-                        current.JsonPropertyInfo.Write(options, ref current, writer);
+                        current.JsonPropertyInfo.Write(ref current, writer);
                         finishedSerializing = true;
                         break;
                     case ClassType.Object:

diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs
@@ -136,7 +136,7 @@ public static object CreateEnumerableValue(ref Utf8JsonReader reader, ref ReadSt
             if (typeof(IList).IsAssignableFrom(propType))
             {
                 // If IList, add the members as we create them.
-                JsonClassInfo collectionClassInfo = options.GetOrAddClass(propType);
+                JsonClassInfo collectionClassInfo = state.Current.JsonPropertyInfo.RuntimeClassInfo;
                 IList collection = (IList)collectionClassInfo.CreateObject();
                 return collection;
             }