From deddaad4216728fda18808258ead36396b55cf75 Mon Sep 17 00:00:00 2001 From: ImmutableJeffrey Date: Fri, 24 Apr 2026 11:50:56 +1000 Subject: [PATCH 1/4] fix(audience): guard Json.Serialize against cyclic / pathologically deep dicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defences against StackOverflowException (uncatchable in .NET). - MaxDepth = 64 — WriteObject/WriteArray throw FormatException past the cap rather than recursing. - ReferenceEquality visited-set — self-referential or shared- instance containers throw on re-entry. Sibling diamonds (same dict under two independent keys) are NOT cycles; enforced by visited.Remove after the recursive write returns. HashSet is allocated lazily, so scalar-only payloads pay nothing. No behaviour change for any acyclic, shallow caller. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Packages/Audience/Runtime/Utility/Json.cs | 58 +++++++++++++++---- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/src/Packages/Audience/Runtime/Utility/Json.cs b/src/Packages/Audience/Runtime/Utility/Json.cs index 7868a0279..5b0a2999d 100644 --- a/src/Packages/Audience/Runtime/Utility/Json.cs +++ b/src/Packages/Audience/Runtime/Utility/Json.cs @@ -1,3 +1,4 @@ +using System; using System.Collections; using System.Collections.Generic; using System.Globalization; @@ -7,10 +8,14 @@ namespace Immutable.Audience { internal static class Json { + // Depth cap so a pathological input throws FormatException + // instead of blowing the stack (StackOverflow is uncatchable). + internal const int MaxDepth = 64; + internal static string Serialize(Dictionary data) { var sb = new StringBuilder(); - WriteObject(sb, data, indent: 0, depth: 0); + WriteObject(sb, data, indent: 0, depth: 0, visited: null); return sb.ToString(); } @@ -22,11 +27,11 @@ internal static string Serialize(Dictionary data, int indent) { if (indent <= 0) return Serialize(data); var sb = new StringBuilder(); - WriteObject(sb, data, indent, depth: 0); + WriteObject(sb, data, indent, depth: 0, visited: null); return sb.ToString(); } - private static void WriteValue(StringBuilder sb, object value, int indent, int depth) + private static void WriteValue(StringBuilder sb, object value, int indent, int depth, HashSet visited) { if (value == null) { @@ -68,11 +73,11 @@ private static void WriteValue(StringBuilder sb, object value, int indent, int d } else if (value is Dictionary dict) { - WriteObject(sb, dict, indent, depth); + WriteObject(sb, dict, indent, depth, visited); } else if (value is IList list) { - WriteArray(sb, list, indent, depth); + WriteArray(sb, list, indent, depth, visited); } else { @@ -80,10 +85,13 @@ private static void WriteValue(StringBuilder sb, object value, int indent, int d } } - private static void WriteObject(StringBuilder sb, Dictionary dict, int indent, int depth) + private static void WriteObject(StringBuilder sb, Dictionary dict, int indent, int depth, HashSet visited) { + GuardDepth(depth); + visited = EnterContainer(dict, visited); + sb.Append('{'); - if (dict.Count == 0) { sb.Append('}'); return; } + if (dict.Count == 0) { sb.Append('}'); visited.Remove(dict); return; } var pretty = indent > 0; var first = true; @@ -94,26 +102,31 @@ private static void WriteObject(StringBuilder sb, Dictionary dic if (pretty) AppendNewline(sb, indent, depth + 1); WriteString(sb, kvp.Key); sb.Append(pretty ? ": " : ":"); - WriteValue(sb, kvp.Value, indent, depth + 1); + WriteValue(sb, kvp.Value, indent, depth + 1, visited); } if (pretty) AppendNewline(sb, indent, depth); sb.Append('}'); + visited.Remove(dict); } - private static void WriteArray(StringBuilder sb, IList list, int indent, int depth) + private static void WriteArray(StringBuilder sb, IList list, int indent, int depth, HashSet visited) { + GuardDepth(depth); + visited = EnterContainer(list, visited); + sb.Append('['); - if (list.Count == 0) { sb.Append(']'); return; } + if (list.Count == 0) { sb.Append(']'); visited.Remove(list); return; } var pretty = indent > 0; for (var i = 0; i < list.Count; i++) { if (i > 0) sb.Append(','); if (pretty) AppendNewline(sb, indent, depth + 1); - WriteValue(sb, list[i], indent, depth + 1); + WriteValue(sb, list[i], indent, depth + 1, visited); } if (pretty) AppendNewline(sb, indent, depth); sb.Append(']'); + visited.Remove(list); } private static void AppendNewline(StringBuilder sb, int indent, int depth) @@ -122,6 +135,29 @@ private static void AppendNewline(StringBuilder sb, int indent, int depth) sb.Append(' ', indent * depth); } + private static void GuardDepth(int depth) + { + if (depth >= MaxDepth) + throw new FormatException( + $"JSON nesting exceeds {MaxDepth} levels — refusing to serialize. " + + "Check for a cyclic or excessively deep dictionary/list."); + } + + private static HashSet EnterContainer(object container, HashSet visited) + { + visited ??= new HashSet(ReferenceEqualityComparer.Instance); + if (!visited.Add(container)) + throw new FormatException("JSON graph contains a cycle — refusing to serialize."); + return visited; + } + + private sealed class ReferenceEqualityComparer : IEqualityComparer + { + internal static readonly ReferenceEqualityComparer Instance = new ReferenceEqualityComparer(); + public new bool Equals(object x, object y) => ReferenceEquals(x, y); + public int GetHashCode(object obj) => System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(obj); + } + private static void WriteString(StringBuilder sb, string s) { sb.Append('"'); From 4600052ea0c3977d1ebd67a6d844f1f1cf6d8c10 Mon Sep 17 00:00:00 2001 From: ImmutableJeffrey Date: Fri, 24 Apr 2026 09:13:17 +1000 Subject: [PATCH 2/4] test(audience): pin Json.Serialize depth guard and cycle detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both guards exist because their absence yields StackOverflowException, which the .NET runtime does not catch — the process terminates. Neither guard had a test. Three new tests: - nesting one level past MaxDepth throws FormatException with "nesting exceeds" message. Sabotage: disabling GuardDepth fails the test. - self-referential dict throws FormatException with "cycle" message. Sabotage: disabling the visited.Add(container) check makes the test recurse into the depth guard and the assertion on "cycle" text fails. - shared-child diamond (same dict in two sibling keys) is not treated as a cycle. Sabotage: removing the post-recursion visited.Remove makes the second sibling throw "cycle". Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Tests/Runtime/Utility/JsonTests.cs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/Packages/Audience/Tests/Runtime/Utility/JsonTests.cs b/src/Packages/Audience/Tests/Runtime/Utility/JsonTests.cs index abb9d5446..9ad95adeb 100644 --- a/src/Packages/Audience/Tests/Runtime/Utility/JsonTests.cs +++ b/src/Packages/Audience/Tests/Runtime/Utility/JsonTests.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using NUnit.Framework; @@ -203,5 +204,48 @@ public void Serialize_RealisticEventPayload_ProducesCorrectJson() StringAssert.Contains("\"perfect\":true", result); StringAssert.Contains("\"tags\":[\"fast\",\"clean\"]", result); } + + [Test] + public void Serialize_NestingExceedsMaxDepth_ThrowsFormatException() + { + var root = new Dictionary(); + var current = root; + for (var i = 0; i < Json.MaxDepth; i++) + { + var next = new Dictionary(); + current["next"] = next; + current = next; + } + + var ex = Assert.Throws(() => Json.Serialize(root)); + StringAssert.Contains("nesting exceeds", ex.Message); + } + + [Test] + public void Serialize_SelfReferentialDict_ThrowsFormatException() + { + var root = new Dictionary(); + root["self"] = root; + + var ex = Assert.Throws(() => Json.Serialize(root)); + StringAssert.Contains("cycle", ex.Message); + } + + [Test] + public void Serialize_SharedChildInSiblingKeys_IsNotTreatedAsCycle() + { + // Diamond: visited set tracks the current recursion stack, not all objects ever seen. + var shared = new Dictionary { ["k"] = "v" }; + var root = new Dictionary + { + ["a"] = shared, + ["b"] = shared, + }; + + var result = Json.Serialize(root); + + StringAssert.Contains("\"a\":{\"k\":\"v\"}", result); + StringAssert.Contains("\"b\":{\"k\":\"v\"}", result); + } } } From 7b034e72e83bff3754b00b18c02b40572052a247 Mon Sep 17 00:00:00 2001 From: ImmutableJeffrey Date: Fri, 24 Apr 2026 12:37:20 +1000 Subject: [PATCH 3/4] perf(audience): cache DiskStore.Count to avoid per-tick directory scans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sample diagnostics panels poll QueueSize on a UI tick — at 10 Hz with a non-trivial spool that's a continuous Directory.GetFiles scan for a value that barely changes. Cache the count instead. - Seed at construction from the existing spool so first poll is correct without a pre-mutation branch. Lazy-seeding on first bump had a double-count bug: the seed scan already saw the just-written file, then the +1 delta pushed the cache past the real on-disk count. - Write / Delete / DeleteAll / ApplyAnonymousDowngrade maintain the delta via BumpCount (Interlocked.Add) and TryDelete now returns a bool so "actually removed" vs "already gone" can drive the decrement. - TryDelete folds DirectoryNotFoundException into the true (idempotent) branch — per the MS docs it fires on invalid paths (example: "unmapped drive"), and an unreachable path can't point to a real file — and keeps IOException / UnauthorizedAccessException in the false branch so the cache stays honest when a file survived the delete attempt. FileNotFoundException is deliberately absent: File.Delete silently succeeds when the file is gone, so there is nothing to catch. - Count() reads through Volatile.Read so a caller on a different thread sees the latest published value. Cache invariant: on-disk-count-at-ctor + Σ tracked deltas. Events written outside the DiskStore API (direct File.* calls from tests that plant fixtures) are not tracked and will drift the cache; such tests assert on the post-op filesystem state, not Count(). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Audience/Runtime/Transport/DiskStore.cs | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/Packages/Audience/Runtime/Transport/DiskStore.cs b/src/Packages/Audience/Runtime/Transport/DiskStore.cs index 7aefee056..b407a7f37 100644 --- a/src/Packages/Audience/Runtime/Transport/DiskStore.cs +++ b/src/Packages/Audience/Runtime/Transport/DiskStore.cs @@ -5,6 +5,7 @@ using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; +using System.Threading; namespace Immutable.Audience { @@ -14,10 +15,17 @@ internal sealed class DiskStore { private readonly string _queueDir; + // Cached queue file count: on-disk count at construction, plus + // tracked deltas from Write/Delete. Tests that plant files + // outside the DiskStore API will drift this and should assert + // on filesystem state, not Count(). + private int _cachedCount; + internal DiskStore(string persistentDataPath) { _queueDir = Path.Combine(persistentDataPath, "imtbl_audience", "queue"); Directory.CreateDirectory(_queueDir); + _cachedCount = Directory.GetFiles(_queueDir, "*.json").Length; } // Atomically writes json as a new event file. @@ -29,16 +37,19 @@ internal void Write(string json) File.WriteAllText(tmpPath, json); + var replaced = false; try { File.Move(tmpPath, finalPath); } catch (IOException) { - // Destination already exists (unlikely but safe to handle) File.Delete(finalPath); File.Move(tmpPath, finalPath); + replaced = true; } + + if (!replaced) BumpCount(+1); } // Returns up to maxSize file paths, oldest first. Stale files @@ -71,7 +82,7 @@ internal IReadOnlyList ReadBatch(int maxSize) var fileTime = new DateTime(ticks, DateTimeKind.Utc); if (fileTime < cutoff) { - TryDelete(path); + if (TryDelete(path)) BumpCount(-1); continue; } } @@ -86,17 +97,21 @@ internal IReadOnlyList ReadBatch(int maxSize) internal void Delete(IEnumerable paths) { foreach (var path in paths) - TryDelete(path); + if (TryDelete(path)) BumpCount(-1); } - // Total number of event files currently on disk. - internal int Count() => Directory.GetFiles(_queueDir, "*.json").Length; + // Total number of event files currently on disk. Reads the cached + // count seeded at construction; mutating ops maintain it. + internal int Count() => Volatile.Read(ref _cachedCount); + + private void BumpCount(int delta) => Interlocked.Add(ref _cachedCount, delta); - private static void TryDelete(string path) + private static bool TryDelete(string path) { - try { File.Delete(path); } - catch (IOException) { } - catch (UnauthorizedAccessException) { } + try { File.Delete(path); return true; } + catch (DirectoryNotFoundException) { return true; } + catch (IOException) { return false; } + catch (UnauthorizedAccessException) { return false; } } internal void DeleteAll() { @@ -105,7 +120,7 @@ internal void DeleteAll() catch (DirectoryNotFoundException) { return; } foreach (var path in paths) - TryDelete(path); + if (TryDelete(path)) BumpCount(-1); } // Drops queued identify/alias files, strips userId from track files. @@ -126,13 +141,13 @@ private void ApplyAnonymousDowngradeToFile(string path) !msg.TryGetValue(MessageFields.Type, out var typeObj) || !(typeObj is string type)) { - TryDelete(path); + if (TryDelete(path)) BumpCount(-1); return; } if (IsIdentityMessage(type)) { - TryDelete(path); + if (TryDelete(path)) BumpCount(-1); return; } @@ -176,11 +191,11 @@ private void RewriteTrackWithoutUserId(string path, Dictionary m catch (IOException) { // Delete rather than leave the old userId-bearing payload. - TryDelete(path); + if (TryDelete(path)) BumpCount(-1); } catch (UnauthorizedAccessException) { - TryDelete(path); + if (TryDelete(path)) BumpCount(-1); } } From ba5f256c0ad95d9856f5235347848b0551e89226 Mon Sep 17 00:00:00 2001 From: ImmutableJeffrey Date: Fri, 24 Apr 2026 09:13:25 +1000 Subject: [PATCH 4/4] =?UTF-8?q?refactor(audience):=20rename=20CurrentConse?= =?UTF-8?q?ntForTesting=20=E2=86=92=20CurrentConsent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "ForTesting" suffix signalled a test-only affordance, but the accessor is the canonical internal read of the consent level and is used the same way inside the SDK. Drop the suffix. Internal-only API — no SDK consumers break. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Packages/Audience/Runtime/ImmutableAudience.cs | 2 +- src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Packages/Audience/Runtime/ImmutableAudience.cs b/src/Packages/Audience/Runtime/ImmutableAudience.cs index ca2f1c1cf..0d96df933 100644 --- a/src/Packages/Audience/Runtime/ImmutableAudience.cs +++ b/src/Packages/Audience/Runtime/ImmutableAudience.cs @@ -720,7 +720,7 @@ internal static void ResetState() } } - internal static ConsentLevel CurrentConsentForTesting => _state.Level; + internal static ConsentLevel CurrentConsent => _state.Level; internal static void FlushQueueToDiskForTesting() => _queue?.FlushSync(); diff --git a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs index 287f32d37..8222e5714 100644 --- a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs +++ b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs @@ -677,7 +677,7 @@ public void Init_ConcurrentWithSetConsent_LeavesConsistentState() // early-returns, Init then initialises with Anonymous. // - Init runs first: Init sets Anonymous, SetConsent flips // to None under the lock, consent ends at None. - var finalConsent = ImmutableAudience.CurrentConsentForTesting; + var finalConsent = ImmutableAudience.CurrentConsent; Assert.That(finalConsent, Is.EqualTo(ConsentLevel.None).Or.EqualTo(ConsentLevel.Anonymous), $"iteration {iter}: unexpected final consent {finalConsent}"); @@ -879,7 +879,7 @@ public void SetConsent_PersistsAcrossInit() // Re-init with the *original* (Anonymous) config — persisted Full should win. ImmutableAudience.Init(MakeConfig(ConsentLevel.Anonymous)); - Assert.AreEqual(ConsentLevel.Full, ImmutableAudience.CurrentConsentForTesting, + Assert.AreEqual(ConsentLevel.Full, ImmutableAudience.CurrentConsent, "persisted consent must override the config default after restart"); }