From 3c8f8b44258260039f2e460d1b6942bb9e403a14 Mon Sep 17 00:00:00 2001
From: Adam Simon <adamosimoni@gmail.com>
Date: Fri, 24 Nov 2023 12:56:20 +0100
Subject: [PATCH] Make deserialization of flag override json files tolerant to
 comments and trailing commas in builds other than .NET 4.5 as well

---
 src/ConfigCat.Client.Tests/OverrideTests.cs   | 63 +++++++++++++++++++
 .../Extensions/SerializationExtensions.cs     | 18 +++---
 .../Override/LocalFileDataSource.cs           |  4 +-
 3 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/src/ConfigCat.Client.Tests/OverrideTests.cs b/src/ConfigCat.Client.Tests/OverrideTests.cs
index 83be4c10..1ba1f9c1 100644
--- a/src/ConfigCat.Client.Tests/OverrideTests.cs
+++ b/src/ConfigCat.Client.Tests/OverrideTests.cs
@@ -1,9 +1,11 @@
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
 
 #if USE_NEWTONSOFT_JSON
 using JsonValue = Newtonsoft.Json.Linq.JValue;
@@ -473,6 +475,67 @@ public async Task LocalFile_Watcher_Reload_Sync()
         File.Delete(SampleFileToCreate);
     }
 
+    [TestMethod]
+    public async Task LocalFile_TolerantJsonParsing_SimplifiedConfig()
+    {
+        const string key = "flag";
+        const bool expectedEvaluatedValue = true;
+        var overrideValue = expectedEvaluatedValue.ToString(CultureInfo.InvariantCulture).ToLowerInvariant();
+
+        var filePath = Path.GetTempFileName();
+        File.WriteAllText(filePath, $"{{ \"flags\": {{ \"{key}\": {overrideValue} }}, /* comment */ }}");
+
+        try
+        {
+            using var client = ConfigCatClient.Get("localhost", options =>
+            {
+                options.PollingMode = PollingModes.ManualPoll;
+                options.FlagOverrides = FlagOverrides.LocalFile(filePath, autoReload: false, OverrideBehaviour.LocalOnly);
+            });
+            var actualEvaluatedValue = await client.GetValueAsync<bool?>(key, null);
+
+            Assert.AreEqual(expectedEvaluatedValue, actualEvaluatedValue);
+        }
+        finally
+        {
+            if (File.Exists(filePath))
+            {
+                File.Delete(filePath);
+            }
+        }
+    }
+
+    [TestMethod]
+    public async Task LocalFile_TolerantJsonParsing_ComplexConfig()
+    {
+        const string key = "flag";
+        const bool expectedEvaluatedValue = true;
+        var overrideValue = expectedEvaluatedValue.ToString(CultureInfo.InvariantCulture).ToLowerInvariant();
+        var settingType = ((int)SettingType.Boolean).ToString(CultureInfo.InvariantCulture);
+
+        var filePath = Path.GetTempFileName();
+        File.WriteAllText(filePath, $"{{ \"f\": {{ \"{key}\": {{ \"t\": {settingType}, \"v\": {{ \"b\": {overrideValue} }} }} }}, /* comment */ }}");
+
+        try
+        {
+            using var client = ConfigCatClient.Get("localhost", options =>
+            {
+                options.PollingMode = PollingModes.ManualPoll;
+                options.FlagOverrides = FlagOverrides.LocalFile(filePath, autoReload: false, OverrideBehaviour.LocalOnly);
+            });
+            var actualEvaluatedValue = await client.GetValueAsync<bool?>(key, null);
+
+            Assert.AreEqual(expectedEvaluatedValue, actualEvaluatedValue);
+        }
+        finally
+        {
+            if (File.Exists(filePath))
+            {
+                File.Delete(filePath);
+            }
+        }
+    }
+
     [DataRow(true, false, true)]
     [DataRow(true, "", "")]
     [DataRow(true, 0, 0)]
diff --git a/src/ConfigCatClient/Extensions/SerializationExtensions.cs b/src/ConfigCatClient/Extensions/SerializationExtensions.cs
index 2f3d3e86..9c483ccb 100644
--- a/src/ConfigCatClient/Extensions/SerializationExtensions.cs
+++ b/src/ConfigCatClient/Extensions/SerializationExtensions.cs
@@ -14,34 +14,36 @@ internal static class SerializationExtensions
 #if USE_NEWTONSOFT_JSON
     private static readonly JsonSerializer Serializer = JsonSerializer.Create();
 #else
-    private static readonly JsonSerializerOptions SerializerOptions = new()
+    private static readonly JsonSerializerOptions TolerantSerializerOptions = new()
     {
+        AllowTrailingCommas = true,
+        ReadCommentHandling = JsonCommentHandling.Skip,
         Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping
     };
 #endif
 
-    public static T? Deserialize<T>(this string json) => json.AsMemory().Deserialize<T>();
+    public static T? Deserialize<T>(this string json, bool tolerant = false) => json.AsMemory().Deserialize<T>(tolerant);
 
     // NOTE: It would be better to use ReadOnlySpan<char>, however when the full string is wrapped in a span, json.ToString() result in a copy of the string.
     // This is not the case with ReadOnlyMemory<char>, so we use that until support for .NET 4.5 support is dropped.
-    public static T? Deserialize<T>(this ReadOnlyMemory<char> json)
+    public static T? Deserialize<T>(this ReadOnlyMemory<char> json, bool tolerant = false)
     {
 #if USE_NEWTONSOFT_JSON
         using var stringReader = new StringReader(json.ToString());
         using var reader = new JsonTextReader(stringReader);
         return Serializer.Deserialize<T>(reader);
 #else
-        return JsonSerializer.Deserialize<T>(json.Span);
+        return JsonSerializer.Deserialize<T>(json.Span, tolerant ? TolerantSerializerOptions : null);
 #endif
     }
 
-    public static T? DeserializeOrDefault<T>(this string json) => json.AsMemory().DeserializeOrDefault<T>();
+    public static T? DeserializeOrDefault<T>(this string json, bool tolerant = false) => json.AsMemory().DeserializeOrDefault<T>(tolerant);
 
-    public static T? DeserializeOrDefault<T>(this ReadOnlyMemory<char> json)
+    public static T? DeserializeOrDefault<T>(this ReadOnlyMemory<char> json, bool tolerant = false)
     {
         try
         {
-            return json.Deserialize<T>();
+            return json.Deserialize<T>(tolerant);
         }
         catch
         {
@@ -54,7 +56,7 @@ public static string Serialize<T>(this T objectToSerialize)
 #if USE_NEWTONSOFT_JSON
         return JsonConvert.SerializeObject(objectToSerialize);
 #else
-        return JsonSerializer.Serialize(objectToSerialize, SerializerOptions);
+        return JsonSerializer.Serialize(objectToSerialize, TolerantSerializerOptions);
 #endif
     }
 }
diff --git a/src/ConfigCatClient/Override/LocalFileDataSource.cs b/src/ConfigCatClient/Override/LocalFileDataSource.cs
index 33f3bb6e..e089175c 100644
--- a/src/ConfigCatClient/Override/LocalFileDataSource.cs
+++ b/src/ConfigCatClient/Override/LocalFileDataSource.cs
@@ -104,14 +104,14 @@ private async Task ReloadFileAsync(bool isAsync, CancellationToken cancellationT
                 try
                 {
                     var content = File.ReadAllText(this.fullPath);
-                    var simplified = content.DeserializeOrDefault<SimplifiedConfig>();
+                    var simplified = content.DeserializeOrDefault<SimplifiedConfig>(tolerant: true);
                     if (simplified?.Entries is not null)
                     {
                         this.overrideValues = simplified.Entries.ToDictionary(kv => kv.Key, kv => kv.Value.ToSetting());
                         break;
                     }
 
-                    var deserialized = content.Deserialize<Config>()
+                    var deserialized = content.Deserialize<Config>(tolerant: true)
                         ?? throw new InvalidOperationException("Invalid config JSON content: " + content);
                     this.overrideValues = deserialized.Settings;
                     break;