From 6c61030c8a0168685e4ba28fb5acf13f533d918a Mon Sep 17 00:00:00 2001 From: Loretta Date: Thu, 14 May 2026 00:13:06 +0200 Subject: [PATCH] Add MaxDepthBehavior for serializer recursion control Introduced MaxDepthBehavior enum and option to control recursion depth handling (Truncate, Throw, Disable) in AcSerializerOptions. Refactored depth-check logic to use a precomputed NeedsDepthCheck flag. Enhanced exception messages for depth-limit violations. Updated tests to assert correct exception behavior for cycles. Improved documentation and added new test/log commands in settings.local.json. --- .claude/settings.local.json | 6 ++- .../AcBinarySourceGenerator.cs | 20 ++++---- .../AcBinarySerializerIIdReferenceTests.cs | 22 +++++---- .../Serializers/AcSerializerOptions.cs | 46 ++++++++++++++++++- ...rySerializer.BinarySerializationContext.cs | 30 +++++++++++- .../Binaries/AcBinarySerializer.ScanPass.cs | 11 +++-- .../Binaries/AcBinarySerializer.cs | 13 ++++-- 7 files changed, 118 insertions(+), 30 deletions(-) diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 44ed17f..a804aa4 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -79,7 +79,11 @@ "PowerShell($progPath = \"H:\\\\Applications\\\\Aycode\\\\Source\\\\AyCode.Core\\\\AyCode.Core.Serializers.Console\\\\Program.cs\"; $c = [IO.File]::ReadAllText\\($progPath\\); $usings = \"using AyCode.Core.Serializers.Console.Benchmarks;`r`n\"; if \\(-not $c.Contains\\($usings.Trim\\(\\)\\)\\) { $idx = $c.IndexOf\\(\"namespace AyCode.Core.Serializers.Console;\"\\); $before = $c.Substring\\(0, $idx\\); $after = $c.Substring\\($idx\\); $c2 = $before + $usings + \"`r`n\" + $after; [IO.File]::WriteAllText\\($progPath, $c2\\); Write-Output \"Added 'using AyCode.Core.Serializers.Console.Benchmarks;' to Program.cs\" } else { Write-Output \"Using already present in Program.cs\" })", "Bash(git -C H:/Applications/Aycode/Source/AyCode.Core log --oneline -10 -- AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.cs)", "Bash(git -C H:/Applications/Aycode/Source/AyCode.Core log -p --all -S \"if \\(isComplexElement\\)\" -- AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.cs)", - "Bash(git -C H:/Applications/Aycode/Source/AyCode.Core show 2aa2eec -- AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.cs)" + "Bash(git -C H:/Applications/Aycode/Source/AyCode.Core show 2aa2eec -- AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.cs)", + "Bash(dotnet test *)", + "Read(//tmp/**)", + "Bash(git -C H:/Applications/Aycode/Source/AyCode.Core log -p --all -S \"MaxDepth\" -- AyCode.Core/Serializers/Binaries/AcBinarySerializer.ScanPass.cs)", + "Bash(git -C H:/Applications/Aycode/Source/AyCode.Core show ac6e66f^:AyCode.Core/Serializers/Binaries/AcBinarySerializer.cs)" ] } } diff --git a/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.cs b/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.cs index 33aba58..942b16d 100644 --- a/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.cs +++ b/AyCode.Core.Serializers.SourceGenerator/AcBinarySourceGenerator.cs @@ -544,13 +544,14 @@ public class AcBinarySourceGenerator : IIncrementalGenerator sb.AppendLine(); sb.AppendLine(" public void WriteProperties(object value, AcBinarySerializer.BinarySerializationContext context) where TOutput : struct, IBinaryOutputBase"); sb.AppendLine(" {"); - sb.AppendLine(" // Global recursion depth safety net — only when ReferenceHandling != All"); - sb.AppendLine(" // (HasAllRefHandling=true tracks every type → write plan already prevents cycles via ObjectRef)."); - sb.AppendLine(" var needsDepthCheck = !context.HasAllRefHandling;"); + sb.AppendLine(" // Global recursion depth safety net — gated by context.NeedsDepthCheck"); + sb.AppendLine(" // (pre-computed at Reset(): !HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable)."); + sb.AppendLine(" // Local-cached flag: 1 ctx field-read, register-resident — re-used at inc and dec."); + sb.AppendLine(" var needsDepthCheck = context.NeedsDepthCheck;"); sb.AppendLine(" if (needsDepthCheck)"); sb.AppendLine(" {"); sb.AppendLine(" if (context.RecursionDepth >= context.MaxDepth)"); - sb.AppendLine($" throw new System.InvalidOperationException(\"AcBinary serialize: recursion depth exceeded MaxDepth=\" + context.MaxDepth + \" at {ci.FullTypeName}\");"); + sb.AppendLine($" throw new System.InvalidOperationException(\"AcBinary serialize: recursion depth exceeded MaxDepth=\" + context.MaxDepth + \" at type '{ci.FullTypeName}' (depth=\" + context.RecursionDepth + \", position=\" + context.Position + \")\");"); sb.AppendLine(" context.RecursionDepth++;"); sb.AppendLine(" }"); sb.AppendLine(); @@ -645,7 +646,7 @@ public class AcBinarySourceGenerator : IIncrementalGenerator sb.AppendLine(" }"); sb.AppendLine(" }"); } - else if (ci.EnableRefHandling) + if (ci.EnableRefHandling && !ci.IsIId) { // Non-IId type: track via wrapper.TryTrackInt32 with RuntimeHelpers.GetHashCode sb.AppendLine(); @@ -676,16 +677,17 @@ public class AcBinarySourceGenerator : IIncrementalGenerator (p.TypeKind == PropertyTypeKind.Collection && p.ElementKind == PropertyTypeKind.String && p.InterningFlags != 0) || (p.TypeKind == PropertyTypeKind.Dictionary && (p.DictKeyKind == PropertyTypeKind.String || p.DictValueKind == PropertyTypeKind.String) && p.InterningFlags != 0)); - // Global recursion depth safety net — only when ReferenceHandling != All - // (HasAllRefHandling=true tracks every type → 2nd-occurrence early-return already prevents cycles). + // Global recursion depth safety net — gated by context.NeedsDepthCheck + // (pre-computed at Reset(): !HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable). // Emitted AFTER all early returns (NeedsScan=false, feature-flag, null guard, IId 2nd-occurrence) // and BEFORE the property scan loop that recurses into children. + // Local-cached flag: 1 ctx field-read, register-resident — re-used at inc and dec (JIT-independent guarantee). sb.AppendLine(); - sb.AppendLine(" var needsDepthCheck = !context.HasAllRefHandling;"); + sb.AppendLine(" var needsDepthCheck = context.NeedsDepthCheck;"); sb.AppendLine(" if (needsDepthCheck)"); sb.AppendLine(" {"); sb.AppendLine(" if (context.RecursionDepth >= context.MaxDepth)"); - sb.AppendLine($" throw new System.InvalidOperationException(\"AcBinary scan: recursion depth exceeded MaxDepth=\" + context.MaxDepth + \" at {ci.FullTypeName}\");"); + sb.AppendLine($" throw new System.InvalidOperationException(\"AcBinary scan: recursion depth exceeded MaxDepth=\" + context.MaxDepth + \" at type '{ci.FullTypeName}' (depth=\" + context.RecursionDepth + \")\");"); sb.AppendLine(" context.RecursionDepth++;"); sb.AppendLine(" }"); diff --git a/AyCode.Core.Tests/Serialization/AcBinarySerializerIIdReferenceTests.cs b/AyCode.Core.Tests/Serialization/AcBinarySerializerIIdReferenceTests.cs index 48d246f..fd7d548 100644 --- a/AyCode.Core.Tests/Serialization/AcBinarySerializerIIdReferenceTests.cs +++ b/AyCode.Core.Tests/Serialization/AcBinarySerializerIIdReferenceTests.cs @@ -131,6 +131,19 @@ public class AcBinarySerializerIIdReferenceTests Console.WriteLine($"\n========== ReferenceHandling: {options.ReferenceHandling}, UseSgen: {options.UseGeneratedCode}, UseMeta: {options.UseMetadata} =========="); + // ReferenceHandling.None: cycle (Items[1].ParentOrder = order) is unprotected because no ref tracking. + // The global recursion depth safety net must throw before stack overflow (MaxDepth=10). + // Previous silent-null truncation (ACCORE-BIN-I-P2H8) caused data loss instead of surfacing the bug. + if (mode == ReferenceHandlingMode.None) + { + var ex = Assert.ThrowsExactly(() => AcBinarySerializer.Serialize(order, options), $"[{mode}] Expected InvalidOperationException for unprotected cycle, but Serialize succeeded"); + + StringAssert.Contains(ex.Message, "recursion depth exceeded", $"[{mode}] Exception message should mention recursion depth, got: {ex.Message}"); + Console.WriteLine($"[{mode}] PASSED ✓ (depth safety net threw as expected: {ex.Message})"); + + continue; + } + // Act var binary = AcBinarySerializer.Serialize(order, options); //WriteBinaryToConsole(binary); @@ -147,15 +160,6 @@ public class AcBinarySerializerIIdReferenceTests // Assert based on mode switch (mode) { - case ReferenceHandlingMode.None: - //none esetén miért nincs infinite loop??? - J. - // Note: CountObjectRefs raw byte scan is unreliable in None mode — - // byte 65 (ObjectRef) == ASCII 'A', so "Product-A" and circular-ref - // depth expansion produce many false positives. Skip count assertion; - // data integrity checks below verify correct deserialization. - //WriteBinaryToConsole(binary); - break; - case ReferenceHandlingMode.OnlyId: // sharedTag (Id=1) 4x → 3 ObjectRefs, sharedUser (Id=1) 2x → 1 ObjectRef = 4 total Assert.IsTrue(objectRefCount >= 4, $"[{mode}] Expected at least 4 ObjectRefs, found {objectRefCount}"); diff --git a/AyCode.Core/Serializers/AcSerializerOptions.cs b/AyCode.Core/Serializers/AcSerializerOptions.cs index 9e0e1d8..f413c75 100644 --- a/AyCode.Core/Serializers/AcSerializerOptions.cs +++ b/AyCode.Core/Serializers/AcSerializerOptions.cs @@ -40,7 +40,7 @@ public abstract class AcSerializerOptions /// /// Maximum depth for serialization/deserialization. - /// 0 = root level only (primitives of root object) + /// 0 = depth check disabled (equivalent to ) /// 1 = root + first level of nested objects/collections /// byte.MaxValue (255) = effectively unlimited /// Default: byte.MaxValue @@ -51,6 +51,15 @@ public abstract class AcSerializerOptions init => _maxDepth = value; } + /// + /// Action taken when serialization recursion depth reaches . + /// Default: (cycle detection / bug surfacing). + /// Switch to for intentional shallow serialization + /// (client → server delta updates), or for + /// max-perf when the graph is known cycle-free. + /// + public MaxDepthBehavior MaxDepthBehavior { get; init; } = MaxDepthBehavior.Throw; + /// /// Throw exception on circular reference detection for non-IId types. /// When true: Tracks all objects and throws InvalidOperationException on circular references. @@ -92,6 +101,41 @@ public enum AcSerializerType : byte Toon = 2, } +/// +/// Behavior when serialization recursion depth reaches . +/// Controls whether the over-depth slice of the object graph is silently truncated, surfaces as +/// an exception, or whether the entire depth check is skipped for maximum throughput. +/// +public enum MaxDepthBehavior : byte +{ + /// + /// Replace the over-depth value with a Null marker in the wire stream — intentional shallow + /// serialization. Use when sending partial graphs (e.g., client → server delta updates, view-model + /// projections) where deep references are intentionally cut. The receiver reads null at the + /// truncation boundary: at the wire level this is indistinguishable from a genuine null, so + /// the consumer must encode that semantic in its protocol (e.g., "only modified properties are + /// persisted; nested nulls are skipped, not overwritten"). + /// + Truncate = 0, + + /// + /// Throw when depth limit is reached — cycle + /// detection and bug surfacing. Recommended default: an over-depth payload is almost always a bug + /// (circular reference without proper tracking, or + /// pathologically deep graph). The exception message reports the offending type and depth so the + /// failure mode is debuggable rather than silent. + /// + Throw = 1, + + /// + /// Skip the depth limit check entirely — maximum hot-path performance. Use only when the developer + /// guarantees the object graph is cycle-free and structurally bounded; otherwise the serializer + /// can stack-overflow on pathological input. Equivalent to setting =0 + /// but expresses intent at the option level instead of via a magic depth value. + /// + Disable = 2, +} + /// /// Reference handling mode for serialization. /// diff --git a/AyCode.Core/Serializers/Binaries/AcBinarySerializer.BinarySerializationContext.cs b/AyCode.Core/Serializers/Binaries/AcBinarySerializer.BinarySerializationContext.cs index 4aef045..bc78833 100644 --- a/AyCode.Core/Serializers/Binaries/AcBinarySerializer.BinarySerializationContext.cs +++ b/AyCode.Core/Serializers/Binaries/AcBinarySerializer.BinarySerializationContext.cs @@ -90,10 +90,31 @@ public static partial class AcBinarySerializer /// Global recursion depth counter — final safety net against pathological graphs and non-IId cycles. /// Incremented/decremented at object-recursion entry/exit points (WriteObject runtime + generated WriteProperties/ScanObject). /// Checked against (byte, default 255). - /// Replaces the per-call int depth parameter passing — one byte field on context, ~5-7 cycles per object instead of per call. + /// Gated by — single context field-read at each call site. /// internal byte RecursionDepth; + /// + /// Pre-computed depth-check gate, set at : + /// !HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable. + /// + /// HasAllRefHandling=true: every type tracked → 2nd-occurrence early-return prevents cycles → check redundant + /// MaxDepth=0: explicit developer opt-out (magic value) — disables the safety net entirely + /// MaxDepthBehavior=Disable: explicit developer opt-out (intent-level) — same effect, expressed via the behavior enum + /// + /// All three opt-outs collapse to NeedsDepthCheck=false so the hot path is a single field-read + + /// register-test, no per-call enum compare or negation. Truncate vs Throw dispatch (when implemented for + /// Truncate placement) consults on the rare depth-limit-hit path only. + /// + internal bool NeedsDepthCheck; + + /// + /// Pre-computed action when reaches . + /// Mirrors ; checked inside the depth-comparison hot path + /// (gated short-circuit: comparison runs first, this enum is consulted only when the depth limit is actually hit). + /// + internal MaxDepthBehavior MaxDepthAction; + #region WriteDuplicateEntry — scan pass output for write pass cursor private WriteDuplicateEntry[]? _writePlan; @@ -299,6 +320,13 @@ public static partial class AcBinarySerializer HasStringInterning = Options.UseStringInterning != StringInterningMode.None; FastWire = Options.WireMode == WireMode.Fast; + + // Pre-compute depth-check gate. Three opt-out paths collapsed here: + // 1. ReferenceHandling=All → every type tracked, per-type 2nd-occurrence prevents cycles + // 2. MaxDepth=0 → explicit developer opt-out (magic value) + // 3. MaxDepthBehavior=Disable → explicit developer opt-out (intent-level enum) + MaxDepthAction = Options.MaxDepthBehavior; + NeedsDepthCheck = !HasAllRefHandling && Options.MaxDepth > 0 && MaxDepthAction != MaxDepthBehavior.Disable; } public override void Clear() diff --git a/AyCode.Core/Serializers/Binaries/AcBinarySerializer.ScanPass.cs b/AyCode.Core/Serializers/Binaries/AcBinarySerializer.ScanPass.cs index 164c4c2..e5de300 100644 --- a/AyCode.Core/Serializers/Binaries/AcBinarySerializer.ScanPass.cs +++ b/AyCode.Core/Serializers/Binaries/AcBinarySerializer.ScanPass.cs @@ -156,12 +156,15 @@ public static partial class AcBinarySerializer } // Fallback: runtime property loop (no SGen writer for this type) - // Global recursion depth safety net — only when ReferenceHandling != All - // (HasAllRefHandling=true tracks every type → 2nd-occurrence early-return above already prevents cycles). - var needsDepthCheck = !context.HasAllRefHandling; + // Global recursion depth safety net — gated by context.NeedsDepthCheck + // (pre-computed at Reset(): !HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable). + // Local-cached flag: 1 ctx field-read, register-resident — re-used at inc and dec. + var needsDepthCheck = context.NeedsDepthCheck; if (needsDepthCheck) { - if (context.RecursionDepth >= context.MaxDepth) throw new InvalidOperationException($"AcBinary scan: recursion depth exceeded MaxDepth={context.MaxDepth}"); + if (context.RecursionDepth >= context.MaxDepth) + throw new InvalidOperationException( + $"AcBinary scan: recursion depth exceeded MaxDepth={context.MaxDepth} at type '{wrapper.Metadata.SourceType.FullName}' (depth={context.RecursionDepth})"); context.RecursionDepth++; } diff --git a/AyCode.Core/Serializers/Binaries/AcBinarySerializer.cs b/AyCode.Core/Serializers/Binaries/AcBinarySerializer.cs index 206c5c0..a2245a0 100644 --- a/AyCode.Core/Serializers/Binaries/AcBinarySerializer.cs +++ b/AyCode.Core/Serializers/Binaries/AcBinarySerializer.cs @@ -1611,18 +1611,21 @@ public static partial class AcBinarySerializer var generatedWriter = wrapper.GeneratedWriter; if (generatedWriter != null) { - // SGen path handles its own RecursionDepth inc/dec via generated emit (gated on !HasAllRefHandling) + // SGen path handles its own RecursionDepth inc/dec via generated emit (gated by NeedsDepthCheck) generatedWriter.WriteProperties(value, context); return; } } - // Runtime path: global recursion depth safety net — only when ReferenceHandling != All - // (HasAllRefHandling=true tracks every type → write plan already prevents cycles via ObjectRef). - var needsDepthCheck = !context.HasAllRefHandling; + // Runtime path: global recursion depth safety net — gated by context.NeedsDepthCheck + // (pre-computed at Reset(): !HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable). + // Local-cached flag: 1 ctx field-read, register-resident — re-used at inc and dec. + var needsDepthCheck = context.NeedsDepthCheck; if (needsDepthCheck) { - if (context.RecursionDepth >= context.MaxDepth) throw new InvalidOperationException($"AcBinary serialize: recursion depth exceeded MaxDepth={context.MaxDepth}"); + if (context.RecursionDepth >= context.MaxDepth) + throw new InvalidOperationException($"AcBinary serialize: recursion depth exceeded MaxDepth={context.MaxDepth} at type '{wrapper.Metadata.SourceType.FullName}' (depth={context.RecursionDepth}, position={context.Position})"); + context.RecursionDepth++; }