From 00e709302a5e20f18f7458832960c393ee3f490b Mon Sep 17 00:00:00 2001 From: glucaci Date: Mon, 5 Jan 2026 17:16:17 +0100 Subject: [PATCH] fix: prevent infinite loops in collection deserialization by skipping unexpected tokens --- .../YamlSourceGenerator.cs | 6 + src/Yamlify/Reader/Utf8YamlReader.cs | 7 + .../InfiniteLoopRegressionTests.cs | 493 ++++++++++++++++++ 3 files changed, 506 insertions(+) create mode 100644 test/Yamlify.Tests/Serialization/InfiniteLoopRegressionTests.cs diff --git a/src/Yamlify.SourceGenerator/YamlSourceGenerator.cs b/src/Yamlify.SourceGenerator/YamlSourceGenerator.cs index 711a411..165c710 100644 --- a/src/Yamlify.SourceGenerator/YamlSourceGenerator.cs +++ b/src/Yamlify.SourceGenerator/YamlSourceGenerator.cs @@ -566,6 +566,8 @@ private static void GenerateReadMethod(StringBuilder sb, TypeToGenerate type, IR sb.AppendLine(" if (reader.TokenType != YamlTokenType.MappingStart)"); sb.AppendLine(" {"); + sb.AppendLine(" // Skip unexpected token to prevent infinite loops when reading collections"); + sb.AppendLine(" reader.Skip();"); sb.AppendLine(" return default;"); sb.AppendLine(" }"); sb.AppendLine(); @@ -2819,6 +2821,8 @@ private static void GenerateRootCollectionRead(StringBuilder sb, ITypeSymbol col sb.AppendLine(" if (reader.TokenType != YamlTokenType.SequenceStart)"); sb.AppendLine(" {"); + sb.AppendLine(" // Skip unexpected token to prevent infinite loops when reading collections"); + sb.AppendLine(" reader.Skip();"); sb.AppendLine(" return default;"); sb.AppendLine(" }"); sb.AppendLine(); @@ -2860,6 +2864,8 @@ private static void GenerateRootDictionaryRead(StringBuilder sb, ITypeSymbol dic sb.AppendLine(" if (reader.TokenType != YamlTokenType.MappingStart)"); sb.AppendLine(" {"); + sb.AppendLine(" // Skip unexpected token to prevent infinite loops when reading collections"); + sb.AppendLine(" reader.Skip();"); sb.AppendLine(" return default;"); sb.AppendLine(" }"); sb.AppendLine(); diff --git a/src/Yamlify/Reader/Utf8YamlReader.cs b/src/Yamlify/Reader/Utf8YamlReader.cs index 30a3d7e..baab585 100644 --- a/src/Yamlify/Reader/Utf8YamlReader.cs +++ b/src/Yamlify/Reader/Utf8YamlReader.cs @@ -587,6 +587,13 @@ public void Skip() // Keep reading until we exit the current depth } } + else if (_tokenType is YamlTokenType.Scalar) + { + // For scalar values (including nulls like ~, null, Null, NULL), just advance to the next token + Read(); + } + // For other tokens (end markers, etc.), do nothing - they're structural tokens + // that don't need to be skipped } } diff --git a/test/Yamlify.Tests/Serialization/InfiniteLoopRegressionTests.cs b/test/Yamlify.Tests/Serialization/InfiniteLoopRegressionTests.cs new file mode 100644 index 0000000..7cb5390 --- /dev/null +++ b/test/Yamlify.Tests/Serialization/InfiniteLoopRegressionTests.cs @@ -0,0 +1,493 @@ +using Yamlify.Serialization; + +namespace Yamlify.Tests.Serialization; + +/// +/// Regression tests for infinite loop bug that caused OOM exception. +/// +/// Bug Description: +/// When deserializing collections, if an element converter encountered an unexpected +/// token type (e.g., null or wrong type), it returned default without advancing the reader. +/// This caused the outer collection loop to repeatedly call the converter on the same token, +/// resulting in an infinite loop and eventually OOM. +/// +/// Fix: +/// Added reader.Skip() before returning default in generated converters when +/// token type doesn't match expected (MappingStart for objects, SequenceStart for collections). +/// +public class InfiniteLoopRegressionTests +{ + #region Test Models + + /// + /// Class with array of objects - the original bug scenario. + /// + public class ContainerWithObjectArray + { + public ObjectItem[]? Items { get; set; } + } + + public class ObjectItem + { + public string? Name { get; set; } + public int Value { get; set; } + } + + /// + /// Class with list of objects. + /// + public class ContainerWithObjectList + { + public List? Items { get; set; } + } + + /// + /// Base class for polymorphic test. + /// + [YamlDerivedType("type-a")] + [YamlDerivedType("type-b")] + public class BasePolymorphicType + { + public string? CommonProperty { get; set; } + } + + public class DerivedTypeA : BasePolymorphicType + { + public string? TypeAProperty { get; set; } + } + + public class DerivedTypeB : BasePolymorphicType + { + public int TypeBValue { get; set; } + } + + /// + /// Container with polymorphic array. + /// + public class ContainerWithPolymorphicArray + { + public BasePolymorphicType[]? Items { get; set; } + } + + /// + /// Container with dictionary having object values. + /// + public class ContainerWithObjectDictionary + { + public Dictionary? ItemsByKey { get; set; } + } + + #endregion + + #region Null Element Tests + + /// + /// Test: Array with explicit null element should not cause infinite loop. + /// The null should be skipped or handled gracefully. + /// + [Fact] + public void DeserializeArrayWithNullElement_DoesNotHang() + { + var yaml = """ + items: + - name: First + value: 1 + - ~ + - name: Third + value: 3 + """; + + // If the fix is not in place, this will hang forever (infinite loop). + // With the fix, it completes quickly with null elements represented as null in the array. + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectArray); + + Assert.NotNull(result?.Items); + Assert.Equal(3, result.Items.Length); + Assert.NotNull(result.Items[0]); + Assert.Null(result.Items[1]); // Null element + Assert.NotNull(result.Items[2]); + } + + /// + /// Test: Array with null keyword element should not cause infinite loop. + /// + [Fact] + public void DeserializeArrayWithNullKeyword_DoesNotHang() + { + var yaml = """ + items: + - name: First + value: 1 + - null + - name: Third + value: 3 + """; + + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectArray); + + Assert.NotNull(result?.Items); + Assert.Equal(3, result.Items.Length); + Assert.Null(result.Items[1]); // Null element + } + + /// + /// Test: List with null elements should not cause infinite loop. + /// + [Fact] + public void DeserializeListWithNullElements_DoesNotHang() + { + var yaml = """ + items: + - name: First + value: 1 + - ~ + - ~ + - name: Fourth + value: 4 + """; + + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectList); + + Assert.NotNull(result?.Items); + Assert.Equal(4, result.Items.Count); + Assert.NotNull(result.Items[0]); + Assert.Null(result.Items[1]); // Null element + Assert.Null(result.Items[2]); // Null element + Assert.NotNull(result.Items[3]); + } + + #endregion + + #region Wrong Type Element Tests + + /// + /// Test: Array element that is a scalar instead of mapping should not cause infinite loop. + /// + [Fact] + public void DeserializeArrayWithScalarElement_DoesNotHang() + { + var yaml = """ + items: + - name: First + value: 1 + - just-a-string + - name: Third + value: 3 + """; + + // Scalar element where object expected should be skipped (null) + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectArray); + + Assert.NotNull(result?.Items); + Assert.Equal(3, result.Items.Length); + Assert.NotNull(result.Items[0]); + Assert.Null(result.Items[1]); // Scalar skipped, becomes null + Assert.NotNull(result.Items[2]); + } + + /// + /// Test: Array element that is a sequence instead of mapping should not cause infinite loop. + /// + [Fact] + public void DeserializeArrayWithSequenceElement_DoesNotHang() + { + var yaml = """ + items: + - name: First + value: 1 + - - nested + - sequence + - name: Third + value: 3 + """; + + // The key assertion is that this completes without hanging (OOM). + // Sequence element where object expected is skipped, and the sequence contents + // may also be consumed during the skip operation. + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectArray); + + Assert.NotNull(result?.Items); + // At least the valid items should be present + Assert.True(result.Items.Length >= 1, "At least the first valid item should be parsed"); + Assert.Equal("First", result.Items[0]?.Name); + } + + #endregion + + #region Polymorphic Collection Tests + + /// + /// Test: Polymorphic array with null elements should not cause infinite loop. + /// + [Fact] + public void DeserializePolymorphicArrayWithNullElement_DoesNotHang() + { + var yaml = """ + items: + - $type: type-a + common-property: Common1 + type-a-property: ValueA + - ~ + - $type: type-b + common-property: Common2 + type-b-value: 42 + """; + + // The key assertion is that this completes without hanging (OOM). + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithPolymorphicArray); + + Assert.NotNull(result?.Items); + Assert.Equal(3, result.Items.Length); + // The polymorphic deserialization may fall back to base type if discriminator handling differs + Assert.NotNull(result.Items[0]); + Assert.Null(result.Items[1]); // Null element + Assert.NotNull(result.Items[2]); + } + + /// + /// Test: Polymorphic array with invalid discriminator should not cause infinite loop. + /// + [Fact] + public void DeserializePolymorphicArrayWithInvalidDiscriminator_DoesNotHang() + { + var yaml = """ + items: + - $type: type-a + common-property: Common1 + type-a-property: ValueA + - $type: unknown-type + some-property: value + - $type: type-b + common-property: Common2 + type-b-value: 42 + """; + + // The key assertion is that this completes without hanging (OOM). + // Invalid discriminator results in fallback behavior (base type or null) + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithPolymorphicArray); + + Assert.NotNull(result?.Items); + Assert.Equal(3, result.Items.Length); + Assert.NotNull(result.Items[0]); // First valid item + // Unknown type may parse as base type or null - both are acceptable + Assert.NotNull(result.Items[2]); // Third valid item + } + + #endregion + + #region Dictionary Tests + + /// + /// Test: Dictionary with null values should not cause infinite loop. + /// + [Fact] + public void DeserializeDictionaryWithNullValue_DoesNotHang() + { + var yaml = """ + items-by-key: + first: + name: First + value: 1 + second: ~ + third: + name: Third + value: 3 + """; + + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectDictionary); + + Assert.NotNull(result?.ItemsByKey); + Assert.Equal(3, result.ItemsByKey.Count); + Assert.NotNull(result.ItemsByKey["first"]); + Assert.Null(result.ItemsByKey["second"]); // Null value + Assert.NotNull(result.ItemsByKey["third"]); + } + + /// + /// Test: Dictionary with scalar value instead of object should not cause infinite loop. + /// + [Fact] + public void DeserializeDictionaryWithScalarValue_DoesNotHang() + { + var yaml = """ + items-by-key: + first: + name: First + value: 1 + second: just-a-string + third: + name: Third + value: 3 + """; + + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectDictionary); + + Assert.NotNull(result?.ItemsByKey); + Assert.Equal(3, result.ItemsByKey.Count); + Assert.NotNull(result.ItemsByKey["first"]); + Assert.Null(result.ItemsByKey["second"]); // Scalar skipped, becomes null + Assert.NotNull(result.ItemsByKey["third"]); + } + + #endregion + + #region Nested Collection Tests + + // Note: Nested collections with object elements (List>) + // require special handling that is beyond the scope of this regression test. + // The core issue (infinite loop on null/scalar elements) is tested by other tests. + + #endregion + + #region Root Collection Tests + + /// + /// Test: Root-level list with null elements should not cause infinite loop. + /// + [Fact] + public void DeserializeRootListWithNullElements_DoesNotHang() + { + var yaml = """ + - name: First + value: 1 + - ~ + - name: Third + value: 3 + """; + + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ListObjectItem); + + Assert.NotNull(result); + Assert.Equal(3, result.Count); + Assert.NotNull(result[0]); + Assert.Null(result[1]); // Null element + Assert.NotNull(result[2]); + } + + /// + /// Test: Root-level dictionary with null values should not cause infinite loop. + /// + [Fact] + public void DeserializeRootDictionaryWithNullValues_DoesNotHang() + { + var yaml = """ + first: + name: First + value: 1 + second: ~ + third: + name: Third + value: 3 + """; + + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.DictionaryStringObjectItem); + + Assert.NotNull(result); + Assert.Equal(3, result.Count); + Assert.NotNull(result["first"]); + Assert.Null(result["second"]); // Null value + Assert.NotNull(result["third"]); + } + + #endregion + + #region Empty Element Tests + + /// + /// Test: Array with empty mapping element should work correctly. + /// + [Fact] + public void DeserializeArrayWithEmptyMappingElement_Works() + { + var yaml = """ + items: + - name: First + value: 1 + - {} + - name: Third + value: 3 + """; + + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectArray); + + Assert.NotNull(result?.Items); + Assert.Equal(3, result.Items.Length); + Assert.NotNull(result.Items[0]); + Assert.NotNull(result.Items[1]); // Empty mapping creates object with defaults + Assert.NotNull(result.Items[2]); + } + + #endregion + + #region Multiple Null Elements in Sequence + + /// + /// Test: Multiple consecutive null elements should not cause infinite loop. + /// This specifically tests that the reader advances correctly through multiple nulls. + /// + [Fact] + public void DeserializeArrayWithMultipleConsecutiveNulls_DoesNotHang() + { + var yaml = """ + items: + - ~ + - ~ + - ~ + - name: OnlyItem + value: 1 + - ~ + - ~ + """; + + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectArray); + + Assert.NotNull(result?.Items); + Assert.Equal(6, result.Items.Length); + Assert.Null(result.Items[0]); + Assert.Null(result.Items[1]); + Assert.Null(result.Items[2]); + Assert.NotNull(result.Items[3]); + Assert.Equal("OnlyItem", result.Items[3]!.Name); + Assert.Null(result.Items[4]); + Assert.Null(result.Items[5]); + } + + /// + /// Test: Array with all null elements should not cause infinite loop. + /// + [Fact] + public void DeserializeArrayWithAllNullElements_DoesNotHang() + { + var yaml = """ + items: + - ~ + - ~ + - ~ + """; + + var result = YamlSerializer.Deserialize(yaml, InfiniteLoopTestContext.Default.ContainerWithObjectArray); + + Assert.NotNull(result?.Items); + Assert.Equal(3, result.Items.Length); + Assert.All(result.Items, item => Assert.Null(item)); + } + + #endregion +} + +/// +/// Serializer context for infinite loop regression tests. +/// +[YamlSerializable(typeof(InfiniteLoopRegressionTests.ContainerWithObjectArray))] +[YamlSerializable(typeof(InfiniteLoopRegressionTests.ContainerWithObjectList))] +[YamlSerializable(typeof(InfiniteLoopRegressionTests.ObjectItem))] +[YamlSerializable(typeof(InfiniteLoopRegressionTests.BasePolymorphicType))] +[YamlSerializable(typeof(InfiniteLoopRegressionTests.DerivedTypeA))] +[YamlSerializable(typeof(InfiniteLoopRegressionTests.DerivedTypeB))] +[YamlSerializable(typeof(InfiniteLoopRegressionTests.ContainerWithPolymorphicArray))] +[YamlSerializable(typeof(InfiniteLoopRegressionTests.ContainerWithObjectDictionary))] +[YamlSerializable(typeof(List))] +[YamlSerializable(typeof(Dictionary))] +public partial class InfiniteLoopTestContext : YamlSerializerContext +{ +}