From db966f690d5a17e2fa812defce42d4caab4aeaa4 Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Thu, 7 Sep 2017 23:57:22 +0200 Subject: [PATCH 1/8] Make DBNull serializable --- .../System.Private.CoreLib.Shared.projitems | 1 + src/mscorlib/shared/System/DBNull.cs | 10 +++- .../shared/System/UnitySerializationHolder.cs | 58 +++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 src/mscorlib/shared/System/UnitySerializationHolder.cs diff --git a/src/mscorlib/shared/System.Private.CoreLib.Shared.projitems b/src/mscorlib/shared/System.Private.CoreLib.Shared.projitems index bd68b542f1e4..981113094015 100644 --- a/src/mscorlib/shared/System.Private.CoreLib.Shared.projitems +++ b/src/mscorlib/shared/System.Private.CoreLib.Shared.projitems @@ -486,6 +486,7 @@ + diff --git a/src/mscorlib/shared/System/DBNull.cs b/src/mscorlib/shared/System/DBNull.cs index 4f4d64bf6615..486eb72f2acc 100644 --- a/src/mscorlib/shared/System/DBNull.cs +++ b/src/mscorlib/shared/System/DBNull.cs @@ -6,17 +6,23 @@ namespace System { + [Serializable] public sealed class DBNull : ISerializable, IConvertible { private DBNull() { } - + + private DBNull(SerializationInfo info, StreamingContext context) + { + throw new NotSupportedException(SR.NotSupported_DBNullSerial); + } + public static readonly DBNull Value = new DBNull(); public void GetObjectData(SerializationInfo info, StreamingContext context) { - throw new PlatformNotSupportedException(); + UnitySerializationHolder.GetUnitySerializationInfo(info, UnitySerializationHolder.NullUnity, null, null); } public override string ToString() diff --git a/src/mscorlib/shared/System/UnitySerializationHolder.cs b/src/mscorlib/shared/System/UnitySerializationHolder.cs new file mode 100644 index 000000000000..d2aa882080d2 --- /dev/null +++ b/src/mscorlib/shared/System/UnitySerializationHolder.cs @@ -0,0 +1,58 @@ +// 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. + +using System.Runtime.Serialization; +using System.Reflection; +using System.Diagnostics.Contracts; + +namespace System +{ + // Holds classes (Empty, Null, Missing) for which we guarantee that there is only ever one instance of. +#if CORECLR + internal +#else + public // On CoreRT, this must be public because of the Reflection.Core/CoreLib divide and the need to whitelist past the ReflectionBlock. +#endif + class UnitySerializationHolder : ISerializable, IObjectReference + { + internal const int NullUnity = 0x0002; + const string DataName = "Data"; + const string UnityTypeName = "UnityType"; + const string AssemblyNameName = "AssemblyName"; + + private readonly string _data; + private readonly string _assemblyName; + private int _unityType; + + public static void GetUnitySerializationInfo(SerializationInfo info, int unityType, string data, Assembly assembly) + { + // A helper method that returns the SerializationInfo that a class utilizing + // UnitySerializationHelper should return from a call to GetObjectData. It contains + // the unityType (defined above) and any optional data (used only for the reflection + // types.) + + info.SetType(typeof(UnitySerializationHolder)); + info.AddValue(DataName, data, typeof(string)); + info.AddValue(UnityTypeName, unityType); + info.AddValue(AssemblyNameName, assembly?.FullName ?? string.Empty); + } + + public UnitySerializationHolder(SerializationInfo info, StreamingContext context) + { + if (info == null) + { + throw new ArgumentNullException(nameof(info)); + } + + _unityType = info.GetInt32(UnityTypeName); + _data = info.GetString(DataName); + _assemblyName = info.GetString(AssemblyNameName); + } + + public virtual void GetObjectData(SerializationInfo info, StreamingContext context) => + throw new NotSupportedException(SR.NotSupported_UnitySerHolder); + + public virtual object GetRealObject(StreamingContext context) => DBNull.Value; + } +} From f2706ae32e1198e7c5f0cfecb4d83da38802bb16 Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Fri, 8 Sep 2017 01:01:50 +0200 Subject: [PATCH 2/8] PR feedback --- .../shared/System/UnitySerializationHolder.cs | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/mscorlib/shared/System/UnitySerializationHolder.cs b/src/mscorlib/shared/System/UnitySerializationHolder.cs index d2aa882080d2..d9a3c07eb0c5 100644 --- a/src/mscorlib/shared/System/UnitySerializationHolder.cs +++ b/src/mscorlib/shared/System/UnitySerializationHolder.cs @@ -4,26 +4,21 @@ using System.Runtime.Serialization; using System.Reflection; -using System.Diagnostics.Contracts; namespace System { - // Holds classes (Empty, Null, Missing) for which we guarantee that there is only ever one instance of. + /// + /// Holds Null class for which we guarantee that there is only ever one instance of. + /// This only exists for backwarts compatibility with + /// #if CORECLR internal #else public // On CoreRT, this must be public because of the Reflection.Core/CoreLib divide and the need to whitelist past the ReflectionBlock. #endif - class UnitySerializationHolder : ISerializable, IObjectReference + sealed class UnitySerializationHolder : ISerializable, IObjectReference { internal const int NullUnity = 0x0002; - const string DataName = "Data"; - const string UnityTypeName = "UnityType"; - const string AssemblyNameName = "AssemblyName"; - - private readonly string _data; - private readonly string _assemblyName; - private int _unityType; public static void GetUnitySerializationInfo(SerializationInfo info, int unityType, string data, Assembly assembly) { @@ -33,9 +28,9 @@ public static void GetUnitySerializationInfo(SerializationInfo info, int unityTy // types.) info.SetType(typeof(UnitySerializationHolder)); - info.AddValue(DataName, data, typeof(string)); - info.AddValue(UnityTypeName, unityType); - info.AddValue(AssemblyNameName, assembly?.FullName ?? string.Empty); + info.AddValue("Data", data, typeof(string)); + info.AddValue("UnityType", unityType); + info.AddValue("AssemblyName", assembly?.FullName ?? string.Empty); } public UnitySerializationHolder(SerializationInfo info, StreamingContext context) @@ -44,15 +39,16 @@ public UnitySerializationHolder(SerializationInfo info, StreamingContext context { throw new ArgumentNullException(nameof(info)); } - - _unityType = info.GetInt32(UnityTypeName); - _data = info.GetString(DataName); - _assemblyName = info.GetString(AssemblyNameName); } - public virtual void GetObjectData(SerializationInfo info, StreamingContext context) => + public void GetObjectData(SerializationInfo info, StreamingContext context) => throw new NotSupportedException(SR.NotSupported_UnitySerHolder); - public virtual object GetRealObject(StreamingContext context) => DBNull.Value; + public object GetRealObject(StreamingContext context) + { + // We are always returning the same DBNull instance and ignoring serialization input. + + return DBNull.Value; + } } } From 29408dcf9c2b00c7e2a77fa9ab41a4fdf9f59b0b Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Fri, 8 Sep 2017 01:04:15 +0200 Subject: [PATCH 3/8] More comments and remove of corert special case --- src/mscorlib/shared/System/UnitySerializationHolder.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/mscorlib/shared/System/UnitySerializationHolder.cs b/src/mscorlib/shared/System/UnitySerializationHolder.cs index d9a3c07eb0c5..557a25cc808b 100644 --- a/src/mscorlib/shared/System/UnitySerializationHolder.cs +++ b/src/mscorlib/shared/System/UnitySerializationHolder.cs @@ -11,12 +11,7 @@ namespace System /// Holds Null class for which we guarantee that there is only ever one instance of. /// This only exists for backwarts compatibility with /// -#if CORECLR - internal -#else - public // On CoreRT, this must be public because of the Reflection.Core/CoreLib divide and the need to whitelist past the ReflectionBlock. -#endif - sealed class UnitySerializationHolder : ISerializable, IObjectReference + internal sealed class UnitySerializationHolder : ISerializable, IObjectReference { internal const int NullUnity = 0x0002; @@ -39,6 +34,8 @@ public UnitySerializationHolder(SerializationInfo info, StreamingContext context { throw new ArgumentNullException(nameof(info)); } + + // We are ignoring any serialization input as we are only concerned about DBNull. } public void GetObjectData(SerializationInfo info, StreamingContext context) => From b142440f83ba0c11ea14cd266cfb657fbd119395 Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Fri, 8 Sep 2017 01:12:19 +0200 Subject: [PATCH 4/8] Revert access modifier change and update its comment --- src/mscorlib/shared/System/UnitySerializationHolder.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/mscorlib/shared/System/UnitySerializationHolder.cs b/src/mscorlib/shared/System/UnitySerializationHolder.cs index 557a25cc808b..ffa5dd2c856f 100644 --- a/src/mscorlib/shared/System/UnitySerializationHolder.cs +++ b/src/mscorlib/shared/System/UnitySerializationHolder.cs @@ -11,7 +11,12 @@ namespace System /// Holds Null class for which we guarantee that there is only ever one instance of. /// This only exists for backwarts compatibility with /// - internal sealed class UnitySerializationHolder : ISerializable, IObjectReference +#if CORECLR + internal +#else + public // On CoreRT this must be public. +#endif + sealed class UnitySerializationHolder : ISerializable, IObjectReference { internal const int NullUnity = 0x0002; From 35f886799786dd7571747b15cd3e8c88b55cd9fd Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Fri, 8 Sep 2017 01:25:51 +0200 Subject: [PATCH 5/8] Add exception handling for other input and add comments --- .../shared/System/UnitySerializationHolder.cs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/mscorlib/shared/System/UnitySerializationHolder.cs b/src/mscorlib/shared/System/UnitySerializationHolder.cs index ffa5dd2c856f..c2261473c540 100644 --- a/src/mscorlib/shared/System/UnitySerializationHolder.cs +++ b/src/mscorlib/shared/System/UnitySerializationHolder.cs @@ -19,14 +19,15 @@ namespace System sealed class UnitySerializationHolder : ISerializable, IObjectReference { internal const int NullUnity = 0x0002; + private readonly int _unityType; + /// + /// A helper method that returns the SerializationInfo that a class utilizing + /// UnitySerializationHelper should return from a call to GetObjectData. It contains + /// the unityType (defined above) and any optional data (used only for the reflection types). + /// public static void GetUnitySerializationInfo(SerializationInfo info, int unityType, string data, Assembly assembly) { - // A helper method that returns the SerializationInfo that a class utilizing - // UnitySerializationHelper should return from a call to GetObjectData. It contains - // the unityType (defined above) and any optional data (used only for the reflection - // types.) - info.SetType(typeof(UnitySerializationHolder)); info.AddValue("Data", data, typeof(string)); info.AddValue("UnityType", unityType); @@ -40,7 +41,8 @@ public UnitySerializationHolder(SerializationInfo info, StreamingContext context throw new ArgumentNullException(nameof(info)); } - // We are ignoring any serialization input as we are only concerned about DBNull. + // We are ignoring any other serialization input as we are only concerned about DBNull. + _unityType = info.GetInt32("UnityType"); } public void GetObjectData(SerializationInfo info, StreamingContext context) => @@ -48,8 +50,13 @@ public void GetObjectData(SerializationInfo info, StreamingContext context) => public object GetRealObject(StreamingContext context) { - // We are always returning the same DBNull instance and ignoring serialization input. + // We are only support deserializing DBNull and throwing for everything else. + if (_unityType != NullUnity) + { + throw new ArgumentException(SR.Argument_InvalidUnity); + } + // We are always returning the same DBNull instance. return DBNull.Value; } } From 423d10d5c56bd2e45050a478af2068a3fd70f4f2 Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Fri, 8 Sep 2017 01:36:46 +0200 Subject: [PATCH 6/8] Remove unused params --- src/mscorlib/shared/System/DBNull.cs | 2 +- src/mscorlib/shared/System/UnitySerializationHolder.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mscorlib/shared/System/DBNull.cs b/src/mscorlib/shared/System/DBNull.cs index 486eb72f2acc..3cee2b15c8d9 100644 --- a/src/mscorlib/shared/System/DBNull.cs +++ b/src/mscorlib/shared/System/DBNull.cs @@ -22,7 +22,7 @@ private DBNull(SerializationInfo info, StreamingContext context) public void GetObjectData(SerializationInfo info, StreamingContext context) { - UnitySerializationHolder.GetUnitySerializationInfo(info, UnitySerializationHolder.NullUnity, null, null); + UnitySerializationHolder.GetUnitySerializationInfo(info, UnitySerializationHolder.NullUnity); } public override string ToString() diff --git a/src/mscorlib/shared/System/UnitySerializationHolder.cs b/src/mscorlib/shared/System/UnitySerializationHolder.cs index c2261473c540..80de8c10347f 100644 --- a/src/mscorlib/shared/System/UnitySerializationHolder.cs +++ b/src/mscorlib/shared/System/UnitySerializationHolder.cs @@ -26,12 +26,12 @@ sealed class UnitySerializationHolder : ISerializable, IObjectReference /// UnitySerializationHelper should return from a call to GetObjectData. It contains /// the unityType (defined above) and any optional data (used only for the reflection types). /// - public static void GetUnitySerializationInfo(SerializationInfo info, int unityType, string data, Assembly assembly) + internal static void GetUnitySerializationInfo(SerializationInfo info, int unityType) { info.SetType(typeof(UnitySerializationHolder)); - info.AddValue("Data", data, typeof(string)); + info.AddValue("Data", null, typeof(string)); info.AddValue("UnityType", unityType); - info.AddValue("AssemblyName", assembly?.FullName ?? string.Empty); + info.AddValue("AssemblyName", string.Empty); } public UnitySerializationHolder(SerializationInfo info, StreamingContext context) From b9a716a7cc4cedcc8e1f7c262b9a0594b9bad3cb Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Fri, 8 Sep 2017 16:56:25 +0200 Subject: [PATCH 7/8] PR feedback --- src/mscorlib/Resources/Strings.resx | 2 +- src/mscorlib/shared/System/UnitySerializationHolder.cs | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/mscorlib/Resources/Strings.resx b/src/mscorlib/Resources/Strings.resx index 3bd9cd2ef00d..12c6f11264b9 100644 --- a/src/mscorlib/Resources/Strings.resx +++ b/src/mscorlib/Resources/Strings.resx @@ -1274,7 +1274,7 @@ Cannot use type '{0}'. Only value types without pointers or references are supported. - Invalid Unity type. + Type '{0}' is not deserializable. Value was invalid. diff --git a/src/mscorlib/shared/System/UnitySerializationHolder.cs b/src/mscorlib/shared/System/UnitySerializationHolder.cs index 80de8c10347f..ad8364711881 100644 --- a/src/mscorlib/shared/System/UnitySerializationHolder.cs +++ b/src/mscorlib/shared/System/UnitySerializationHolder.cs @@ -9,7 +9,7 @@ namespace System { /// /// Holds Null class for which we guarantee that there is only ever one instance of. - /// This only exists for backwarts compatibility with + /// This only exists for compatibility with .NET Framework. /// #if CORECLR internal @@ -20,6 +20,7 @@ sealed class UnitySerializationHolder : ISerializable, IObjectReference { internal const int NullUnity = 0x0002; private readonly int _unityType; + private readonly string _data; /// /// A helper method that returns the SerializationInfo that a class utilizing @@ -42,7 +43,9 @@ public UnitySerializationHolder(SerializationInfo info, StreamingContext context } // We are ignoring any other serialization input as we are only concerned about DBNull. + // We also store data and use it for erorr logging. _unityType = info.GetInt32("UnityType"); + _data = info.GetString("Data"); } public void GetObjectData(SerializationInfo info, StreamingContext context) => @@ -53,7 +56,7 @@ public object GetRealObject(StreamingContext context) // We are only support deserializing DBNull and throwing for everything else. if (_unityType != NullUnity) { - throw new ArgumentException(SR.Argument_InvalidUnity); + throw new ArgumentException(SR.Format(SR.Argument_InvalidUnity, _data ?? "UnityType")); } // We are always returning the same DBNull instance. From d19b3accedd3f08d1322015e065528aefc138fe5 Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Fri, 8 Sep 2017 18:35:23 +0200 Subject: [PATCH 8/8] Adding serializable attribute to type --- src/mscorlib/shared/System/UnitySerializationHolder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mscorlib/shared/System/UnitySerializationHolder.cs b/src/mscorlib/shared/System/UnitySerializationHolder.cs index ad8364711881..bbfebff8a680 100644 --- a/src/mscorlib/shared/System/UnitySerializationHolder.cs +++ b/src/mscorlib/shared/System/UnitySerializationHolder.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System.Runtime.Serialization; -using System.Reflection; namespace System { @@ -11,6 +10,7 @@ namespace System /// Holds Null class for which we guarantee that there is only ever one instance of. /// This only exists for compatibility with .NET Framework. /// + [Serializable] #if CORECLR internal #else