Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Make DBNull serializable #13845

Merged
merged 8 commits into from
Sep 8, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@
<Compile Include="$(MSBuildThisFileDirectory)System\UnauthorizedAccessException.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\UnhandledExceptionEventArgs.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\UnhandledExceptionEventHandler.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\UnitySerializationHolder.cs"/>
<Compile Include="$(MSBuildThisFileDirectory)System\ValueTuple.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Version.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Void.cs" />
Expand Down
10 changes: 8 additions & 2 deletions src/mscorlib/shared/System/DBNull.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to add the string

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, it's already there by mistake...

}

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()
Expand Down
56 changes: 56 additions & 0 deletions src/mscorlib/shared/System/UnitySerializationHolder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// 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;

namespace System
{
/// <summary>
/// Holds Null class for which we guarantee that there is only ever one instance of.
/// This only exists for backwarts compatibility with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backwarts is a new word .. and the sentence isn't complete

/// </summary>
#if CORECLR
internal
#else
public // On CoreRT this must be public.
#endif
sealed class UnitySerializationHolder : ISerializable, IObjectReference
{
internal const int NullUnity = 0x0002;

public static void GetUnitySerializationInfo(SerializationInfo info, int unityType, string data, Assembly assembly)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetUnitySerializationInfo can be made internal and the "data" and "assembly" parameters serve no purpose anymore.

Aside from that, LGTM.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forget my comment. was wrong.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't DBNull be able to access it? It lives in the same assembly as UnitySerializationHolder.

{
// 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);
info.AddValue("AssemblyName", assembly?.FullName ?? string.Empty);
}

public UnitySerializationHolder(SerializationInfo info, StreamingContext context)
{
if (info == null)
{
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) =>
throw new NotSupportedException(SR.NotSupported_UnitySerHolder);

public object GetRealObject(StreamingContext context)
{
// We are always returning the same DBNull instance and ignoring serialization input.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're still trying to be compatible with legacy blobs, should we at least throw if "UnityType" isn't NullUnity?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely should throw as otherwise users would just get a random object. But before I can do that I need to verify the weird behavior I posted above. Thanks!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SerializationInfo isn't an ordered list - it's a dictionary of name-value pairs. So there's no problem.

return DBNull.Value;
}
}
}