Merge pull request #452 from nblumhardt/suppress-host-deserialization-errors

Santize trace and span ids to avoid deserialization errors when hosting plug-in apps

Author: nblumhardt

src/SeqCli/Apps/Hosting/AppContainer.cs

@@ -15,6 +15,7 @@
 using System;
 using System.Globalization;
 using System.IO;
+using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
@@ -29,11 +30,13 @@
 
 namespace SeqCli.Apps.Hosting;
 
-class AppContainer : IAppHost, IDisposable
+partial class AppContainer : IAppHost, IDisposable
 {
     readonly SeqApp _seqApp;
     readonly AppLoader _loader;
 
+    static readonly Regex HexDigits = HexDigitsRegex();
+
     readonly JsonSerializer _serializer = JsonSerializer.Create(new JsonSerializerSettings
     {
         DateParseHandling = DateParseHandling.None,
@@ -143,6 +146,8 @@ LogEvent ReadSerilogEvent(string clef, out string eventId, out uint eventType)
             jobject.Add("@l", new JValue(LevelMapping.ToSerilogLevel(levelToken.Value<string>()!).ToString()));
         }
 
+        SanitizeTraceIdentifiers(jobject);
+
         var raw = LogEventReader.ReadFromJObject(jobject);
 
         eventId = "event-0";
@@ -158,6 +163,29 @@ LogEvent ReadSerilogEvent(string clef, out string eventId, out uint eventType)
         return raw;
     }
 
+    internal static void SanitizeTraceIdentifiers(JObject jobject)
+    {
+        // Serilog.Formatting.Compact.Reader constructs LogEvents which use the System.Diagnostics ActivityTraceId
+        // and ActivitySpanId types; these throw when constructed with invalid inputs.
+        
+        if (jobject.TryGetValue("@tr", out var traceIdToken) && !IsValidHexIdentifier(traceIdToken, 32))
+            jobject.Remove("@tr");
+        
+        if (jobject.TryGetValue("@sp", out var spanIdToken) && !IsValidHexIdentifier(spanIdToken, 16))
+            jobject.Remove("@sp");
+        
+        if (jobject.TryGetValue("@ps", out var parentSpanIdToken) && !IsValidHexIdentifier(parentSpanIdToken, 16))
+            jobject.Remove("@ps");
+    }
+
+    static bool IsValidHexIdentifier(JToken? value, int requiredChars)
+    {
+        if (value?.Value<string>() is not { } id)
+            return false;
+
+        return id.Length == requiredChars && HexDigits.IsMatch(id);
+    }
+
     public void StartPublishing(TextWriter inputWriter)
     {
         if (_seqApp is IPublishJson pjson)
@@ -169,4 +197,8 @@ public void StopPublishing()
         if (_seqApp is IPublishJson pjson)
             pjson.Stop();
     }
+
+    // Technically, 
+    [GeneratedRegex("^[0-9a-f]*$")]
+    private static partial Regex HexDigitsRegex();
 }

test/SeqCli.Tests/Apps/AppContainerTests.cs

@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using System.IO;
+using Newtonsoft.Json.Linq;
 using Seq.Apps;
 using SeqCli.Apps.Hosting;
 using SeqCli.Tests.Support;
@@ -37,4 +38,46 @@ public void CanLoadInputApp()
 
         appContainer.Dispose();
     }
+
+    [Fact]
+    public void RemovesInvalidTraceAndSpanIds()
+    {
+        var jo = new JObject
+        {
+            ["@tr"] = "A234567890123456A234567890123456",
+            ["@sp"] = "A234567890123456",
+            ["@ps"] = "A234567890123456"
+        };
+        Assert.NotEmpty(jo);
+        AppContainer.SanitizeTraceIdentifiers(jo);
+        Assert.Empty(jo);
+    }
+
+    [Fact]
+    public void RemovesShortTraceAndSpanIds()
+    {
+        var jo = new JObject
+        {
+            ["@tr"] = "234567890123456234567890123456",
+            ["@sp"] = "234567890123456",
+            ["@ps"] = "234567890123456"
+        };
+        Assert.NotEmpty(jo);
+        AppContainer.SanitizeTraceIdentifiers(jo);
+        Assert.Empty(jo);
+    }
+
+    [Fact]
+    public void PreservesValidTraceAndSpanIds()
+    {
+        var jo = new JObject
+        {
+            ["@tr"] = "a234567890123456a234567890123456",
+            ["@sp"] = "a234567890123456",
+            ["@ps"] = "a234567890123456"
+        };
+        Assert.Equal(3, jo.Count);
+        AppContainer.SanitizeTraceIdentifiers(jo);
+        Assert.Equal(3, jo.Count);
+    }
 }