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

Make DBNull serializable #13845

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db966f6
Make DBNull serializable
ViktorHofer Sep 7, 2017
f2706ae
PR feedback
ViktorHofer Sep 7, 2017
29408dc
More comments and remove of corert special case
ViktorHofer Sep 7, 2017
b142440
Revert access modifier change and update its comment
ViktorHofer Sep 7, 2017
35f8867
Add exception handling for other input and add comments
ViktorHofer Sep 7, 2017
423d10d
Remove unused params
ViktorHofer Sep 7, 2017
b9a716a
PR feedback
ViktorHofer Sep 8, 2017
d19b3ac
Adding serializable attribute to type
ViktorHofer Sep 8, 2017
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
2 changes: 1 addition & 1 deletion src/mscorlib/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,7 @@
<value>Cannot use type '{0}'. Only value types without pointers or references are supported.</value>
</data>
<data name="Argument_InvalidUnity" xml:space="preserve">
<value>Invalid Unity type.</value>
<value>Type '{0}' is not deserializable.</value>
</data>
<data name="Argument_InvalidValue" xml:space="preserve">
<value>Value was invalid.</value>
Expand Down
7 changes: 5 additions & 2 deletions src/mscorlib/shared/System/UnitySerializationHolder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ 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
/// This only exists for compatibility with .NET Framework.
/// </summary>
#if CORECLR
internal
Expand All @@ -20,6 +20,7 @@ sealed class UnitySerializationHolder : ISerializable, IObjectReference
{
internal const int NullUnity = 0x0002;
private readonly int _unityType;
private readonly string _data;

/// <summary>
/// A helper method that returns the SerializationInfo that a class utilizing
Expand All @@ -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) =>
Expand All @@ -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"));
Copy link
Member

Choose a reason for hiding this comment

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

throw new ArgumentException(SR.Format(SR.Argument_InvalidUnity, _data ?? _unityType ?? "UnityType"));

}

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.

// We are always returning the same DBNull instance.
Expand Down