From b62a0358afb37d416993030ca9a34bab66cb6dc6 Mon Sep 17 00:00:00 2001 From: Temo Nikolaishvili Date: Mon, 24 Aug 2026 12:28:11 +0400 Subject: [PATCH 1/2] Prevent stack overflow on cyclic and XML types in TypeExt - Update project version to 2.1.5 in Directory.Build.props - Bump NuGet packages: AltaSoft.DomainPrimitives 8.1.2, Microsoft.NET.Test.Sdk 18.9.0, xunit.runner.visualstudio 4.0.0 - Refactor TypeExt.cs to detect cycles and treat XML types as leaf nodes - Add XML type detection helpers and improve recursive property handling - Add tests for XmlElement, XmlNode, and circular dependencies in SimpraMetaDataTest.cs --- Directory.Build.props | 2 +- Directory.Packages.props | 8 +- src/AltaSoft.Simpra.Metadata/TypeExt.cs | 211 ++++++++++++------ .../SimpraMetaDataTest.cs | 153 +++++++++++++ 4 files changed, 296 insertions(+), 78 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 7bba322..08167f9 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -11,7 +11,7 @@ Simpra ALTA Software llc. Copyright © 2024 ALTA Software llc. - 2.1.4 + 2.1.5 diff --git a/Directory.Packages.props b/Directory.Packages.props index f3e303b..3991a89 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -4,13 +4,13 @@ - - + + - + - + diff --git a/src/AltaSoft.Simpra.Metadata/TypeExt.cs b/src/AltaSoft.Simpra.Metadata/TypeExt.cs index 3f43ae4..e0a1cfb 100644 --- a/src/AltaSoft.Simpra.Metadata/TypeExt.cs +++ b/src/AltaSoft.Simpra.Metadata/TypeExt.cs @@ -55,108 +55,173 @@ internal static List ProcessProperties(this Type type) return cachedProperties; } + var visited = new HashSet(); + var properties = ProcessPropertiesInternal(type, visited); + s_propertyCache[type] = properties; + return properties; + } + + private static List ProcessPropertiesInternal(Type type, HashSet visited) + { var properties = type.GetProperties(BindingFlags.Instance | BindingFlags.Public); var processedProperties = properties .Where(x => x.GetCustomAttribute() is null) .Where(x => x.GetMethod is not null && x.CanRead) - .SelectMany(prop => Process(prop.Name, Nullable.GetUnderlyingType(prop.PropertyType) is null, prop.PropertyType)).Distinct().ToList(); + .SelectMany(prop => Process(prop.Name, Nullable.GetUnderlyingType(prop.PropertyType) is null, prop.PropertyType, visited)).Distinct().ToList(); - s_propertyCache[type] = processedProperties; return processedProperties; } - private static List Process(string name, bool isRequired, Type type) + private static List Process(string name, bool isRequired, Type type, HashSet visited) { - if (type.TryGetUnderlyingDomainPrimitiveType(out var primitive)) - { - return - [ - new PropertyModel - { - Name = name, - Description = type.GetSummaryWithoutException(), - Type = primitive.AliasOrName(), - Required = isRequired - } - ]; - } - if (s_primitiveTypes.Contains(type)) + var actualType = Nullable.GetUnderlyingType(type) ?? type; + + // Check for cycle: if type is already in visited path, skip expansion + // but still return the property with its actual type name + if (visited.Contains(actualType)) { - return - [ - new PropertyModel - { - Name = name, - Description = type.GetSummaryWithoutException(), - Type = (Nullable.GetUnderlyingType(type) ?? type).AliasOrName(), - Required = isRequired - } - ]; + return [new PropertyModel + { + Name = name, + Description = type.GetSummaryWithoutException(), + Type = actualType.AliasOrName(), // Return actual type, not generic "object" + Required = isRequired + }]; } - if (type.IsEnum || Nullable.GetUnderlyingType(type)?.IsEnum == true) + + // Skip XML types (System.Xml namespace) - treat as leaf nodes + if (IsXmlType(actualType)) { - var enumType = Nullable.GetUnderlyingType(type) ?? type; - return - [ - new PropertyModel - { - Name = name, - Description = string.Join(" ", enumType.GetEnumNames()), - Type = "enum", - Required = isRequired - } - ]; + return [new PropertyModel + { + Name = name, + Description = type.GetSummaryWithoutException(), + Type = "object", // XML types are opaque - safe to use generic object + Required = isRequired + }]; } - if (type.IsArray) - { - var elementType = type.GetElementType() ?? throw new InvalidOperationException("Should not be null"); - if (elementType.Namespace == "System") + // Add current type to visited set to track the recursion path + visited.Add(actualType); + + try + { + if (type.TryGetUnderlyingDomainPrimitiveType(out var primitive)) { - var aliasOrName = (Nullable.GetUnderlyingType(type) ?? type).AliasOrName(); - return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = aliasOrName, Required = isRequired }]; + return + [ + new PropertyModel + { + Name = name, + Description = type.GetSummaryWithoutException(), + Type = primitive.AliasOrName(), + Required = isRequired + } + ]; } - - if (elementType.TryGetUnderlyingDomainPrimitiveType(out var primitiveType)) + if (s_primitiveTypes.Contains(type)) { - var aliasOrName = primitiveType.AliasOrName(); - return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = aliasOrName, Required = isRequired }]; + return + [ + new PropertyModel + { + Name = name, + Description = type.GetSummaryWithoutException(), + Type = actualType.AliasOrName(), + Required = isRequired + } + ]; } + if (type.IsEnum || Nullable.GetUnderlyingType(type)?.IsEnum == true) + { + var enumType = actualType; + return + [ + new PropertyModel + { + Name = name, + Description = string.Join(" ", enumType.GetEnumNames()), + Type = "enum", + Required = isRequired + } + ]; + } + if (type.IsArray) + { + var elementType = type.GetElementType() ?? throw new InvalidOperationException("Should not be null"); - var collectionProp = ProcessProperties(elementType); + if (elementType.Namespace == "System") + { + var aliasOrName = actualType.AliasOrName(); + return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = aliasOrName, Required = isRequired }]; + } - return MapToPrefixedPropertyModels(collectionProp, name); - } + if (elementType.TryGetUnderlyingDomainPrimitiveType(out var primitiveType)) + { + var aliasOrName = primitiveType.AliasOrName(); + return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = aliasOrName, Required = isRequired }]; + } - if (type is { IsGenericType: true, GenericTypeArguments.Length: >= 1 } && typeof(IEnumerable).IsAssignableFrom(type)) - { - var elementType = type.GetGenericArguments()[0]; + if (IsXmlType(elementType)) + { + return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = "object", Required = isRequired }]; + } - if (elementType.Namespace == "System") - { - var aliasOrName = (Nullable.GetUnderlyingType(type) ?? type).AliasOrName(); - return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = aliasOrName, Required = isRequired }]; + var collectionProp = ProcessPropertiesInternal(elementType, visited); + + return MapToPrefixedPropertyModels(collectionProp, name); } - if (elementType.TryGetUnderlyingDomainPrimitiveType(out var primitiveType)) + if (type is { IsGenericType: true, GenericTypeArguments.Length: >= 1 } && typeof(IEnumerable).IsAssignableFrom(type)) { - var aliasOrName = primitiveType.AliasOrName(); - return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = aliasOrName, Required = isRequired }]; - } + var elementType = type.GetGenericArguments()[0]; - var collectionProp = ProcessProperties(elementType); + if (elementType.Namespace == "System") + { + var aliasOrName = actualType.AliasOrName(); + return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = aliasOrName, Required = isRequired }]; + } - return MapToPrefixedPropertyModels(collectionProp, name); - } + if (elementType.TryGetUnderlyingDomainPrimitiveType(out var primitiveType)) + { + var aliasOrName = primitiveType.AliasOrName(); + return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = aliasOrName, Required = isRequired }]; + } + + if (IsXmlType(elementType)) + { + return [new PropertyModel { Name = $"{name}[]", Description = type.GetSummaryWithoutException(), Type = "object", Required = isRequired }]; + } - var obj = ProcessProperties(Nullable.GetUnderlyingType(type) ?? type); - return obj.Select(x => new PropertyModel + var collectionProp = ProcessPropertiesInternal(elementType, visited); + + return MapToPrefixedPropertyModels(collectionProp, name); + } + + var obj = ProcessPropertiesInternal(Nullable.GetUnderlyingType(type) ?? type, visited); + return obj.Select(x => new PropertyModel + { + Name = $"{name}.{x.Name}", + Description = x.Description, + Type = x.Type, + Required = x.Required + }).ToList(); + } + finally { - Name = $"{name}.{x.Name}", - Description = x.Description, - Type = x.Type, - Required = x.Required - }).ToList(); + // Remove from visited set when backtracking - allows same type in different branches + visited.Remove(actualType); + } + } + + private static bool IsXmlType(Type type) + { + if (type.Namespace?.StartsWith("System.Xml") != true) + return false; + + // List of XML DOM types that should not be expanded + var xmlTypeNames = new[] { "XmlNode", "XmlElement", "XmlAttribute", "XmlDocument" }; + return xmlTypeNames.Contains(type.Name); } private static List MapToPrefixedPropertyModels(List collection, string name) diff --git a/tests/AltaSoft.Simpra.tests/SimpraMetaDataTest.cs b/tests/AltaSoft.Simpra.tests/SimpraMetaDataTest.cs index 3b8ba98..cc9b130 100644 --- a/tests/AltaSoft.Simpra.tests/SimpraMetaDataTest.cs +++ b/tests/AltaSoft.Simpra.tests/SimpraMetaDataTest.cs @@ -1,4 +1,5 @@ using System.Text.Json.Serialization; +using System.Xml; using System.Xml.Serialization; using AltaSoft.Simpra.Metadata; using AltaSoft.Simpra.Metadata.Models; @@ -66,6 +67,99 @@ public void GetTypeModel_Should_ReturnCorrectModel_ForBaseClass() Assert.Contains(typeModel.Properties, p => p.Name == nameof(MyClass.Base)); Assert.Contains(typeModel.Functions, f => f.Name == nameof(MyClass.Father)); } + + [Fact] + public void GetTypeModel_Should_NotStackOverflow_WithXmlElementProperty() + { + var metadataService = new SimpraMetadataService([new MetadataDetails + { + ModelType = typeof(ReproModelWithXmlElement), + ExternalFunctionsType = null, + Name = "ReproModelWithXmlElement" + }]); + + var typeModel = metadataService.GetTypeModel("ReproModelWithXmlElement"); + + Assert.NotNull(typeModel); + Assert.Equal("ReproModelWithXmlElement", typeModel.Name); + Assert.NotEmpty(typeModel.Properties); + } + + [Fact] + public void GetTypeModel_Should_NotStackOverflow_WithNestedXmlElement() + { + var metadataService = new SimpraMetadataService([new MetadataDetails + { + ModelType = typeof(ReproSupplementaryDataModel), + ExternalFunctionsType = null, + Name = "ReproSupplementaryDataModel" + }]); + + var typeModel = metadataService.GetTypeModel("ReproSupplementaryDataModel"); + + Assert.NotNull(typeModel); + Assert.Equal("ReproSupplementaryDataModel", typeModel.Name); + Assert.NotEmpty(typeModel.Properties); + } + + [Fact] + public void GetTypeModel_Should_NotStackOverflow_WithXmlNode() + { + var metadataService = new SimpraMetadataService([new MetadataDetails + { + ModelType = typeof(ReproModelWithXmlNode), + ExternalFunctionsType = null, + Name = "ReproModelWithXmlNode" + }]); + + var typeModel = metadataService.GetTypeModel("ReproModelWithXmlNode"); + + Assert.NotNull(typeModel); + Assert.Equal("ReproModelWithXmlNode", typeModel.Name); + Assert.NotEmpty(typeModel.Properties); + } + + [Fact] + public void GetTypeModel_Should_HandleCircularDependency_WithoutStackOverflow() + { + var metadataService = new SimpraMetadataService([new MetadataDetails + { + ModelType = typeof(CircularModelA), + ExternalFunctionsType = null, + Name = "CircularModelA" + }]); + + var typeModel = metadataService.GetTypeModel("CircularModelA"); + + Assert.NotNull(typeModel); + Assert.Equal("CircularModelA", typeModel.Name); + Assert.NotEmpty(typeModel.Properties); + + // Verify the circular property is handled (should be present but not infinitely expanded) + Assert.Contains(typeModel.Properties, p => p.Name == nameof(CircularModelA.Name)); + Assert.Contains(typeModel.Properties, p => p.Name.StartsWith(nameof(CircularModelA.RefToB))); + } + + [Fact] + public void GetTypeModel_Should_HandleCircularDependency_StartingFromModelB() + { + var metadataService = new SimpraMetadataService([new MetadataDetails + { + ModelType = typeof(CircularModelB), + ExternalFunctionsType = null, + Name = "CircularModelB" + }]); + + var typeModel = metadataService.GetTypeModel("CircularModelB"); + + Assert.NotNull(typeModel); + Assert.Equal("CircularModelB", typeModel.Name); + Assert.NotEmpty(typeModel.Properties); + + // Verify properties are present + Assert.Contains(typeModel.Properties, p => p.Name == nameof(CircularModelB.Value)); + Assert.Contains(typeModel.Properties, p => p.Name.StartsWith(nameof(CircularModelB.RefToA))); + } } public class MyChildClass : MyClass @@ -98,3 +192,62 @@ public class MyClass1 public int Info { get; set; } public required List CollectionMyClasses { get; set; } } + +/// +/// Minimal repro model containing XmlElement property to trigger cyclic traversal +/// +public sealed class ReproModelWithXmlElement +{ + public ReproEnvelopeWithXmlElement Envelope { get; set; } = new(); +} + +public sealed class ReproEnvelopeWithXmlElement +{ + public XmlElement? Element { get; set; } +} + +/// +/// More complex repro model with nested XmlElement similar to actual issue +/// +public sealed class ReproSupplementaryDataModel +{ + public ReproSupplementaryData[] SupplementaryData { get; set; } = []; +} + +public sealed class ReproSupplementaryData +{ + public ReproSupplementaryDataEnvelope Envelope { get; set; } = new(); +} + +public sealed class ReproSupplementaryDataEnvelope +{ + public XmlElement? Any { get; set; } +} + +/// +/// Repro model with XmlNode property +/// +public sealed class ReproModelWithXmlNode +{ + public ReproNodeContainer NodeContainer { get; set; } = new(); +} + +public sealed class ReproNodeContainer +{ + public XmlNode? Node { get; set; } +} + +/// +/// Circular dependency models A B to test cycle detection (non-XML scenario) +/// +public sealed class CircularModelA +{ + public required string Name { get; set; } + public CircularModelB? RefToB { get; set; } +} + +public sealed class CircularModelB +{ + public int Value { get; set; } + public CircularModelA? RefToA { get; set; } +} From 452d381490a89bf21ea60c2684790639cd6480bf Mon Sep 17 00:00:00 2001 From: Teimuraz Nikolaishvili <47372530+temonk@users.noreply.github.com> Date: Mon, 24 Aug 2026 12:34:02 +0400 Subject: [PATCH 2/2] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/AltaSoft.Simpra.Metadata/TypeExt.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/AltaSoft.Simpra.Metadata/TypeExt.cs b/src/AltaSoft.Simpra.Metadata/TypeExt.cs index e0a1cfb..c780675 100644 --- a/src/AltaSoft.Simpra.Metadata/TypeExt.cs +++ b/src/AltaSoft.Simpra.Metadata/TypeExt.cs @@ -216,12 +216,7 @@ private static List Process(string name, bool isRequired, Type ty private static bool IsXmlType(Type type) { - if (type.Namespace?.StartsWith("System.Xml") != true) - return false; - - // List of XML DOM types that should not be expanded - var xmlTypeNames = new[] { "XmlNode", "XmlElement", "XmlAttribute", "XmlDocument" }; - return xmlTypeNames.Contains(type.Name); + return typeof(System.Xml.XmlNode).IsAssignableFrom(type); } private static List MapToPrefixedPropertyModels(List collection, string name)