Mitigate the usage of JsonConvert.DefaultSettings in user code

src/Microsoft.TestPlatform.CommunicationUtilities/JsonDataSerializer.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Diagnostics.CodeAnalysis;
+using System.Globalization;
 using System.IO;
 
 using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
@@ -25,19 +26,37 @@ public class JsonDataSerializer : IDataSerializer
 
     private static readonly JsonSerializer PayloadSerializerV1; // payload serializer for version <= 1
     private static readonly JsonSerializer PayloadSerializerV2; // payload serializer for version >= 2
+    private static readonly JsonSerializer FastSerializer;
     private static readonly JsonSerializerSettings FastJsonSettings; // serializer settings for faster json
     private static readonly JsonSerializerSettings JsonSettings; // serializer settings for serializer v1, which should use to deserialize message headers
     private static readonly JsonSerializer Serializer; // generic serializer
 
     static JsonDataSerializer()
     {
+
         var jsonSettings = new JsonSerializerSettings
         {
             DateFormatHandling = DateFormatHandling.IsoDateFormat,
             DateParseHandling = DateParseHandling.DateTimeOffset,
             DateTimeZoneHandling = DateTimeZoneHandling.Utc,
             TypeNameHandling = TypeNameHandling.None,
             ReferenceLoopHandling = ReferenceLoopHandling.Ignore,
+            MissingMemberHandling = MissingMemberHandling.Ignore,
+            NullValueHandling = NullValueHandling.Include,
+            DefaultValueHandling = DefaultValueHandling.Include,
+            ObjectCreationHandling = ObjectCreationHandling.Auto,
+            PreserveReferencesHandling = PreserveReferencesHandling.None,
+            ConstructorHandling = ConstructorHandling.Default,
+            MetadataPropertyHandling = MetadataPropertyHandling.Default,
+            Formatting = Formatting.None,
+            FloatParseHandling = FloatParseHandling.Double,
+            FloatFormatHandling = FloatFormatHandling.String,
+            StringEscapeHandling = StringEscapeHandling.Default,
+            TypeNameAssemblyFormatHandling = TypeNameAssemblyFormatHandling.Simple,
+            Culture = CultureInfo.InvariantCulture,
+            CheckAdditionalContent = false,
+            DateFormatString = @"yyyy'-'MM'-'dd'T'HH':'mm':'ss.FFFFFFFK",
+            MaxDepth = 64,
         };
 
         JsonSettings = jsonSettings;
@@ -54,13 +73,29 @@ static JsonDataSerializer()
             DateTimeZoneHandling = jsonSettings.DateTimeZoneHandling,
             TypeNameHandling = jsonSettings.TypeNameHandling,
             ReferenceLoopHandling = jsonSettings.ReferenceLoopHandling,
+            MissingMemberHandling = jsonSettings.MissingMemberHandling,
             // PERF: Null value handling has very small impact on serialization and deserialization. Enabling it does not warrant the risk we run
-            // of changing how our consumers get their data.
-            // NullValueHandling = NullValueHandling.Ignore,
-
+            // of changing how our consumers get their data. so we leave it at the default value
+            NullValueHandling = jsonSettings.NullValueHandling,
+            DefaultValueHandling = jsonSettings.DefaultValueHandling,
+            ObjectCreationHandling = jsonSettings.ObjectCreationHandling,
+            PreserveReferencesHandling = jsonSettings.PreserveReferencesHandling,
+            ConstructorHandling = jsonSettings.ConstructorHandling,
+            MetadataPropertyHandling = jsonSettings.MetadataPropertyHandling,
+            Formatting = jsonSettings.Formatting,
+            FloatParseHandling = jsonSettings.FloatParseHandling,
+            FloatFormatHandling = jsonSettings.FloatFormatHandling,
+            StringEscapeHandling = jsonSettings.StringEscapeHandling,
+            TypeNameAssemblyFormatHandling = jsonSettings.TypeNameAssemblyFormatHandling,
+            Culture = jsonSettings.Culture,
+            CheckAdditionalContent = jsonSettings.CheckAdditionalContent,
+            DateFormatString = jsonSettings.DateFormatString,
+            MaxDepth = jsonSettings.MaxDepth,
             ContractResolver = contractResolver,
         };
 
+        FastSerializer = JsonSerializer.Create(FastJsonSettings);
+
         PayloadSerializerV1.ContractResolver = new TestPlatformContractResolver1();
         PayloadSerializerV2.ContractResolver = contractResolver;
 
@@ -74,6 +109,8 @@ static JsonDataSerializer()
 #endif
     }
 
+
+
     /// <summary>
     /// Prevents a default instance of the <see cref="JsonDataSerializer"/> class from being created.
     /// </summary>
@@ -105,7 +142,7 @@ public Message DeserializeMessage(string rawMessage)
         {
             // PERF: If the fast path fails, deserialize into header object that does not have any Payload. When the message type info
             // is at the start of the message, this is also pretty fast. Again, this won't touch the payload.
-            MessageHeader header = JsonConvert.DeserializeObject<MessageHeader>(rawMessage, JsonSettings)!;
+            MessageHeader header = DeserializeObjectFast<MessageHeader>(rawMessage)!;
             version = header.Version;
             messageType = header.MessageType;
         }
@@ -171,7 +208,7 @@ public Message DeserializeMessage(string rawMessage)
         {
             // PERF: Fast path is compatibile only with protocol versions that use serializer_2,
             // and this is faster than deserializing via deserializer_2.
-            var messageWithPayload = JsonConvert.DeserializeObject<PayloadedMessage<T>>(rawMessage, FastJsonSettings);
+            var messageWithPayload = DeserializeObjectFast<PayloadedMessage<T>>(rawMessage);
 
             return messageWithPayload == null ? default : messageWithPayload.Payload;
         }
@@ -187,6 +224,12 @@ public Message DeserializeMessage(string rawMessage)
         }
     }
 
+    private static T? DeserializeObjectFast<T>(string value)
+    {
+        using JsonTextReader reader = new(new StringReader(value));
+        return (T?)FastSerializer.Deserialize(reader, typeof(T));
+    }
+
     private static bool FastHeaderParse(string rawMessage, out int version, out string? messageType)
     {
         // PERF: This can be also done slightly better using ReadOnlySpan<char> but we don't have that available by default in .NET Framework
@@ -345,7 +388,7 @@ public string SerializePayload(string? messageType, object? payload, int version
         }
         else
         {
-            return JsonConvert.SerializeObject(new VersionedMessageForSerialization { MessageType = messageType, Version = version, Payload = payload }, FastJsonSettings);
+            return Serialize(FastSerializer, new VersionedMessageForSerialization { MessageType = messageType, Version = version, Payload = payload });
         }
     }
 

src/Microsoft.TestPlatform.CoreUtilities/FeatureFlag/FeatureFlag.cs

@@ -51,7 +51,7 @@ internal partial class FeatureFlag : IFeatureFlag
     public const string DISABLE_MULTI_TFM_RUN = VSTEST_ + nameof(DISABLE_MULTI_TFM_RUN);
 
     // Disables setting a higher value for SetMinThreads. Setting SetMinThreads value to higher allows testhost to connect back faster
-    // even though we are blocking additional threads becuase we don't have to wait for ThreadPool to start more threads.
+    // even though we are blocking additional threads because we don't have to wait for ThreadPool to start more threads.
     public const string DISABLE_THREADPOOL_SIZE_INCREASE = VSTEST_ + nameof(DISABLE_THREADPOOL_SIZE_INCREASE);
 
     // Disable the SerialTestRunDecorator

test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/JsonDataSerializerTests.cs

@@ -5,7 +5,9 @@
 using System.Linq;
 
 using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
+using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.ObjectModel;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel;
+using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 using Newtonsoft.Json;
@@ -26,22 +28,40 @@ public JsonDataSerializerTests()
     }
 
     [TestMethod]
-    public void SerializePayloadShouldNotPickDefaultSettings()
+    [DataRow(0)]
+    [DataRow(1)]
+    [DataRow(2)]
+    [DataRow(3)]
+    [DataRow(4)]
+    [DataRow(5)]
+    [DataRow(6)]
+    [DataRow(7)]
+    public void SerializePayloadShouldNotPickDefaultSettings(int version)
     {
         JsonConvert.DefaultSettings = () => new JsonSerializerSettings
         {
             ContractResolver = new DefaultContractResolver
             {
                 NamingStrategy = new SnakeCaseNamingStrategy()
-            }
+            },
+            PreserveReferencesHandling = PreserveReferencesHandling.All,
         };
 
         var classWithSelfReferencingLoop = new ClassWithSelfReferencingLoop(null);
         classWithSelfReferencingLoop = new ClassWithSelfReferencingLoop(classWithSelfReferencingLoop);
         classWithSelfReferencingLoop.InfiniteRefernce!.InfiniteRefernce = classWithSelfReferencingLoop;
 
-        string serializedPayload = _jsonDataSerializer.SerializePayload("dummy", classWithSelfReferencingLoop);
-        Assert.AreEqual("{\"MessageType\":\"dummy\",\"Payload\":{\"InfiniteRefernce\":{}}}", serializedPayload);
+        string serializedPayload = _jsonDataSerializer.SerializePayload("dummy", classWithSelfReferencingLoop, version);
+        if (version <= 1)
+        {
+            Assert.AreEqual("{\"MessageType\":\"dummy\",\"Payload\":{\"InfiniteRefernce\":{}}}", serializedPayload);
+        }
+        else
+        {
+            Assert.AreEqual($"{{\"Version\":{version},\"MessageType\":\"dummy\",\"Payload\":{{\"InfiniteRefernce\":{{}}}}}}", serializedPayload);
+        }
+
+        JsonConvert.DefaultSettings = null;
     }
 
     [TestMethod]
@@ -52,13 +72,79 @@ public void DeserializeMessageShouldNotPickDefaultSettings()
             ContractResolver = new DefaultContractResolver
             {
                 NamingStrategy = new SnakeCaseNamingStrategy()
-            }
+            },
+            PreserveReferencesHandling = PreserveReferencesHandling.All,
         };
 
         Message message = _jsonDataSerializer.DeserializeMessage("{\"MessageType\":\"dummy\",\"Payload\":{\"InfiniteRefernce\":{}}}");
         Assert.AreEqual("dummy", message?.MessageType);
+        JsonConvert.DefaultSettings = null;
     }
 
+
+    [TestMethod]
+    [DataRow(0)]
+    [DataRow(1)]
+    [DataRow(2)]
+    [DataRow(3)]
+    [DataRow(4)]
+    [DataRow(5)]
+    [DataRow(6)]
+    [DataRow(7)]
+
+    public void SerializePayloadIsUnaffectedByJsonConverterDefaultSettings(int version)
+    {
+
+        Assert.IsNull(JsonConvert.DefaultSettings);
+        //todo: how to check feature flag
+        var completeArgs = new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero);
+        var payload = new TestRunCompletePayload { TestRunCompleteArgs = completeArgs };
+
+        JsonConvert.DefaultSettings = () =>
+        {
+            //restore the default settings to null
+            JsonConvert.DefaultSettings = null;
+            Assert.Fail("Should Not Access DefaultSettings");
+            return new();
+        };
+
+        var withDefaultSettingUpdated = JsonDataSerializer.Instance.SerializePayload(MessageType.ExecutionComplete, payload, version);
+
+        //restore the default settings to null
+        JsonConvert.DefaultSettings = null;
+    }
+
+    [TestMethod]
+    [DataRow(0)]
+    [DataRow(1)]
+    [DataRow(2)]
+    [DataRow(3)]
+    [DataRow(4)]
+    [DataRow(5)]
+    [DataRow(6)]
+    [DataRow(7)]
+    public void DeserializePayloadIsUnaffectedByJsonConverterDefaultSettings(int version)
+    {
+
+        Assert.IsNull(JsonConvert.DefaultSettings, "If this is not null some other test didn't clean up its default setti0ngs");
+
+        JsonConvert.DefaultSettings = () =>
+        {
+            //restore the default settings to null
+            JsonConvert.DefaultSettings = null;
+            Assert.Fail("Should Not Access DefaultSettings");
+            return new();
+        };
+
+        // This line should deserialize properly
+        Message message = _jsonDataSerializer.DeserializeMessage($"{{\"Version\":\"{version}\",\"MessageType\":\"dummy\",\"Payload\":{{\"InfiniteRefernce\":{{}}}}}}");
+
+
+        //restore the default settings to null
+        JsonConvert.DefaultSettings = null;
+    }
+
+
     [TestMethod]
     public void SerializePayloadShouldSerializeAnObjectWithSelfReferencingLoop()
     {