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

Force EnableUnsafeBinaryFormatterSerialization using SetSwitch - BinaryFormatter Issue #1349

Merged
merged 5 commits into from
Jul 14, 2024

Conversation

ricaun
Copy link
Contributor

@ricaun ricaun commented Mar 14, 2024

Hello everyone,

This PR forces to EnableUnsafeBinaryFormatterSerialization in runtime instead of adding the configuration in the csproj.

This is done by using the code below before using the BinaryFormatter. (Reference: pythonnet/pythonnet#2282)

AppContext.SetSwitch("System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization", true);

I add a simple test in the Nuke.Tooling.Tests and changes to NET 8.0 to make sure the BinaryFormatter gonna work without any EnableUnsafeBinaryFormatterSerialization in the csproj.

This gonna make sure the BinaryFormatter gonna works regardless if has or hasn't the configuration in the csproj.

And just to remind that theBinaryFormatter gonna be removed in the next .NET Core (.NET 9.0 - dotnet/announcements#293)


I confirm that the pull-request:

  • Follows the contribution guidelines
  • Is based on my own work
  • Is in compliance with my employer

@matkoch
Copy link
Member

matkoch commented Mar 16, 2024

What exactly is the benefit compared to the auto-loaded .props file?

@ricaun
Copy link
Contributor Author

ricaun commented Mar 16, 2024

The .props is goes only to the main csproj that referencing the Nuke.Commum.

This mean if I create a package with the dependencies Nuke.Commum or Nuke.Tooling, the .props does not progate to the main csproj that use the package.

Here is my package the uses that. https://www.nuget.org/packages/ricaun.Nuke#dependencies-body-tab

Now I could add the .props in my ricaun.Nuke package, or try to fix in the Nuke.Tooling to force enable the switch on runtime fixing the issue with net8.0.

The benefits would be using the Nuke.Tooling without any extra confuguratuon in the csproj or .props.

@ITaluone
Copy link
Contributor

This looks like the proper solution (especially for projects which using/extending Nuke.*)

@matkoch matkoch force-pushed the develop branch 4 times, most recently from 0719711 to 6fe1e3b Compare March 20, 2024 01:24
@ricaun
Copy link
Contributor Author

ricaun commented Mar 28, 2024

At the moment I gonna use this in my package.

By default when the DLL is load the module switch the EnableUnsafeBinaryFormatterSerialization to true.

@matkoch matkoch force-pushed the develop branch 2 times, most recently from 6d02194 to 4bfee84 Compare June 13, 2024 21:33
@matkoch matkoch force-pushed the develop branch 4 times, most recently from 85998da to 01b2b6e Compare July 9, 2024 00:20
@matkoch matkoch added this to the v8.1.0 milestone Jul 12, 2024
@matkoch matkoch merged commit 5be064d into nuke-build:develop Jul 14, 2024
3 of 4 checks passed
@ScarletKuro
Copy link

ScarletKuro commented Nov 15, 2024

Sorry for necro posting, but setting

AppContext.SetSwitch("System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization", isEnabled: true);

Is not enough, at least for the net9 SDK as the code for binary formatter is missing there.
You also need to include the package

<ItemGroup>
    <PackageReference Include="System.Runtime.Serialization.Formatters" Version="9.0.0" />
</ItemGroup>

@ricaun
Copy link
Contributor Author

ricaun commented Nov 15, 2024

@ScarletKuro adding System.Runtime.Serialization.Formatters in .NET 9 make binary formatter to work?

@ScarletKuro
Copy link

ScarletKuro commented Nov 15, 2024

@ScarletKuro adding System.Runtime.Serialization.Formatters in .NET 9 make binary formatter to work?

Yes.

MSFT talked about the removal of BinaryFormatter from .NET 9 a long time ago and providing compatibility package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants