-
-
Notifications
You must be signed in to change notification settings - Fork 50
Fix: Prevent infinite loop when serializing assets with reflection types #296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /* | ||
| ┌──────────────────────────────────────────────────────────────────┐ | ||
| │ Author: Ivan Murzak (https://github.com/IvanMurzak) │ | ||
| │ Repository: GitHub (https://github.com/IvanMurzak/Unity-MCP) │ | ||
| │ Copyright (c) 2025 Ivan Murzak │ | ||
| │ Licensed under the Apache License, Version 2.0. │ | ||
| │ See the LICENSE file in the project root for more information. │ | ||
| └──────────────────────────────────────────────────────────────────┘ | ||
| */ | ||
| #pragma warning disable CS8632 // The annotation for nullable reference types should only be used in code within a '#nullable' annotations context. | ||
| using System; | ||
|
|
||
| namespace com.IvanMurzak.Unity.MCP.Common.Reflection.Convertor | ||
| { | ||
| /// <summary> | ||
| /// Utility class for filtering reflection types that cause circular references during serialization. | ||
| /// </summary> | ||
| public static class ReflectionTypeFilter | ||
| { | ||
| /// <summary> | ||
| /// Determines if a type is a System.Reflection type that should be excluded from serialization. | ||
| /// Reflection types form circular references (RuntimeType -> RuntimeModule -> RuntimeAssembly -> RuntimeModule) | ||
| /// and cannot be meaningfully serialized to JSON or reconstructed during deserialization. | ||
| /// </summary> | ||
| /// <param name="type">The type to check.</param> | ||
| /// <returns>True if the type should be excluded from serialization; otherwise, false.</returns> | ||
| public static bool IsReflectionType(Type type) | ||
| { | ||
| if (type == null) return false; | ||
|
|
||
| var fullName = type.FullName; | ||
| if (string.IsNullOrEmpty(fullName)) return false; | ||
|
|
||
| // Catch System.Reflection.* namespace types | ||
| // Note: System.RuntimeType and System.Type are in System namespace, not System.Reflection | ||
| return fullName.StartsWith("System.Reflection.") || | ||
| fullName == "System.RuntimeType" || | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change it to type comparison instead of string comparison type == typeof(System.RuntimeType)
type == typeof(System.Type)There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| fullName == "System.Type"; | ||
| } | ||
| } | ||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,15 @@ public partial class UnityArrayReflectionConvertor : ArrayReflectionConvertor | |
| public override IEnumerable<FieldInfo>? GetSerializableFields(Reflector reflector, Type objType, BindingFlags flags, ILogger? logger = null) | ||
| => objType.GetFields(flags) | ||
| .Where(field => field.GetCustomAttribute<ObsoleteAttribute>() == null) | ||
| .Where(field => field.IsPublic || field.IsPrivate && field.GetCustomAttribute<SerializeField>() != null); | ||
| .Where(field => field.IsPublic || field.IsPrivate && field.GetCustomAttribute<SerializeField>() != null) | ||
| .Where(field => !ReflectionTypeFilter.IsReflectionType(field.FieldType)); | ||
|
|
||
| public override IEnumerable<PropertyInfo>? GetSerializableProperties(Reflector reflector, Type objType, BindingFlags flags, ILogger? logger = null) | ||
| { | ||
| var baseProperties = base.GetSerializableProperties(reflector, objType, flags, logger); | ||
| if (baseProperties == null) return null; | ||
|
|
||
| return baseProperties.Where(prop => !ReflectionTypeFilter.IsReflectionType(prop.PropertyType)); | ||
| } | ||
|
Comment on lines
+27
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to do it in this class. Need to take care of the problematic types globally by making a custom ReflectionConvertors for that type. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,15 @@ public partial class UnityGenericReflectionConvertor<T> : GenericReflectionConve | |
| public override IEnumerable<FieldInfo>? GetSerializableFields(Reflector reflector, Type objType, BindingFlags flags, ILogger? logger = null) | ||
| => objType.GetFields(flags) | ||
| .Where(field => field.GetCustomAttribute<ObsoleteAttribute>() == null) | ||
| .Where(field => field.IsPublic || field.IsPrivate && field.GetCustomAttribute<SerializeField>() != null); | ||
| .Where(field => field.IsPublic || field.IsPrivate && field.GetCustomAttribute<SerializeField>() != null) | ||
| .Where(field => !ReflectionTypeFilter.IsReflectionType(field.FieldType)); | ||
|
|
||
| public override IEnumerable<PropertyInfo>? GetSerializableProperties(Reflector reflector, Type objType, BindingFlags flags, ILogger? logger = null) | ||
| { | ||
| var baseProperties = base.GetSerializableProperties(reflector, objType, flags, logger); | ||
| if (baseProperties == null) return null; | ||
|
|
||
| return baseProperties.Where(prop => !ReflectionTypeFilter.IsReflectionType(prop.PropertyType)); | ||
| } | ||
|
Comment on lines
+27
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to do it in this class. Need to take care of the problematic types globally by making a custom ReflectionConvertors for that type. |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure sure about ignoring entire namespace
System.Reflection.*. Any purposes of this?