-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move away from BinaryFormatter for ActiveX purposes #9104
Conversation
The PropertyBags now serialize without it where possible. This allows a standard Control to completely serialize without the BinaryFormatter enabled. For the AxHost case, I've left the BinaryFormatter as a fall-back. I've also extended the Hashtable serialization to handle all primitive types. Note the two line change to make this happen. :) This change also moves the "Reader" methods to "BinaryFormattedObjectExtensions". Due to our layering and the need to check multiple states I realized that we need to have all of the reading of objects directly using an existing BinaryFormattedObject instance. Fixes a few bugs and adds more tests. Some random cleanup related to VARIANT (as I was exploring the scope of things).
src/System.Windows.Forms.Primitives/src/System/Windows/Forms/BinaryFormat/BinaryFormatWriter.cs
Show resolved
Hide resolved
src/System.Windows.Forms.Primitives/src/System/Windows/Forms/BinaryFormat/BinaryFormatReader.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Had a question around Variant.
@@ -938,7 +938,7 @@ private static Span<T> GetSpan<T>(Array array) | |||
private static T ThrowInvalidCast<T>() => throw new InvalidCastException(); | |||
|
|||
/// <summary> | |||
/// Converts the given object to <see cref="VARIANT"/>. WARNING: Only handles <see cref="string"/>. | |||
/// Converts the given object to <see cref="VARIANT"/>. | |||
/// </summary> | |||
public static VARIANT FromObject(object? value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal question for my better personal understanding:
Is this for the VARIANT type that has been used in the context of VB6?
If yes, there once was a DBNull
subtype also, if I (vaguely) remember correctly (and I can be completely wrong). I am just mentioning it, and maybe it needs to be taken into consideration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there is a special null type. It's "handled" for the moment in that we fall back to the .NET interop conversion here. What we have to do is get the rest of this implemented on our side for AOT purposes.
The PropertyBags now serialize without it where possible. This allows a standard Control to completely serialize without the BinaryFormatter enabled.
For the AxHost case, I've left the BinaryFormatter as a fall-back.
I've also extended the Hashtable serialization to handle all primitive types. Note the two line change to make this happen. :)
This change also moves the "Reader" methods to "BinaryFormattedObjectExtensions". Due to our layering and the need to check multiple states I realized that we need to have all of the reading of objects directly using an existing BinaryFormattedObject instance.
Fixes a few bugs and adds more tests. Some random cleanup related to VARIANT (as I was exploring the scope of things).
Microsoft Reviewers: Open in CodeFlow