Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting MaxDepth parameter in the JsonSerializerSettings as safe defaults #3045

Merged
merged 3 commits into from
Jan 6, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Add more test cases
schoims committed Jan 6, 2023
commit 722c1e97dc5b3a745f095bd0b66924e6f7d8c69f
2 changes: 1 addition & 1 deletion iothub/device/src/InternalClient.cs
Original file line number Diff line number Diff line change
@@ -157,7 +157,7 @@ public InternalClient(IotHubConnectionString iotHubConnectionString, ITransportS
Logging.Enter(this, transportSettings, pipelineBuilder, nameof(InternalClient) + "_ctor");
}

JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();

TlsVersions.Instance.SetLegacyAcceptableVersions();

43 changes: 42 additions & 1 deletion iothub/device/tests/DeviceClientTests.cs
Original file line number Diff line number Diff line change
@@ -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>();
}
}
}
36 changes: 36 additions & 0 deletions iothub/device/tests/ModuleClientTests.cs
Original file line number Diff line number Diff line change
@@ -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>();
}
}
}
2 changes: 1 addition & 1 deletion iothub/service/src/DigitalTwin/DigitalTwinClient.cs
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@ private DigitalTwinClient(Uri uri, IotServiceClientCredentials credentials, para
_client = new IotHubGatewayServiceAPIs(uri, credentials, handlers);
_protocolLayer = new DigitalTwin(_client);

JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();
}

/// <summary>
2 changes: 1 addition & 1 deletion iothub/service/src/JobClient/JobClient.cs
Original file line number Diff line number Diff line change
@@ -36,7 +36,7 @@ public static JobClient CreateFromConnectionString(string connectionString, Http
{
throw new ArgumentNullException(nameof(transportSettings), "HTTP Transport settings cannot be null.");
}
JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();
TlsVersions.Instance.SetLegacyAcceptableVersions();

var iotHubConnectionString = IotHubConnectionString.Parse(connectionString);
2 changes: 1 addition & 1 deletion iothub/service/src/RegistryManager.cs
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@ public static RegistryManager CreateFromConnectionString(string connectionString
throw new ArgumentNullException(nameof(transportSettings), "The HTTP transport settings cannot be null.");
}

JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();
TlsVersions.Instance.SetLegacyAcceptableVersions();

var iotHubConnectionString = IotHubConnectionString.Parse(connectionString);
2 changes: 1 addition & 1 deletion iothub/service/src/ServiceClient.cs
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@ public abstract class ServiceClient : IDisposable
/// </summary>
internal ServiceClient()
{
JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();
TlsVersions.Instance.SetLegacyAcceptableVersions();
}

53 changes: 53 additions & 0 deletions iothub/service/tests/JobClient/JobClientTests.cs
Original file line number Diff line number Diff line change
@@ -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>();
}
}
}
36 changes: 36 additions & 0 deletions iothub/service/tests/RegistryManagerTests.cs
Original file line number Diff line number Diff line change
@@ -10,10 +10,12 @@ namespace Microsoft.Azure.Devices.Api.Test
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using FluentAssertions;
using Microsoft.Azure.Devices;
using Microsoft.Azure.Devices.Shared;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;
using Newtonsoft.Json;

[TestClass]
[TestCategory("Unit")]
@@ -889,5 +891,39 @@ public async Task CloseAsyncTest()
await registryManager.CloseAsync().ConfigureAwait(false);
restOpMock.Verify(restOp => restOp.Dispose(), Times.Never());
}

[TestMethod]
public void RegistryManager_DefaultMaxDepth()
{
// arrange
string fakeConnectionString = "HostName=acme.azure-devices.net;SharedAccessKeyName=AllAccessKey;SharedAccessKey=dGVzdFN0cmluZzE=";
var registryManager = RegistryManager.CreateFromConnectionString(fakeConnectionString);
// above arragement is only for setting the defaultJsonSerializerSettings

var defaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
defaultSettings.MaxDepth.Should().Be(128);
}

[TestMethod]
public void RegistryManager_OverrideDefaultMaxDepth_ExceedMaxDepthThrows()
{
// arrange
string fakeConnectionString = "HostName=acme.azure-devices.net;SharedAccessKeyName=AllAccessKey;SharedAccessKey=dGVzdFN0cmluZzE=";
var registryManager = RegistryManager.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>();
}
}
}
4 changes: 4 additions & 0 deletions iothub/service/tests/SerializationTests.cs
Original file line number Diff line number Diff line change
@@ -74,6 +74,10 @@ public void Configuration_TestNullSchema_Ok()
public void Twin_Json_OverrideDefaultJsonSerializer_ExceedMaxDepthThrows()
{
// arrange
string fakeConnectionString = "HostName=acme.azure-devices.net;SharedAccessKeyName=AllAccessKey;SharedAccessKey=dGVzdFN0cmluZzE=";
var ServiceClinet = ServiceClient.CreateFromConnectionString(fakeConnectionString);
// above arragement is only for setting the defaultJsonSerializerSettings

const string jsonString = @"
{
""deviceId"": ""test"",
40 changes: 39 additions & 1 deletion iothub/service/tests/ServiceClientTests.cs
Original file line number Diff line number Diff line change
@@ -5,15 +5,19 @@ namespace Microsoft.Azure.Devices.Api.Test
{
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using FluentAssertions;
using Microsoft.Azure.Amqp;
using Microsoft.Azure.Devices;
using Microsoft.Azure.Devices.Common.Exceptions;
using Microsoft.Azure.Devices.Shared;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;
using Newtonsoft.Json;

[TestClass]
[TestCategory("Unit")]
@@ -56,7 +60,7 @@ public async Task PurgeMessageQueueDeviceNotFoundTest()
PurgeMessageQueueResult result = await serviceClient.PurgeMessageQueueAsync("TestDevice", CancellationToken.None).ConfigureAwait(false);
}

Tuple<Mock<IHttpClientHelper>, AmqpServiceClient, PurgeMessageQueueResult> SetupPurgeMessageQueueTests()
private Tuple<Mock<IHttpClientHelper>, AmqpServiceClient, PurgeMessageQueueResult> SetupPurgeMessageQueueTests()
{
// Create expected return object
var deviceId = "TestDevice";
@@ -117,5 +121,39 @@ public async Task CloseAsyncTest()
restOpMock.Verify(restOp => restOp.Dispose(), Times.Never());
Assert.IsTrue(connectionClosed);
}

[TestMethod]
public void ServiceClient_DefaultMaxDepth()
{
// arrange
string fakeConnectionString = "HostName=acme.azure-devices.net;SharedAccessKeyName=AllAccessKey;SharedAccessKey=dGVzdFN0cmluZzE=";
var serviceClinet = ServiceClient.CreateFromConnectionString(fakeConnectionString);
// above arragement is only for setting the defaultJsonSerializerSettings

var defaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
defaultSettings.MaxDepth.Should().Be(128);
}

[TestMethod]
public void ServiceClient_Json_OverrideDefaultJsonSerializer_ExceedMaxDepthThrows()
{
// arrange
string fakeConnectionString = "HostName=acme.azure-devices.net;SharedAccessKeyName=AllAccessKey;SharedAccessKey=dGVzdFN0cmluZzE=";
var serviceClinet = ServiceClient.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>();
}
}
}
2 changes: 1 addition & 1 deletion provisioning/service/src/ProvisioningServiceClient.cs
Original file line number Diff line number Diff line change
@@ -114,7 +114,7 @@ public static ProvisioningServiceClient CreateFromConnectionString(string connec
/// <exception cref="ArgumentException">if the connectionString is <code>null</code>, empty, or invalid.</exception>
private ProvisioningServiceClient(string connectionString, HttpTransportSettings httpTransportSettings)
{
JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettings();
JsonConvert.DefaultSettings = JsonSerializerSettingsInitializer.GetDefaultJsonSerializerSettingsDelegate();

/* SRS_PROVISIONING_SERVICE_CLIENT_21_002: [The constructor shall throw ArgumentException if the provided connectionString is null or empty.] */
if (string.IsNullOrWhiteSpace(connectionString ?? throw new ArgumentNullException(nameof(connectionString))))
Loading