From 29ce49c1e87673419f11cf0e444d971c0c8a84f1 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Sat, 28 Mar 2020 18:38:40 -0700 Subject: [PATCH 1/3] Add System.Type converter for JsonSerializer --- .../src/Resources/Strings.resx | 3 ++ .../src/System.Text.Json.csproj | 1 + .../Converters/Value/TypeConverter.cs | 19 +++++++++ .../JsonSerializerOptions.Converters.cs | 3 +- .../Text/Json/ThrowHelper.Serialization.cs | 8 +++- .../tests/Serialization/ExceptionTests.cs | 18 ++++++++ .../tests/Serialization/TestClasses.cs | 5 +++ .../tests/Serialization/WriteValueTests.cs | 42 +++++++++++++++++++ 8 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index 5186d1dc48727c..1cb9400bb096e1 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -506,4 +506,7 @@ Cannot allocate a buffer of size {0}. + + Deserialization of 'System.Type' instances is not supported and should be avoided since it can lead to security issues. A workaround is to use a custom converter. + \ No newline at end of file diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index ee9488ed2c0cb2..94c7b47f1b0c6f 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -109,6 +109,7 @@ + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs new file mode 100644 index 00000000000000..c4c40a6b12e00f --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace System.Text.Json.Serialization.Converters +{ + internal sealed class TypeConverter : JsonConverter + { + public override Type Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + throw ThrowHelper.GetNotSupportedException_DeserializeTypeInstanceNotSupported(); + } + + public override void Write(Utf8JsonWriter writer, Type value, JsonSerializerOptions options) + { + writer.WriteStringValue(value.AssemblyQualifiedName); + } + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 3e884eb49621bc..8a241a13c9fd99 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -37,7 +37,7 @@ public sealed partial class JsonSerializerOptions private static Dictionary GetDefaultSimpleConverters() { - const int NumberOfSimpleConverters = 21; + const int NumberOfSimpleConverters = 22; var converters = new Dictionary(NumberOfSimpleConverters); // Use a dictionary for simple converters. @@ -59,6 +59,7 @@ private static Dictionary GetDefaultSimpleConverters() Add(new SByteConverter()); Add(new SingleConverter()); Add(new StringConverter()); + Add(new TypeConverter()); Add(new UInt16Converter()); Add(new UInt32Converter()); Add(new UInt64Converter()); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index 6ff469e1616d0d..ee9547221fff2e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -28,11 +28,17 @@ public static void ThrowNotSupportedException_SerializationNotSupported(Type pro [DoesNotReturn] [MethodImpl(MethodImplOptions.NoInlining)] - public static NotSupportedException ThrowNotSupportedException_ConstructorMaxOf64Parameters(ConstructorInfo constructorInfo, Type type) + public static void ThrowNotSupportedException_ConstructorMaxOf64Parameters(ConstructorInfo constructorInfo, Type type) { throw new NotSupportedException(SR.Format(SR.ConstructorMaxOf64Parameters, constructorInfo, type)); } + [MethodImpl(MethodImplOptions.NoInlining)] + public static NotSupportedException GetNotSupportedException_DeserializeTypeInstanceNotSupported() + { + return new NotSupportedException(SR.DeserializeTypeInstanceNotSupported); + } + [DoesNotReturn] [MethodImpl(MethodImplOptions.NoInlining)] public static void ThrowJsonException_DeserializeUnableToConvertValue(Type propertyType) diff --git a/src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs b/src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs index ae5a45b9431383..fce13e60f45507 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs @@ -526,5 +526,23 @@ public static void UnsupportedTypeFromRoot() // Root-level Types (not from a property) do not include the Path. Assert.DoesNotContain("Path: $", ex.Message); } + + [Fact] + public static void DeserializeTypeInstance() + { + string json = JsonSerializer.Serialize(typeof(int)); + + NotSupportedException ex = Assert.Throws(() => JsonSerializer.Deserialize(json)); + string exAsStr = ex.ToString(); + Assert.Contains("System.Type", exAsStr); + Assert.Contains("$", exAsStr); + + json = $@"{{""Type"":{json}}}"; + + ex = Assert.Throws(() => JsonSerializer.Deserialize(json)); + exAsStr = ex.ToString(); + Assert.Contains("System.Type", exAsStr); + Assert.Contains("$.Type", exAsStr); + } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/TestClasses.cs b/src/libraries/System.Text.Json/tests/Serialization/TestClasses.cs index 7d64e125493ea4..9be040589afe29 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/TestClasses.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/TestClasses.cs @@ -1853,4 +1853,9 @@ public void Verify() MyData.Verify(); } } + + public class ClassWithType + { + public Type Type { get; set; } + } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/WriteValueTests.cs b/src/libraries/System.Text.Json/tests/Serialization/WriteValueTests.cs index fb84ada75914bc..1fec944e096587 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/WriteValueTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/WriteValueTests.cs @@ -394,5 +394,47 @@ public static void SerializeExceedMaximumBufferSize() Assert.Throws(() => JsonSerializer.Serialize(temp, typeof(CustomClassToExceedMaxBufferSize))); } + + [Fact] + public static void SerializeTypeInstance() + { + Type type = typeof(int); + + string serialized = JsonSerializer.Serialize(type); + Assert.Contains("System.Int32", serialized); + Assert.Contains("Version=", serialized); + Assert.Contains("Culture=", serialized); + Assert.Contains("PublicKeyToken=", serialized); + + Assert.Throws(() => JsonSerializer.Deserialize(serialized)); + + type = null; + serialized = JsonSerializer.Serialize(type); + Assert.Equal("null", serialized); + + ClassWithType obj = new ClassWithType { Type = typeof(int) }; + + serialized = JsonSerializer.Serialize(obj); + Assert.Contains(@"{""Type"":", serialized); + Assert.Throws(() => JsonSerializer.Deserialize(serialized)); + + serialized = JsonSerializer.Serialize(obj.Type); + Assert.Contains("System.Int32", serialized); + Assert.Contains("Version=", serialized); + Assert.Contains("Culture=", serialized); + Assert.Contains("PublicKeyToken=", serialized); + + obj.Type = null; + + serialized = JsonSerializer.Serialize(obj); + Assert.Equal(@"{""Type"":null}", serialized); + + obj = JsonSerializer.Deserialize(serialized); + // NSE is not thrown because the serializer handles null and sets the property to null. + Assert.Null(obj.Type); + + serialized = JsonSerializer.Serialize(obj, new JsonSerializerOptions { IgnoreNullValues = true }); + Assert.Equal(@"{}", serialized); + } } } From ad865f4cff946c580a2bb41ef468b5bf72cad30d Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Mon, 30 Mar 2020 09:42:19 -0700 Subject: [PATCH 2/3] Address review feedback --- .../src/Resources/Strings.resx | 4 +- .../Converters/Value/TypeConverter.cs | 4 +- .../Text/Json/ThrowHelper.Serialization.cs | 4 +- .../tests/Serialization/ExceptionTests.cs | 37 +++++++++++++++- .../tests/Serialization/WriteValueTests.cs | 42 ------------------- 5 files changed, 42 insertions(+), 49 deletions(-) diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index 1cb9400bb096e1..9e3b8d332f4eb4 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -506,7 +506,7 @@ Cannot allocate a buffer of size {0}. - - Deserialization of 'System.Type' instances is not supported and should be avoided since it can lead to security issues. A workaround is to use a custom converter. + + Serialization and deserialization of 'System.Type' instances are not supported and should be avoided since they can lead to security issues. \ No newline at end of file diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs index c4c40a6b12e00f..23436c91abef73 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs @@ -8,12 +8,12 @@ internal sealed class TypeConverter : JsonConverter { public override Type Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - throw ThrowHelper.GetNotSupportedException_DeserializeTypeInstanceNotSupported(); + throw ThrowHelper.GetNotSupportedException_SerializeTypeInstanceNotSupported(); } public override void Write(Utf8JsonWriter writer, Type value, JsonSerializerOptions options) { - writer.WriteStringValue(value.AssemblyQualifiedName); + throw ThrowHelper.GetNotSupportedException_SerializeTypeInstanceNotSupported(); } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index ee9547221fff2e..eaa943d19d14e5 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -34,9 +34,9 @@ public static void ThrowNotSupportedException_ConstructorMaxOf64Parameters(Const } [MethodImpl(MethodImplOptions.NoInlining)] - public static NotSupportedException GetNotSupportedException_DeserializeTypeInstanceNotSupported() + public static NotSupportedException GetNotSupportedException_SerializeTypeInstanceNotSupported() { - return new NotSupportedException(SR.DeserializeTypeInstanceNotSupported); + return new NotSupportedException(SR.SerializeTypeInstanceNotSupported); } [DoesNotReturn] diff --git a/src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs b/src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs index fce13e60f45507..a9db0e73b64854 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs @@ -530,7 +530,7 @@ public static void UnsupportedTypeFromRoot() [Fact] public static void DeserializeTypeInstance() { - string json = JsonSerializer.Serialize(typeof(int)); + string json = @"""System.Int32, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e"""; NotSupportedException ex = Assert.Throws(() => JsonSerializer.Deserialize(json)); string exAsStr = ex.ToString(); @@ -543,6 +543,41 @@ public static void DeserializeTypeInstance() exAsStr = ex.ToString(); Assert.Contains("System.Type", exAsStr); Assert.Contains("$.Type", exAsStr); + + // NSE is not thrown because the serializer handles null. + Assert.Null(JsonSerializer.Deserialize("null")); + + ClassWithType obj = JsonSerializer.Deserialize(@"{""Type"":null}"); + Assert.Null(obj.Type); + } + + [Fact] + public static void SerializeTypeInstance() + { + Type type = typeof(int); + + NotSupportedException ex = Assert.Throws(() => JsonSerializer.Serialize(type)); + string exAsStr = ex.ToString(); + Assert.Contains("System.Type", exAsStr); + Assert.Contains("$", exAsStr); + + type = null; + string serialized = JsonSerializer.Serialize(type); + Assert.Equal("null", serialized); + + ClassWithType obj = new ClassWithType { Type = typeof(int) }; + + ex = Assert.Throws(() => JsonSerializer.Serialize(obj)); + exAsStr = ex.ToString(); + Assert.Contains("System.Type", exAsStr); + Assert.Contains("$.Type", exAsStr); + + obj.Type = null; + serialized = JsonSerializer.Serialize(obj); + Assert.Equal(@"{""Type"":null}", serialized); + + serialized = JsonSerializer.Serialize(obj, new JsonSerializerOptions { IgnoreNullValues = true }); + Assert.Equal(@"{}", serialized); } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/WriteValueTests.cs b/src/libraries/System.Text.Json/tests/Serialization/WriteValueTests.cs index 1fec944e096587..fb84ada75914bc 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/WriteValueTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/WriteValueTests.cs @@ -394,47 +394,5 @@ public static void SerializeExceedMaximumBufferSize() Assert.Throws(() => JsonSerializer.Serialize(temp, typeof(CustomClassToExceedMaxBufferSize))); } - - [Fact] - public static void SerializeTypeInstance() - { - Type type = typeof(int); - - string serialized = JsonSerializer.Serialize(type); - Assert.Contains("System.Int32", serialized); - Assert.Contains("Version=", serialized); - Assert.Contains("Culture=", serialized); - Assert.Contains("PublicKeyToken=", serialized); - - Assert.Throws(() => JsonSerializer.Deserialize(serialized)); - - type = null; - serialized = JsonSerializer.Serialize(type); - Assert.Equal("null", serialized); - - ClassWithType obj = new ClassWithType { Type = typeof(int) }; - - serialized = JsonSerializer.Serialize(obj); - Assert.Contains(@"{""Type"":", serialized); - Assert.Throws(() => JsonSerializer.Deserialize(serialized)); - - serialized = JsonSerializer.Serialize(obj.Type); - Assert.Contains("System.Int32", serialized); - Assert.Contains("Version=", serialized); - Assert.Contains("Culture=", serialized); - Assert.Contains("PublicKeyToken=", serialized); - - obj.Type = null; - - serialized = JsonSerializer.Serialize(obj); - Assert.Equal(@"{""Type"":null}", serialized); - - obj = JsonSerializer.Deserialize(serialized); - // NSE is not thrown because the serializer handles null and sets the property to null. - Assert.Null(obj.Type); - - serialized = JsonSerializer.Serialize(obj, new JsonSerializerOptions { IgnoreNullValues = true }); - Assert.Equal(@"{}", serialized); - } } } From 993a649e33a99bb1c87a945210b34cdbf47c516e Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Mon, 30 Mar 2020 10:05:37 -0700 Subject: [PATCH 3/3] Remove throw helper --- .../Json/Serialization/Converters/Value/TypeConverter.cs | 4 ++-- .../src/System/Text/Json/ThrowHelper.Serialization.cs | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs index 23436c91abef73..1fae3f3a58ac86 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TypeConverter.cs @@ -8,12 +8,12 @@ internal sealed class TypeConverter : JsonConverter { public override Type Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - throw ThrowHelper.GetNotSupportedException_SerializeTypeInstanceNotSupported(); + throw new NotSupportedException(SR.SerializeTypeInstanceNotSupported); } public override void Write(Utf8JsonWriter writer, Type value, JsonSerializerOptions options) { - throw ThrowHelper.GetNotSupportedException_SerializeTypeInstanceNotSupported(); + throw new NotSupportedException(SR.SerializeTypeInstanceNotSupported); } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index eaa943d19d14e5..f43c530a6eeda1 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -33,12 +33,6 @@ public static void ThrowNotSupportedException_ConstructorMaxOf64Parameters(Const throw new NotSupportedException(SR.Format(SR.ConstructorMaxOf64Parameters, constructorInfo, type)); } - [MethodImpl(MethodImplOptions.NoInlining)] - public static NotSupportedException GetNotSupportedException_SerializeTypeInstanceNotSupported() - { - return new NotSupportedException(SR.SerializeTypeInstanceNotSupported); - } - [DoesNotReturn] [MethodImpl(MethodImplOptions.NoInlining)] public static void ThrowJsonException_DeserializeUnableToConvertValue(Type propertyType)