From 3d85b1554deab88922ca9ee744c39ac25af3c5ab Mon Sep 17 00:00:00 2001 From: sw-joelmut Date: Fri, 11 Nov 2022 09:55:16 -0300 Subject: [PATCH 1/4] Add AllowedTypesSerializationBinder implementation --- .../BlobsStorage.cs | 32 ++- .../BlobsTranscriptStore.cs | 6 +- .../AzureBlobStorage.cs | 24 +- .../CosmosDbPartitionedStorage.cs | 17 +- .../AllowedTypesSerializationBinder.cs | 248 ++++++++++++++++++ 5 files changed, 314 insertions(+), 13 deletions(-) create mode 100644 libraries/Microsoft.Bot.Builder/AllowedTypesSerializationBinder.cs diff --git a/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs b/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs index 51e128be1c..67ad931f5c 100644 --- a/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs +++ b/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Net; using System.Threading; using System.Threading.Tasks; @@ -13,6 +14,7 @@ using Azure.Storage.Blobs; using Azure.Storage.Blobs.Models; using Newtonsoft.Json; +using Newtonsoft.Json.Linq; namespace Microsoft.Bot.Builder.Azure.Blobs { @@ -29,6 +31,13 @@ namespace Microsoft.Bot.Builder.Azure.Blobs /// public class BlobsStorage : IStorage { + private static readonly AllowedTypesSerializationBinder _allowedTypesBinder = new AllowedTypesSerializationBinder( + new List + { + typeof(IStoreItem), + typeof(Dictionary) + }); + // If a JsonSerializer is not provided during construction, this will be the default JsonSerializer. private readonly JsonSerializer _jsonSerializer; private readonly BlobContainerClient _containerClient; @@ -44,6 +53,7 @@ public class BlobsStorage : IStorage /// jsonSerializer.TypeNameHandling = TypeNameHandling.None. /// jsonSerializer.NullValueHandling = NullValueHandling.Include. /// jsonSerializer.ContractResolver = new DefaultContractResolver(). + /// jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder(). /// public BlobsStorage(string dataConnectionString, string containerName, JsonSerializer jsonSerializer = null) : this(dataConnectionString, containerName, default, jsonSerializer) @@ -60,6 +70,7 @@ public BlobsStorage(string dataConnectionString, string containerName, JsonSeria /// jsonSerializer.TypeNameHandling = TypeNameHandling.None. /// jsonSerializer.NullValueHandling = NullValueHandling.Include. /// jsonSerializer.ContractResolver = new DefaultContractResolver(). + /// jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder(). /// public BlobsStorage(string dataConnectionString, string containerName, StorageTransferOptions storageTransferOptions, JsonSerializer jsonSerializer = null) { @@ -77,7 +88,8 @@ public BlobsStorage(string dataConnectionString, string containerName, StorageTr _jsonSerializer = jsonSerializer ?? JsonSerializer.Create(new JsonSerializerSettings { - TypeNameHandling = TypeNameHandling.All, + TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling] + SerializationBinder = _allowedTypesBinder, }); // Triggers a check for the existence of the container @@ -94,6 +106,7 @@ public BlobsStorage(string dataConnectionString, string containerName, StorageTr /// jsonSerializer.TypeNameHandling = TypeNameHandling.None. /// jsonSerializer.NullValueHandling = NullValueHandling.Include. /// jsonSerializer.ContractResolver = new DefaultContractResolver(). + /// jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder(). /// internal BlobsStorage(BlobContainerClient containerClient, JsonSerializer jsonSerializer = null) { @@ -101,7 +114,8 @@ internal BlobsStorage(BlobContainerClient containerClient, JsonSerializer jsonSe _jsonSerializer = jsonSerializer ?? JsonSerializer.Create(new JsonSerializerSettings { - TypeNameHandling = TypeNameHandling.All, + TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling] + SerializationBinder = _allowedTypesBinder, }); // Triggers a check for the existence of the container @@ -212,7 +226,19 @@ internal BlobsStorage(BlobContainerClient containerClient, JsonSerializer jsonSe { using var memoryStream = new MemoryStream(); using var streamWriter = new StreamWriter(memoryStream); - _jsonSerializer.Serialize(streamWriter, newValue); + using var jsonWriter = new JsonTextWriter(streamWriter); + + var json = JToken.FromObject(newValue, _jsonSerializer); + if (json.Type == JTokenType.Object || json.Type == JTokenType.Array) + { + (_jsonSerializer.SerializationBinder as AllowedTypesSerializationBinder)?.CleanupTypes((JContainer)json); + await json.WriteToAsync(jsonWriter).ConfigureAwait(false); + } + else + { + _jsonSerializer.Serialize(streamWriter, newValue); + } + await streamWriter.FlushAsync().ConfigureAwait(false); memoryStream.Seek(0, SeekOrigin.Begin); diff --git a/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsTranscriptStore.cs b/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsTranscriptStore.cs index 6767c61a61..0537d809a0 100644 --- a/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsTranscriptStore.cs +++ b/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsTranscriptStore.cs @@ -80,7 +80,6 @@ public BlobsTranscriptStore(string dataConnectionString, string containerName, S { NullValueHandling = NullValueHandling.Ignore, Formatting = Formatting.Indented, - TypeNameHandling = TypeNameHandling.None, }); // Triggers a check for the existance of the container @@ -112,10 +111,7 @@ internal BlobsTranscriptStore(BlobContainerClient containerClient, JsonSerialize { _containerClient = new Lazy(() => containerClient); - _jsonSerializer = jsonSerializer ?? JsonSerializer.Create(new JsonSerializerSettings - { - TypeNameHandling = TypeNameHandling.All, - }); + _jsonSerializer = jsonSerializer ?? JsonSerializer.Create(); } /// diff --git a/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs b/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs index df2f3de03c..e369812bc7 100644 --- a/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs +++ b/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Net; using System.Threading; using System.Threading.Tasks; @@ -14,6 +15,7 @@ using Microsoft.WindowsAzure.Storage.Core; using Microsoft.WindowsAzure.Storage.RetryPolicies; using Newtonsoft.Json; +using Newtonsoft.Json.Linq; namespace Microsoft.Bot.Builder.Azure { @@ -33,8 +35,13 @@ public class AzureBlobStorage : IStorage { private static readonly JsonSerializer JsonSerializer = JsonSerializer.Create(new JsonSerializerSettings { - // we use All so that we get typed roundtrip out of storage, but we don't use validation because we don't know what types are valid - TypeNameHandling = TypeNameHandling.All, + TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling] + SerializationBinder = new AllowedTypesSerializationBinder( + new List + { + typeof(IStoreItem), + typeof(Dictionary) + }), }); // If a JsonSerializer is not provided during construction, this will be the default static JsonSerializer. @@ -53,6 +60,7 @@ public class AzureBlobStorage : IStorage /// jsonSerializer.TypeNameHandling = TypeNameHandling.All. /// jsonSerializer.NullValueHandling = NullValueHandling.Include. /// jsonSerializer.ContractResolver = new DefaultContractResolver(). + /// jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder(). /// public AzureBlobStorage(CloudStorageAccount storageAccount, string containerName, JsonSerializer jsonSerializer) { @@ -210,8 +218,18 @@ internal AzureBlobStorage(CloudStorageAccount storageAccount, string containerNa { using (var memoryStream = new MultiBufferMemoryStream(blobReference.ServiceClient.BufferManager)) using (var streamWriter = new StreamWriter(memoryStream)) + using (var jsonWriter = new JsonTextWriter(streamWriter)) { - _jsonSerializer.Serialize(streamWriter, newValue); + var json = JToken.FromObject(newValue, _jsonSerializer); + if (json.Type == JTokenType.Object || json.Type == JTokenType.Array) + { + (_jsonSerializer.SerializationBinder as AllowedTypesSerializationBinder)?.CleanupTypes((JContainer)json); + await json.WriteToAsync(jsonWriter).ConfigureAwait(false); + } + else + { + _jsonSerializer.Serialize(streamWriter, newValue); + } await streamWriter.FlushAsync().ConfigureAwait(false); diff --git a/libraries/Microsoft.Bot.Builder.Azure/CosmosDbPartitionedStorage.cs b/libraries/Microsoft.Bot.Builder.Azure/CosmosDbPartitionedStorage.cs index ef4113d5d1..13766c60f6 100644 --- a/libraries/Microsoft.Bot.Builder.Azure/CosmosDbPartitionedStorage.cs +++ b/libraries/Microsoft.Bot.Builder.Azure/CosmosDbPartitionedStorage.cs @@ -21,7 +21,18 @@ namespace Microsoft.Bot.Builder.Azure public class CosmosDbPartitionedStorage : IStorage, IDisposable { private const int MaxDepthAllowed = 127; - private readonly JsonSerializer _jsonSerializer = JsonSerializer.Create(new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.All, MaxDepth = null }); + + private readonly JsonSerializer _jsonSerializer = JsonSerializer.Create(new JsonSerializerSettings + { + TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling] + SerializationBinder = new AllowedTypesSerializationBinder( + new List + { + typeof(IStoreItem), + typeof(Dictionary) + }), + MaxDepth = null + }); private Container _container; private readonly CosmosDbPartitionedStorageOptions _cosmosDbStorageOptions; @@ -91,6 +102,7 @@ public CosmosDbPartitionedStorage(CosmosDbPartitionedStorageOptions cosmosDbStor /// jsonSerializer.TypeNameHandling = TypeNameHandling.All. /// jsonSerializer.NullValueHandling = NullValueHandling.Include. /// jsonSerializer.ContractResolver = new DefaultContractResolver(). + /// jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder(). /// public CosmosDbPartitionedStorage(CosmosDbPartitionedStorageOptions cosmosDbStorageOptions, JsonSerializer jsonSerializer) : this(cosmosDbStorageOptions) @@ -205,6 +217,7 @@ internal CosmosDbPartitionedStorage(CosmosClient client, CosmosDbPartitionedStor foreach (var change in changes) { var json = JObject.FromObject(change.Value, _jsonSerializer); + (_jsonSerializer.SerializationBinder as AllowedTypesSerializationBinder)?.CleanupTypes(json); // Remove etag from JSON object that was copied from IStoreItem. // The ETag information is updated as an _etag attribute in the document metadata. @@ -381,7 +394,7 @@ private async Task InitializeAsync() try { _container = _client.GetContainer(_cosmosDbStorageOptions.DatabaseId, _cosmosDbStorageOptions.ContainerId); - + // This will throw if the container does not exist. var readContainer = await _container.ReadContainerAsync().ConfigureAwait(false); diff --git a/libraries/Microsoft.Bot.Builder/AllowedTypesSerializationBinder.cs b/libraries/Microsoft.Bot.Builder/AllowedTypesSerializationBinder.cs new file mode 100644 index 0000000000..7d102fd479 --- /dev/null +++ b/libraries/Microsoft.Bot.Builder/AllowedTypesSerializationBinder.cs @@ -0,0 +1,248 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using Newtonsoft.Json.Serialization; + +namespace Microsoft.Bot.Builder +{ + /// + /// An implementation of the , + /// capable of allowing only desired s to be serialized and deserialized. + /// + public class AllowedTypesSerializationBinder : DefaultSerializationBinder + { + /// + /// Initializes a new instance of the class. + /// + /// A list of types to allow this binder to assign upon deserialization. + public AllowedTypesSerializationBinder(IList allowedTypes = default) + { + AllowedTypes = allowedTypes ?? new List(); + } + + /// + /// Gets the collection of the allowed types. + /// + /// + /// A of allowed classes. + /// + public IList AllowedTypes { get; } + + /// + /// + /// Given the parameter, + /// it evaluates if the is allowed by this SerializationBinder. + /// + /// + /// Either allowed or not allowed, it will output the name of the through the parameter. + /// + /// + /// When allowed, it will add the to the collection. + /// + /// + /// The type of the object the formatter creates a new instance of. + /// Specifies the System.Reflection.Assembly name of the serialized object. + /// Specifies the System.Type name of the serialized object. + public override void BindToName(Type serializedType, out string assemblyName, out string typeName) + { + assemblyName = null; + typeName = serializedType?.AssemblyQualifiedName; + + if (serializedType == null || AllowedTypes.Count == 0) + { + return; + } + + if (IsTypeAllowed(serializedType)) + { + AllowType(serializedType); + } + } + + /// + /// Given the and parameters, + /// it validates if the resulted is found in the collection. + /// + /// When found, it will return the serialized . + /// + /// + /// When not found, it will throw an . + /// + /// + /// Specifies the System.Reflection.Assembly name of the serialized object. + /// Specifies the System.Type name of the serialized object. + /// The found in the collection. + /// When the resulted + /// from the and parameters + /// is not found in the collection. + public override Type BindToType(string assemblyName, string typeName) + { + var resolvedTypeName = string.Format(CultureInfo.InvariantCulture, "{0}, {1}", typeName, assemblyName); + var type = Type.GetType(resolvedTypeName); + + // Preload related type. + if (IsTypeAllowed(type)) + { + AllowType(type); + } + + if (!AllowedTypes.Contains(type)) + { + ThrowDisallowedTypesError(new List { type }); + } + + return type; + } + + /// + /// Finds and remove all the '$type' properties within the provided parameter, + /// that are not included in the collection. + /// + /// A JSON object. + public void CleanupTypes(JContainer json) + { + if (AllowedTypes.Count == 0) + { + return; + } + + var disallowedTypes = new List(); + var allowedTypes = AllowedTypes.ToDictionary(e => e.AssemblyQualifiedName); + + // Remove AllowedTypes nested types from the object. + json.Descendants() + .OfType() + .Where(attr => attr.Name == "$type") + .Select(attr => new + { + Property = attr, + Type = Type.GetType(attr.Value.ToString()) + }) + .Where(attr => + { + if (AllowedTypes.Contains(attr.Type)) + { + return false; + } + + var shouldRemove = attr.Type == null || ShouldRemoveType(attr.Type, attr.Property.Parent, allowedTypes); + if (shouldRemove) + { + return true; + } + + disallowedTypes.Add(attr.Type); + return false; + }) + .ToList() + .ForEach(attr => attr.Property.Remove()); + + if (disallowedTypes.Count > 0) + { + ThrowDisallowedTypesError(disallowedTypes); + } + } + + private void ThrowDisallowedTypesError(List types) + { + var items = types.Select(type => $" - {type.AssemblyQualifiedName}"); + var typeOfs = types.Select(type => $"typeof({type.Name}),"); + var message = $"Unable to find the following types in the '{nameof(AllowedTypes)}' collection." + Environment.NewLine + + string.Join(Environment.NewLine, items) + Environment.NewLine + + Environment.NewLine + + $"Please provide the '{nameof(AllowedTypesSerializationBinder)}' in the custom '{nameof(JsonSerializerSettings)}' instance, with the list of types to allow." + Environment.NewLine + + Environment.NewLine + + "Example:" + Environment.NewLine + + " new JsonSerializerSettings" + Environment.NewLine + + " {" + Environment.NewLine + + " SerializationBinder = new AllowedTypesSerializationBinder(" + Environment.NewLine + + " new List" + Environment.NewLine + + " {" + Environment.NewLine + + $" {string.Join(Environment.NewLine + " ", typeOfs)}" + Environment.NewLine + + " })," + Environment.NewLine + + " }"; + throw new InvalidOperationException(message); + } + + private bool ShouldRemoveType(Type child, JContainer parent, IDictionary memory) + { + if (parent == null) + { + return false; + } + + var parentTypeName = parent.Type == JTokenType.Object ? parent?.Value("$type") : string.Empty; + var parentType = Type.GetType(parentTypeName); + if (parentType == null) + { + return ShouldRemoveType(child, parent.Parent, memory); + } + + if (memory.ContainsKey(parentType.AssemblyQualifiedName)) + { + var reference = $"{parentType.AssemblyQualifiedName} / {child.AssemblyQualifiedName}"; + if (!memory.ContainsKey(reference)) + { + memory.Add(reference, child); + } + + return true; + } + + return ShouldRemoveType(parentType, parent.Parent, memory); + } + + private bool IsTypeAllowed(Type serializedType) + { + if (serializedType == null) + { + return false; + } + + // Return Type when found. + var typeFound = AllowedTypes.FirstOrDefault(e => e.AssemblyQualifiedName == serializedType.AssemblyQualifiedName); + if (typeFound != null) + { + return true; + } + + // Return Type when it's inside another Type. + if (serializedType.IsNested) + { + if (IsTypeAllowed(serializedType.ReflectedType)) + { + return true; + } + } + + // Return Type when it have Interfaces. + var interfaces = serializedType.GetInterfaces(); + var interfaceFound = AllowedTypes.FirstOrDefault(t => interfaces.Any(e => e.AssemblyQualifiedName == t.AssemblyQualifiedName)); + if (interfaceFound != null) + { + return true; + } + + return false; + } + + private void AllowType(Type type) + { + if (type == null) + { + return; + } + + if (!AllowedTypes.Contains(type)) + { + AllowedTypes.Add(type); + } + } + } +} From 2f5659ce6a4e40a15e2796be498b077a0ee6d7ec Mon Sep 17 00:00:00 2001 From: sw-joelmut Date: Fri, 11 Nov 2022 09:56:20 -0300 Subject: [PATCH 2/4] Add AllowedTypesSerializationBinder tests --- .../AzureBlobStorageTests.cs | 18 +- .../BlobsStorageTests.cs | 3 +- .../AllowedTypesSerializationBinderTests.cs | 186 ++++++++++++++++++ 3 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 tests/Microsoft.Bot.Builder.Tests/AllowedTypesSerializationBinderTests.cs diff --git a/tests/Microsoft.Bot.Builder.Azure.Tests/AzureBlobStorageTests.cs b/tests/Microsoft.Bot.Builder.Azure.Tests/AzureBlobStorageTests.cs index 0ed879071e..cc1187fb23 100644 --- a/tests/Microsoft.Bot.Builder.Azure.Tests/AzureBlobStorageTests.cs +++ b/tests/Microsoft.Bot.Builder.Azure.Tests/AzureBlobStorageTests.cs @@ -7,6 +7,7 @@ using System.Text; using System.Threading; using System.Threading.Tasks; +using Microsoft.Bot.Schema; using Microsoft.WindowsAzure.Storage; using Microsoft.WindowsAzure.Storage.Auth; using Microsoft.WindowsAzure.Storage.Blob; @@ -177,12 +178,16 @@ public async Task WriteAsyncNullChangesFailure() } [Fact] - public async Task WriteAsyncTwoChanges() + public async Task WriteAsyncMultipleChanges() { var changes = new Dictionary { { "key1", "value1" }, - { "key2", "value2" } + { "key2", 0 }, + { "key3", true }, + { "key4", new StoreItem() }, + { "key5", new List() { new StoreItem() } }, + { "key6", new Activity() } }; InitStorage(); @@ -190,7 +195,7 @@ public async Task WriteAsyncTwoChanges() await _blobStorage.WriteAsync(changes, CancellationToken.None); _mockBlobClient.Verify(x => x.GetContainerReference(It.IsAny()), Times.Once); - _mockContainer.Verify(x => x.GetBlockBlobReference(It.IsAny()), Times.Exactly(2)); + _mockContainer.Verify(x => x.GetBlockBlobReference(It.IsAny()), Times.Exactly(6)); _mockBlockBlob.Verify( x => x.UploadFromStreamAsync( It.IsAny(), @@ -198,7 +203,7 @@ public async Task WriteAsyncTwoChanges() It.IsAny(), It.IsAny(), It.IsAny()), - Times.Exactly(2)); + Times.Exactly(6)); } [Fact] @@ -254,5 +259,10 @@ private void InitStorage() _blobStorage = new AzureBlobStorage(_mockAccount.Object, ContainerName, _mockBlobClient.Object); } + + private class StoreItem : IStoreItem + { + public string ETag { get; set; } + } } } diff --git a/tests/Microsoft.Bot.Builder.Azure.Tests/BlobsStorageTests.cs b/tests/Microsoft.Bot.Builder.Azure.Tests/BlobsStorageTests.cs index 69ac3333b1..6288d243bf 100644 --- a/tests/Microsoft.Bot.Builder.Azure.Tests/BlobsStorageTests.cs +++ b/tests/Microsoft.Bot.Builder.Azure.Tests/BlobsStorageTests.cs @@ -73,6 +73,7 @@ public async void WriteAsync() { "key1", new StoreItem() }, { "key2", new StoreItem { ETag = "*" } }, { "key3", new StoreItem { ETag = "ETag" } }, + { "key4", "value1" }, }; await _storage.WriteAsync(changes); @@ -87,7 +88,7 @@ public async void WriteAsync() It.IsAny(), It.IsAny(), It.IsAny()), - Times.Exactly(3)); + Times.Exactly(4)); } [Fact] diff --git a/tests/Microsoft.Bot.Builder.Tests/AllowedTypesSerializationBinderTests.cs b/tests/Microsoft.Bot.Builder.Tests/AllowedTypesSerializationBinderTests.cs new file mode 100644 index 0000000000..971650b777 --- /dev/null +++ b/tests/Microsoft.Bot.Builder.Tests/AllowedTypesSerializationBinderTests.cs @@ -0,0 +1,186 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Globalization; +using Microsoft.Bot.Schema; +using Newtonsoft.Json.Linq; +using Xunit; + +namespace Microsoft.Bot.Builder.Tests +{ + public class AllowedTypesSerializationBinderTests + { + [Fact] + public void ConstructorValidation() + { + // Empty AllowedTypes. + Assert.NotNull(new AllowedTypesSerializationBinder().AllowedTypes); + Assert.Empty(new AllowedTypesSerializationBinder().AllowedTypes); + + // Null AllowedTypes. + Assert.Empty(new AllowedTypesSerializationBinder(null).AllowedTypes); + Assert.NotNull(new AllowedTypesSerializationBinder(null).AllowedTypes); + + // With AllowedTypes. + Assert.NotNull(new AllowedTypesSerializationBinder(new List { typeof(ExampleType) }).AllowedTypes); + Assert.Single(new AllowedTypesSerializationBinder(new List { typeof(ExampleType) }).AllowedTypes); + } + + [Fact] + public void BindToNameWithEmptyAllowedTypes() + { + var binder = new AllowedTypesSerializationBinder(); + binder.BindToName(null, out var assemblyName, out var typeName); + + Assert.Null(assemblyName); + Assert.Null(typeName); + } + + [Fact] + public void BindToNameWithoutAllowedType() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(new List { typeof(string) }); + binder.BindToName(expectedType, out var assemblyName, out var typeName); + + Assert.Null(assemblyName); + Assert.NotNull(typeName); + } + + [Fact] + public void BindToNameWithType() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(new List { expectedType }); + binder.BindToName(expectedType, out var assemblyName, out var typeName); + + Assert.Null(assemblyName); + Assert.Equal(expectedType.AssemblyQualifiedName, typeName); + } + + [Fact] + public void BindToNameWithNestedType() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(new List { typeof(AllowedTypesSerializationBinderTests) }); + binder.BindToName(expectedType, out var assemblyName, out var typeName); + + Assert.Null(assemblyName); + Assert.Equal(expectedType.AssemblyQualifiedName, typeName); + Assert.Contains(binder.AllowedTypes, e => e.AssemblyQualifiedName == typeName); + } + + [Fact] + public void BindToNameWithInterfaces() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(new List { typeof(IDisposable) }); + binder.BindToName(expectedType, out var assemblyName, out var typeName); + + Assert.Null(assemblyName); + Assert.Equal(expectedType.AssemblyQualifiedName, typeName); + Assert.Contains(binder.AllowedTypes, e => e.AssemblyQualifiedName == typeName); + } + + [Fact] + public void BindToTypeWithAllowedType() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(new List { expectedType }); + var resultType = binder.BindToType(expectedType.Assembly.GetName().Name, expectedType.FullName); + + Assert.Equal(expectedType, resultType); + } + + [Fact] + public void BindToTypeWithoutProcessedAllowedType() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(new List { typeof(AllowedTypesSerializationBinderTests) }); + var resultType = binder.BindToType(expectedType.Assembly.GetName().Name, expectedType.FullName); + + Assert.Equal(expectedType, resultType); + } + + [Fact] + public void BindToTypeWithoutAllowedType() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(new List { typeof(string) }); + Assert.Throws(() => binder.BindToType(expectedType.Assembly.GetName().Name, expectedType.FullName)); + } + + [Fact] + public void CleanupTypesWithAllowedTypes() + { + var expectedType = typeof(ExampleType); + var unknownType = typeof(string); + var binder = new AllowedTypesSerializationBinder(new List { expectedType }); + var resolvedTypeName = string.Format(CultureInfo.InvariantCulture, "{0}, {1}", expectedType.FullName, expectedType.Assembly.GetName().Name); + var unknownTypeName = string.Format(CultureInfo.InvariantCulture, "{0}, {1}", unknownType.FullName, unknownType.Assembly.GetName().Name); + var obj = JObject.Parse(@$"{{'$type':'{resolvedTypeName}', 'inner': {{ '$type': '{unknownTypeName}' }} }}"); + binder.CleanupTypes(obj); + + Assert.Equal(resolvedTypeName, obj["$type"]); + Assert.Null(obj["inner"]["$type"]); + } + + [Fact] + public void CleanupTypesWithoutAllowedTypes() + { + var expectedType = typeof(ExampleType); + var stringType = typeof(string); + var activityType = typeof(Activity); + var binder = new AllowedTypesSerializationBinder(new List { expectedType }); + var resolvedTypeName = string.Format(CultureInfo.InvariantCulture, "{0}, {1}", expectedType.FullName, expectedType.Assembly.GetName().Name); + var stringTypeName = string.Format(CultureInfo.InvariantCulture, "{0}, {1}", stringType.FullName, stringType.Assembly.GetName().Name); + var activityTypeName = string.Format(CultureInfo.InvariantCulture, "{0}, {1}", activityType.FullName, activityType.Assembly.GetName().Name); + var obj = (JContainer)JToken.Parse(@$"[ + {{'$type':'{resolvedTypeName}', 'inner': {{ '$type': '{stringTypeName}' }}, 'activity': {{ '$type': '{activityTypeName}' }} }}, + {{'$type':'{stringTypeName}', 'inner': {{ '$type': '{resolvedTypeName}' }} }}, + {{'$type':'{activityTypeName}', 'inner': {{ '$type': '{resolvedTypeName}' }} }} + ]"); + Assert.Throws(() => binder.CleanupTypes(obj)); + } + + [Fact] + public void CleanupTypesWithEmptyObject() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(new List { expectedType }); + var obj = (JContainer)JToken.Parse("{}"); + binder.CleanupTypes(obj); + } + + [Fact] + public void CleanupTypesWithEmptyArray() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(new List { expectedType }); + var obj = (JContainer)JToken.Parse("[]"); + binder.CleanupTypes(obj); + } + + [Fact] + public void CleanupTypesWithEmptyAllowedTypes() + { + var expectedType = typeof(ExampleType); + var binder = new AllowedTypesSerializationBinder(); + var resolvedTypeName = string.Format(CultureInfo.InvariantCulture, "{0}, {1}", expectedType.FullName, expectedType.Assembly.GetName().Name); + var obj = JObject.Parse(@$"{{'$type':'{resolvedTypeName}'}}"); + binder.CleanupTypes(obj); + + Assert.Equal(resolvedTypeName, obj["$type"]); + } + + private class ExampleType : IDisposable + { + public void Dispose() + { + throw new NotImplementedException(); + } + } + } +} From 1ed3f124a25c4f50412f747528f1ae23922d7377 Mon Sep 17 00:00:00 2001 From: sw-joelmut Date: Fri, 11 Nov 2022 11:37:20 -0300 Subject: [PATCH 3/4] Fix typo and remove usings --- libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs | 1 - libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs | 1 - .../Microsoft.Bot.Builder/AllowedTypesSerializationBinder.cs | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs b/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs index 67ad931f5c..915736280a 100644 --- a/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs +++ b/libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Net; using System.Threading; using System.Threading.Tasks; diff --git a/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs b/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs index e369812bc7..902240957b 100644 --- a/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs +++ b/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Net; using System.Threading; using System.Threading.Tasks; diff --git a/libraries/Microsoft.Bot.Builder/AllowedTypesSerializationBinder.cs b/libraries/Microsoft.Bot.Builder/AllowedTypesSerializationBinder.cs index 7d102fd479..a51ebea10c 100644 --- a/libraries/Microsoft.Bot.Builder/AllowedTypesSerializationBinder.cs +++ b/libraries/Microsoft.Bot.Builder/AllowedTypesSerializationBinder.cs @@ -221,7 +221,7 @@ private bool IsTypeAllowed(Type serializedType) } } - // Return Type when it have Interfaces. + // Return Type when it has Interfaces. var interfaces = serializedType.GetInterfaces(); var interfaceFound = AllowedTypes.FirstOrDefault(t => interfaces.Any(e => e.AssemblyQualifiedName == t.AssemblyQualifiedName)); if (interfaceFound != null) From e96bfaaa0601d453ab78de6a1cc4f7940a9091f8 Mon Sep 17 00:00:00 2001 From: sw-joelmut Date: Tue, 15 Nov 2022 12:37:57 -0300 Subject: [PATCH 4/4] Fix unit tests --- .../AzureBlobStorage.cs | 10 ++++++++-- .../CosmosDbPartitionedStorage.cs | 12 +++++++++++- .../AzureBlobStorageTests.cs | 15 ++++++++++++++- .../CosmosDbPartitionedStorageTests.cs | 19 +++++++++++++++++-- 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs b/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs index e76078005a..89b269608c 100644 --- a/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs +++ b/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs @@ -101,8 +101,14 @@ public AzureBlobStorage(CloudStorageAccount storageAccount, string containerName /// Azure CloudStorageAccount instance. /// Name of the Blob container where entities will be stored. /// Custom implementation of CloudBlobClient. - internal AzureBlobStorage(CloudStorageAccount storageAccount, string containerName, CloudBlobClient blobClient) - : this(storageAccount, containerName, JsonSerializer) + /// If passing in a custom JsonSerializer, we recommend the following settings: + /// jsonSerializer.TypeNameHandling = TypeNameHandling.All. + /// jsonSerializer.NullValueHandling = NullValueHandling.Include. + /// jsonSerializer.ContractResolver = new DefaultContractResolver(). + /// jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder(). + /// + internal AzureBlobStorage(CloudStorageAccount storageAccount, string containerName, CloudBlobClient blobClient, JsonSerializer jsonSerializer = default) + : this(storageAccount, containerName, jsonSerializer ?? JsonSerializer) { _blobClient = blobClient; } diff --git a/libraries/Microsoft.Bot.Builder.Azure/CosmosDbPartitionedStorage.cs b/libraries/Microsoft.Bot.Builder.Azure/CosmosDbPartitionedStorage.cs index 13766c60f6..373fd3df89 100644 --- a/libraries/Microsoft.Bot.Builder.Azure/CosmosDbPartitionedStorage.cs +++ b/libraries/Microsoft.Bot.Builder.Azure/CosmosDbPartitionedStorage.cs @@ -117,10 +117,20 @@ public CosmosDbPartitionedStorage(CosmosDbPartitionedStorageOptions cosmosDbStor /// /// The custom implementation of CosmosClient. /// Cosmos DB partitioned storage configuration options. - internal CosmosDbPartitionedStorage(CosmosClient client, CosmosDbPartitionedStorageOptions cosmosDbStorageOptions) + /// If passing in a custom JsonSerializer, we recommend the following settings: + /// jsonSerializer.TypeNameHandling = TypeNameHandling.All. + /// jsonSerializer.NullValueHandling = NullValueHandling.Include. + /// jsonSerializer.ContractResolver = new DefaultContractResolver(). + /// jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder(). + /// + internal CosmosDbPartitionedStorage(CosmosClient client, CosmosDbPartitionedStorageOptions cosmosDbStorageOptions, JsonSerializer jsonSerializer = default) : this(cosmosDbStorageOptions) { _client = client; + if (jsonSerializer != null) + { + _jsonSerializer = jsonSerializer; + } } /// diff --git a/tests/Microsoft.Bot.Builder.Azure.Tests/AzureBlobStorageTests.cs b/tests/Microsoft.Bot.Builder.Azure.Tests/AzureBlobStorageTests.cs index cc1187fb23..6507b21469 100644 --- a/tests/Microsoft.Bot.Builder.Azure.Tests/AzureBlobStorageTests.cs +++ b/tests/Microsoft.Bot.Builder.Azure.Tests/AzureBlobStorageTests.cs @@ -257,7 +257,20 @@ private void InitStorage() _mockAccount = new Mock(new StorageCredentials("accountName", "S2V5VmFsdWU=", "key"), false); - _blobStorage = new AzureBlobStorage(_mockAccount.Object, ContainerName, _mockBlobClient.Object); + var jsonSerializer = new JsonSerializer + { + TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling] + MaxDepth = null, + SerializationBinder = new AllowedTypesSerializationBinder( + new List + { + typeof(IStoreItem), + typeof(Dictionary), + typeof(Activity) + }), + }; + + _blobStorage = new AzureBlobStorage(_mockAccount.Object, ContainerName, _mockBlobClient.Object, jsonSerializer); } private class StoreItem : IStoreItem diff --git a/tests/Microsoft.Bot.Builder.Azure.Tests/CosmosDbPartitionedStorageTests.cs b/tests/Microsoft.Bot.Builder.Azure.Tests/CosmosDbPartitionedStorageTests.cs index cc76caa1c7..92f29d0d90 100644 --- a/tests/Microsoft.Bot.Builder.Azure.Tests/CosmosDbPartitionedStorageTests.cs +++ b/tests/Microsoft.Bot.Builder.Azure.Tests/CosmosDbPartitionedStorageTests.cs @@ -244,7 +244,7 @@ public async void WriteAsyncFailure() _container.Setup(e => e.UpsertItemAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .ThrowsAsync(new CosmosException("InternalServerError", HttpStatusCode.InternalServerError, 0, "0", 0)); - var changes = new Dictionary { { "key", new { } } }; + var changes = new Dictionary { { "key", new CosmosDbPartitionedStorage.DocumentStoreItem() } }; await Assert.ThrowsAsync(() => _storage.WriteAsync(changes)); _container.Verify(e => e.UpsertItemAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); @@ -348,7 +348,22 @@ private void InitStorage(string partitionKey = "/id", CosmosDbPartitionedStorage DatabaseId = "DatabaseId", ContainerId = "ContainerId", }; - _storage = new CosmosDbPartitionedStorage(client.Object, options); + + var jsonSerializer = new JsonSerializer + { + TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling] + MaxDepth = null, + SerializationBinder = new AllowedTypesSerializationBinder( + new List + { + typeof(IStoreItem), + typeof(Dictionary), + typeof(DialogState), + typeof(DialogInstance) + }), + }; + + _storage = new CosmosDbPartitionedStorage(client.Object, options, jsonSerializer); } private Dictionary GenerateNestedDict()