Skip to content
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

Use multiple target frameworks for Eto.Forms #2439

Merged
merged 6 commits into from
May 19, 2023
Merged

Conversation

Miepee
Copy link
Contributor

@Miepee Miepee commented Mar 6, 2023

Fixes #2248

@Miepee
Copy link
Contributor Author

Miepee commented Mar 6, 2023

error SYSLIB0011: 'BinaryFormatter.Serialize(Stream, object)' is obsolete: 'BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.'

I dunno how the binary formatter is used in Eto, from what I can tell, only for getting/setting the clipboard value and something related to drag-n-drop.
What's your recommendation on going forward here @cwensley ? As far as I can see there is

  1. Use something else than a binary formatter. Not sure what to use here, since MS recommendations are either for primitives, or for XML/Json serialization. Protobuf could be an option, but that's then putting another library into Eto for a feature not too many probably use.
  2. Treat error as warning and continue as usual, and make it clear that using the feature is risky.
  3. Get rid of the feature entirely. API breaking and probably worst option, since the feature does have some genuine neat uses.

Keep in mind that the BinaryFormatter in general is insecure, so this affects Eto even if we keep only the .netstandard 2.0 TFM.

@cwensley
Copy link
Member

cwensley commented Mar 17, 2023

Hey @Miepee, thanks for the PR! Looking into this, I think moving over to XmlSerialization would be best as it doesn't bring in any extra dependencies. We'll have to serialize/deserialize the type along with the actual object data, but it should be fairly straight forward.

This functionality is only provided so it is easy to pass a .NET object to/from the clipboard/dataobject easily so as long as it follows the normal serialization process it should work similarly to before.

The only scenario I can think of that wouldn't work moving to a different serialization mechanism is if someone is using an object with private members that weren't implemented via the serialization API. I can live with breaking this functionality considering the alternatives.

Thanks again for the contribution!

@cwensley
Copy link
Member

Another observation: a lot of the obsolete type converters were moved over to using System.ComponentModel.TypeConverter as a base vs. using Eto.TypeConverter, likely due to NETSTANDARD not being defined. This should be reverted.

@Miepee
Copy link
Contributor Author

Miepee commented Apr 5, 2023

@cwensley Think i fixed both of these issues now.
Was surprised switching to XML was that simple, but I also haven't done that expensive testing on it. Only tried the clipboard behaviour test in Eto.Test, and I can still paste text and images, so I assume it's fine.

@cwensley
Copy link
Member

Just to follow up, I've tested the GetObject/SetObject with these changes and it is completely borked. There are no tests that exercise this behaviour which I will try to add to make doing these changes easier. The easiest right now would be to use the deprecated functionality even in .NET 6.

@Miepee
Copy link
Contributor Author

Miepee commented May 17, 2023

Went ahead and temporarily disabled the warnings/errors then in order for this to get merged sooner, as a few other PRs I have in my backlog rely on this.

@cwensley
Copy link
Member

Great, thanks for the tweaks! I'll try to get those unit tests in there before we need to change the BinaryFormatter out

@cwensley cwensley merged commit 8fe54c2 into picoe:develop May 19, 2023
3 checks passed
@cwensley cwensley added this to the 2.8.0 milestone May 19, 2023
@Miepee Miepee deleted the tf branch May 19, 2023 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple target frameworks for Eto.Forms
2 participants