Setting MaxDepth parameter in the JsonSerializerSettings as safe defa…

…ults (#3045) * Add JsonSerializerInitializer * Address comments * Add more test cases
Azure · Jan 6, 2023 · 2267357 · 2267357
1 parent 3e79cdc
commit 2267357
Show file tree

Hide file tree

Showing 15 changed files with 402 additions and 33 deletions.
diff --git a/iothub/device/src/InternalClient.cs b/iothub/device/src/InternalClient.cs
@@ -12,6 +12,7 @@
 using System.Security.Cryptography.X509Certificates;
 using System.IO;
 using Microsoft.Azure.Devices.Client.Exceptions;
+using Newtonsoft.Json;
 
 #if NET451
 
@@ -156,6 +157,8 @@ public InternalClient(IotHubConnectionString iotHubConnectionString, ITransportS
                 Logging.Enter(this, transportSettings, pipelineBuilder, nameof(InternalClient) + "_ctor");
             }
 
+            JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();
+
             TlsVersions.Instance.SetLegacyAcceptableVersions();
 
             IotHubConnectionString = iotHubConnectionString;
@@ -989,7 +992,7 @@ internal async Task OnMethodCalledAsync(MethodRequestInternal methodRequestInter
                     methodResponseInternal?.Dispose();
                 }
                 finally
-                { 
+                {
                     // Need to release this semaphore even if the above dispose call fails
                     _methodsDictionarySemaphore.Release();
                 }
@@ -1405,7 +1408,7 @@ private async Task OnDeviceMessageReceivedAsync(Message message)
                 return;
             }
 
-            // Grab this semaphore so that there is no chance that the _deviceReceiveMessageCallback instance is set in between the read of the 
+            // Grab this semaphore so that there is no chance that the _deviceReceiveMessageCallback instance is set in between the read of the
             // item1 and the read of the item2
             await _deviceReceiveMessageSemaphore.WaitAsync().ConfigureAwait(false);
             ReceiveMessageCallback callback = _deviceReceiveMessageCallback?.Item1;

diff --git a/iothub/device/tests/DeviceClientTests.cs b/iothub/device/tests/DeviceClientTests.cs
@@ -5,10 +5,12 @@
 using Microsoft.Azure.Devices.Client.Transport.Mqtt;
 using Microsoft.Azure.Devices.Shared;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Newtonsoft.Json;
 using NSubstitute;
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
@@ -309,7 +311,6 @@ await innerHandler
         }
 
         [TestMethod]
-        // Tests_SRS_DEVICECLIENT_10_012: [** If the given methodRequestInternal argument is null, fail silently **]**
         public async Task DeviceClient_OnMethodCalled_NullMethodRequest()
         {
             using var deviceClient = DeviceClient.CreateFromConnectionString(fakeConnectionString);
@@ -1565,5 +1566,45 @@ public async Task MessageIdDefaultSetToGuid_SendEventBatchSetMessageIdIfNotSet()
             messageWithoutId.MessageId.Should().NotBeNullOrEmpty();
             messageWithId.MessageId.Should().Be(messageId);
         }
+
+        [TestMethod]
+        public void DeviceClient_DefaultMaxDepth()
+        {
+            // arrange
+            string hostName = "acme.azure-devices.net";
+            var authMethod = new DeviceAuthenticationWithSakRefresh("device1", s_cs);
+
+            using var deviceClient = DeviceClient.Create(hostName, authMethod);
+
+            // above arragement is only for setting the defaultJsonSerializerSettings
+
+            var defaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
+            defaultSettings.MaxDepth.Should().Be(128);
+        }
+
+        [TestMethod]
+        public void DeviceClient_OverrideDefaultMaxDepth_ExceedMaxDepthThrows()
+        {
+            // arrange
+            const string fakeConnectionString = "HostName=acme.azure-devices.net;SharedAccessKeyName=AllAccessKey;DeviceId=fake;SharedAccessKey=dGVzdFN0cmluZzE=";
+            using var deviceClient = DeviceClient.CreateFromConnectionString(fakeConnectionString, TransportType.Http1, new ClientOptions
+            {
+                ModelId = "TestModel"
+            });
+            // above arragement is only for setting the defaultJsonSerializerSettings
+
+            //Create a string representation of a nested object (JSON serialized)
+            int nRep = 3;
+            string json = string.Concat(Enumerable.Repeat("{a:", nRep)) + "1" +
+            string.Concat(Enumerable.Repeat("}", nRep));
+
+            var settings = new JsonSerializerSettings { MaxDepth = 2 };
+            //// deserialize
+            // act
+            Func<object> act = () => JsonConvert.DeserializeObject(json, settings);
+
+            // assert
+            act.Should().Throw<Newtonsoft.Json.JsonReaderException>();
+        }
     }
 }
diff --git a/iothub/device/tests/ModuleClientTests.cs b/iothub/device/tests/ModuleClientTests.cs
@@ -1,7 +1,11 @@
 using System;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using FluentAssertions;
+using Microsoft.Azure.Devices.Shared;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Newtonsoft.Json;
 using NSubstitute;
 
 namespace Microsoft.Azure.Devices.Client.Test
@@ -266,5 +270,37 @@ public void ModuleClient_InvokeMethodAsyncWithoutBodyShouldNotThrow()
             // act
             _ = new MethodInvokeRequest(request.Name, request.DataAsJson, request.ResponseTimeout, request.ConnectionTimeout);
         }
+
+        [TestMethod]
+        public void ModuleClient_DefaultMaxDepth()
+        {
+            // arrange
+            var moduleClient = ModuleClient.CreateFromConnectionString(FakeConnectionString);
+            // above arragement is only for setting the defaultJsonSerializerSettings
+
+            var defaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
+            defaultSettings.MaxDepth.Should().Be(128);
+        }
+
+        [TestMethod]
+        public void ModuleClient_OverrideDefaultMaxDepth_ExceedMaxDepthThrows()
+        {
+            // arrange
+            var moduleClient = ModuleClient.CreateFromConnectionString(FakeConnectionString);
+            // above arragement is only for setting the defaultJsonSerializerSettings
+
+            //Create a string representation of a nested object (JSON serialized)
+            int nRep = 3;
+            string json = string.Concat(Enumerable.Repeat("{a:", nRep)) + "1" +
+            string.Concat(Enumerable.Repeat("}", nRep));
+
+            var settings = new JsonSerializerSettings { MaxDepth = 2 };
+            //// deserialize
+            // act
+            Func<object> act = () => JsonConvert.DeserializeObject(json, settings);
+
+            // assert
+            act.Should().Throw<Newtonsoft.Json.JsonReaderException>();
+        }
     }
 }
diff --git a/iothub/service/src/DigitalTwin/DigitalTwinClient.cs b/iothub/service/src/DigitalTwin/DigitalTwinClient.cs
@@ -8,6 +8,7 @@
 using Microsoft.Azure.Devices.Authentication;
 using Microsoft.Azure.Devices.Extensions;
 using Microsoft.Azure.Devices.Generated;
+using Microsoft.Azure.Devices.Shared;
 using Microsoft.Rest;
 using Newtonsoft.Json;
 
@@ -38,6 +39,8 @@ private DigitalTwinClient(Uri uri, IotServiceClientCredentials credentials, para
         {
             _client = new IotHubGatewayServiceAPIs(uri, credentials, handlers);
             _protocolLayer = new DigitalTwin(_client);
+
+            JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();
         }
 
         /// <summary>
@@ -69,9 +72,9 @@ public async Task<HttpOperationResponse<T, DigitalTwinGetHeaders>> GetDigitalTwi
         /// <param name="cancellationToken">The cancellationToken.</param>
         /// <returns>The http response.</returns>
         public Task<HttpOperationHeaderResponse<DigitalTwinUpdateHeaders>> UpdateDigitalTwinAsync(
-            string digitalTwinId, 
-            string digitalTwinUpdateOperations, 
-            DigitalTwinUpdateRequestOptions requestOptions = default, 
+            string digitalTwinId,
+            string digitalTwinUpdateOperations,
+            DigitalTwinUpdateRequestOptions requestOptions = default,
             CancellationToken cancellationToken = default)
         {
             return _protocolLayer.UpdateDigitalTwinWithHttpMessagesAsync(digitalTwinId, digitalTwinUpdateOperations, requestOptions?.IfMatch, null, cancellationToken);
@@ -87,19 +90,19 @@ public Task<HttpOperationHeaderResponse<DigitalTwinUpdateHeaders>> UpdateDigital
         /// <param name="cancellationToken">The cancellationToken.</param>
         /// <returns>The application/json command invocation response and the http response. </returns>
         public async Task<HttpOperationResponse<DigitalTwinCommandResponse, DigitalTwinInvokeCommandHeaders>> InvokeCommandAsync(
-            string digitalTwinId, 
-            string commandName, 
-            string payload = default, 
-            DigitalTwinInvokeCommandRequestOptions requestOptions = default, 
+            string digitalTwinId,
+            string commandName,
+            string payload = default,
+            DigitalTwinInvokeCommandRequestOptions requestOptions = default,
             CancellationToken cancellationToken = default)
         {
             using HttpOperationResponse<string, DigitalTwinInvokeRootLevelCommandHeaders> response = await _protocolLayer.InvokeRootLevelCommandWithHttpMessagesAsync(
-                digitalTwinId, 
-                commandName, 
-                payload, 
-                requestOptions?.ConnectTimeoutInSeconds, 
-                requestOptions?.ResponseTimeoutInSeconds, 
-                null, 
+                digitalTwinId,
+                commandName,
+                payload,
+                requestOptions?.ConnectTimeoutInSeconds,
+                requestOptions?.ResponseTimeoutInSeconds,
+                null,
                 cancellationToken)
                 .ConfigureAwait(false);
             return new HttpOperationResponse<DigitalTwinCommandResponse, DigitalTwinInvokeCommandHeaders>
@@ -122,21 +125,21 @@ public async Task<HttpOperationResponse<DigitalTwinCommandResponse, DigitalTwinI
         /// <param name="cancellationToken">The cancellationToken.</param>
         /// <returns>The application/json command invocation response and the http response. </returns>
         public async Task<HttpOperationResponse<DigitalTwinCommandResponse, DigitalTwinInvokeCommandHeaders>> InvokeComponentCommandAsync(
-            string digitalTwinId, 
-            string componentName, 
-            string commandName, 
-            string payload = default, 
-            DigitalTwinInvokeCommandRequestOptions requestOptions = default, 
+            string digitalTwinId,
+            string componentName,
+            string commandName,
+            string payload = default,
+            DigitalTwinInvokeCommandRequestOptions requestOptions = default,
             CancellationToken cancellationToken = default)
         {
             using HttpOperationResponse<string, DigitalTwinInvokeComponentCommandHeaders> response = await _protocolLayer.InvokeComponentCommandWithHttpMessagesAsync(
-                digitalTwinId, 
-                componentName, 
-                commandName, 
-                payload, 
-                requestOptions?.ConnectTimeoutInSeconds, 
-                requestOptions?.ResponseTimeoutInSeconds, 
-                null, 
+                digitalTwinId,
+                componentName,
+                commandName,
+                payload,
+                requestOptions?.ConnectTimeoutInSeconds,
+                requestOptions?.ResponseTimeoutInSeconds,
+                null,
                 cancellationToken)
                 .ConfigureAwait(false);
             return new HttpOperationResponse<DigitalTwinCommandResponse, DigitalTwinInvokeCommandHeaders>

diff --git a/iothub/service/src/JobClient/JobClient.cs b/iothub/service/src/JobClient/JobClient.cs
@@ -4,6 +4,7 @@
 namespace Microsoft.Azure.Devices
 {
     using Microsoft.Azure.Devices.Shared;
+    using Newtonsoft.Json;
     using System;
     using System.Threading;
     using System.Threading.Tasks;
@@ -35,6 +36,7 @@ public static JobClient CreateFromConnectionString(string connectionString, Http
             {
                 throw new ArgumentNullException(nameof(transportSettings), "HTTP Transport settings cannot be null.");
             }
+            JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();
             TlsVersions.Instance.SetLegacyAcceptableVersions();
 
             var iotHubConnectionString = IotHubConnectionString.Parse(connectionString);
@@ -52,7 +54,8 @@ public void Dispose()
         /// Releases unmanaged and - optionally - managed resources.
         /// </summary>
         /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
-        protected virtual void Dispose(bool disposing) { }
+        protected virtual void Dispose(bool disposing)
+        { }
 
         /// <summary>
         /// Explicitly open the JobClient instance.

diff --git a/iothub/service/src/RegistryManager.cs b/iothub/service/src/RegistryManager.cs
@@ -6,6 +6,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Azure.Devices.Shared;
+using Newtonsoft.Json;
 
 namespace Microsoft.Azure.Devices
 {
@@ -40,6 +41,8 @@ public static RegistryManager CreateFromConnectionString(string connectionString
             {
                 throw new ArgumentNullException(nameof(transportSettings), "The HTTP transport settings cannot be null.");
             }
+
+            JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();
             TlsVersions.Instance.SetLegacyAcceptableVersions();
 
             var iotHubConnectionString = IotHubConnectionString.Parse(connectionString);
@@ -57,7 +60,8 @@ public void Dispose()
         /// Releases unmanaged and - optionally - managed resources.
         /// </summary>
         /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
-        protected virtual void Dispose(bool disposing) { }
+        protected virtual void Dispose(bool disposing)
+        { }
 
         /// <summary>
         /// Explicitly open the RegistryManager instance.

diff --git a/iothub/service/src/ServiceClient.cs b/iothub/service/src/ServiceClient.cs
@@ -5,6 +5,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Azure.Devices.Shared;
+using Newtonsoft.Json;
 
 namespace Microsoft.Azure.Devices
 {
@@ -39,6 +40,7 @@ public abstract class ServiceClient : IDisposable
         /// </summary>
         internal ServiceClient()
         {
+            JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();
             TlsVersions.Instance.SetLegacyAcceptableVersions();
         }
 
@@ -64,7 +66,8 @@ public void Dispose()
         /// Releases unmanaged and - optionally - managed resources.
         /// </summary>
         /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param>
-        protected virtual void Dispose(bool disposing) { }
+        protected virtual void Dispose(bool disposing)
+        { }
 
         /// <summary>
         /// Create an instance of ServiceClient from the specified IoT Hub connection string using specified Transport Type.

diff --git a/iothub/service/tests/JobClient/JobClientTests.cs b/iothub/service/tests/JobClient/JobClientTests.cs
@@ -0,0 +1,53 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+using System;
+using System.Collections.Generic;
+using System.Text;
+using Microsoft.Azure.Devices.Shared;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Microsoft.Azure.Devices;
+using FluentAssertions;
+using Newtonsoft.Json;
+using System.Linq;
+
+namespace Microsoft.Azure.Devices
+{
+    [TestClass]
+    [TestCategory("Unit")]
+    public class JobClientTests
+    {
+        [TestMethod]
+        public void JobClient_DefaultMaxDepth()
+        {
+            // arrange
+            string fakeConnectionString = "HostName=acme.azure-devices.net;SharedAccessKeyName=AllAccessKey;SharedAccessKey=dGVzdFN0cmluZzE=";
+            using var jobClient = JobClient.CreateFromConnectionString(fakeConnectionString);
+            // above arragement is only for setting the defaultJsonSerializerSettings
+
+            var defaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
+            defaultSettings.MaxDepth.Should().Be(128);
+        }
+
+        [TestMethod]
+        public void JobClient_Json_OverrideDefaultJsonSerializer_ExceedMaxDepthThrows()
+        {
+            // arrange
+            string fakeConnectionString = "HostName=acme.azure-devices.net;SharedAccessKeyName=AllAccessKey;SharedAccessKey=dGVzdFN0cmluZzE=";
+            using var jobClient = JobClient.CreateFromConnectionString(fakeConnectionString);
+            // above arragement is only for setting the defaultJsonSerializerSettings
+
+            //Create a string representation of a nested object (JSON serialized)
+            int nRep = 3;
+            string json = string.Concat(Enumerable.Repeat("{a:", nRep)) + "1" +
+            string.Concat(Enumerable.Repeat("}", nRep));
+
+            var settings = new JsonSerializerSettings { MaxDepth = 2 };
+            //// deserialize
+            // act
+            Func<object> act = () => JsonConvert.DeserializeObject(json, settings);
+
+            // assert
+            act.Should().Throw<Newtonsoft.Json.JsonReaderException>();
+        }
+    }
+}