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

Migrate away from BinaryFormatter? #818

Closed
tyb-dev opened this issue Dec 1, 2021 · 25 comments
Closed

Migrate away from BinaryFormatter? #818

tyb-dev opened this issue Dec 1, 2021 · 25 comments

Comments

@tyb-dev
Copy link
Contributor

tyb-dev commented Dec 1, 2021

Usage Information

6.0.0-beta0001

Relevant Code / Invocations

Nuke.Common.Tooling.SettingsEntityExtensions.NewInstance

Expected Behavior

No exception

What actually happened?

Exception while using Docker Nuke wrapper

Stacktrace / Log

System.NotSupportedException: BinaryFormatter serialization and deserialization are disabled within this application. See https://aka.ms/binaryformatter for more information.
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream , Object )
   at Nuke.Common.Tooling.SettingsEntityExtensions.NewInstance[T](T settingsEntity)
   at Nuke.Common.Tools.Docker.DockerLoginSettingsExtensions.SetUsername[T](T toolSettings, String username)
   at Build.<get_Login>b__11_5(DockerLoginSettings s) in /src/Apps/DebianPipeline/Build.cs:line 56
   at Nuke.Common.Tools.Docker.DockerTasks.DockerLogin(Configure`1 configurator)
   at Build.<get_Login>b__11_4() in /src/Apps/DebianPipeline/Build.cs:line 56
   at Nuke.Common.Execution.BuildExecutor.<>c.<Execute>b__4_2(Action x)
   at Nuke.Common.Utilities.Collections.EnumerableExtensions.ForEach[T](IEnumerable`1 enumerable, Action`1 action)
   at Nuke.Common.Execution.BuildExecutor.Execute(NukeBuild build, ExecutableTarget target, IReadOnlyCollection`1 previouslyExecutedTargets, Boolean failureMode)

Anything else we should know?

BinaryFormatters are prohibited under certain circumstances. Although this usage within SettingsEntityExtensions arguably has no security implications, the BinaryFormatter might not be available generally for the entire app, which restricts usage of Nuke.

https://docs.microsoft.com/en-us/dotnet/core/compatibility/core-libraries/5.0/binaryformatter-serialization-obsolete

@tyb-dev
Copy link
Contributor Author

tyb-dev commented Dec 1, 2021

It's more a limitation than a bug, but it's not exactly a new feature either, which is why I created a bug.

@tyb-dev
Copy link
Contributor Author

tyb-dev commented Dec 1, 2021

I used .NET 6.0 and I have the following properties in my csproj, which might influence the situation.

	<PublishSingleFile>true</PublishSingleFile>
	<SelfContained>true</SelfContained>
	<RuntimeIdentifier>linux-x64</RuntimeIdentifier>
	<PublishReadyToRun>true</PublishReadyToRun>
	<PublishTrimmed>true</PublishTrimmed>

@tyb-dev tyb-dev changed the title Migrate away away from BinaryFormatter Migrate away from BinaryFormatter? Dec 1, 2021
@matkoch matkoch added this to the v6.1.0 milestone Dec 8, 2021
@matkoch
Copy link
Member

matkoch commented Feb 10, 2022

Just a heads-up: I crossed this before too and plan to replace with JSON serialization.

@matkoch
Copy link
Member

matkoch commented Jun 7, 2022

var newInstance = JsonConvert.DeserializeObject<T>(JsonConvert.SerializeObject(settingsEntity));
[JsonObjectAttribute(MemberSerialization.Fields)]
class Settings {}

@matkoch matkoch removed this from the v6.1.0 milestone Jun 14, 2022
@mahranben
Copy link

It's more a limitation than a bug, but it's not exactly a new feature either, which is why I created a bug.

@tyb-dev have you found a workaround ? I have the same problem with a selfcontained
I tried true and noWarn but still does not work for me

@tyb-dev
Copy link
Contributor Author

tyb-dev commented Jun 26, 2023

It's more a limitation than a bug, but it's not exactly a new feature either, which is why I created a bug.

@tyb-dev have you found a workaround ? I have the same problem with a selfcontained I tried true and noWarn but still does not work for me

Not without changing Nuke itself.

@mahranben
Copy link

It's more a limitation than a bug, but it's not exactly a new feature either, which is why I created a bug.

@tyb-dev have you found a workaround ? I have the same problem with a selfcontained I tried true and noWarn but still does not work for me

Not without changing Nuke itself.

I managed to workaround it by disabling "PublishTrimmed". It's twice as big as my ordinary binary but that should do it for now until the change is made.
Thanks !!

@voroninp
Copy link

voroninp commented Aug 7, 2023

In .NET 8 preview docker Restore step throws.

https://learn.microsoft.com/en-us/dotnet/core/compatibility/serialization/8.0/binaryformatter-disabled

Starting in .NET 8, the affected methods throw a NotSupportedException at run time across all project types except Windows Forms and WPF. The APIs continue to remain obsolete (as error) across all project types, including Windows Forms and WPF.

[ERR] Target Restore has thrown an exception
System.NotSupportedException: BinaryFormatter serialization and deserialization are disabled within this application. See https://aka.ms/binaryformatter for more information.
at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializationStream, Object graph)
at Nuke.Common.Tooling.SettingsEntityExtensions.NewInstance[T](T settingsEntity) in /_/source/Nuke.Common/Tooling/SettingsEntity.NewInstance.cs:line 22
at Nuke.Common.Tools.DotNet.DotNetRestoreSettingsExtensions.SetProjectFile[T](T toolSettings, String projectFile) in /_/source/Nuke.Common/Tools/DotNet/DotNet.Generated.cs:line 6526
at Build.<get_Restore>b__7_2(DotNetRestoreSettings _) in /build/build/Build.cs:line 45
at Nuke.Common.Tools.DotNet.DotNetTasks.DotNetRestore(Configure`1 configurator) in /_/source/Nuke.Common/Tools/DotNet/DotNet.Generated.cs:line 361
at Build.<get_Restore>b__7_1() in /build/build/Build.cs:line 45
at Nuke.Common.Execution.BuildExecutor.<>c.<Execute>b__4_2(Action x) in /_/source/Nuke.Common/Execution/BuildExecutor.cs:line 112
at Nuke.Common.Utilities.Collections.EnumerableExtensions.ForEach[T](IEnumerable`1 enumerable, Action`1 action) in /_/source/Nuke.Common/Utilities/Collections/Enumerable.ForEach.cs:line 17
at Nuke.Common.Execution.BuildExecutor.Execute(NukeBuild build, ExecutableTarget target, IReadOnlyCollection`1 previouslyExecutedTargets, Boolean failureMode) in /_/source/Nuke.Common/Execution/BuildExecutor.cs:line 112

@MichaelKetting
Copy link

@voroninp you can use <EnableUnsafeBinaryFormatterSerialization>true</EnableUnsafeBinaryFormatterSerialization> for the build project.

@matkoch just a head's up: starting with .NET 9, binary serialization will be removed (according to the current roadmap). Is this something where a contribution would help / be feasiable?

@cristipufu
Copy link

Please take a look: #1247

@borland
Copy link

borland commented Nov 20, 2023

Workaround taken from #1282 which has been closed as a duplicate of this bug:

Add this to your Nuke project's csproj file:

<PropertyGroup>
   .. other existing properties ...
  <EnableUnsafeBinaryFormatterSerialization>true</EnableUnsafeBinaryFormatterSerialization>
</PropertyGroup>

Note: Update Oct2024: this workaround only applies to dotnet 8. For dotnet 9, BinaryFormatter has been completely removed and this has no effect. Others have posted updated workarounds below

gustavocalheiros added a commit to gustavocalheiros/humidifier that referenced this issue Jan 3, 2024
@Tridy
Copy link

Tridy commented Jan 15, 2024

Would something like this solve it?


[PublicAPI]
public static partial class SettingsEntityExtensions
{
    public static T NewInstance<T>(this T settingsEntity)
        where T : ISettingsEntity
    {
        T newInstance = DeepCopyWithJson(settingsEntity);
        if (newInstance is ToolSettings toolSettings)
        {
            toolSettings.ProcessArgumentConfigurator = ((ToolSettings) (object) settingsEntity).ProcessArgumentConfigurator;
            toolSettings.ProcessLogger = ((ToolSettings) (object) settingsEntity).ProcessLogger;
            toolSettings.ProcessExitHandler = ((ToolSettings) (object) settingsEntity).ProcessExitHandler;
        }

        return newInstance;
    }
    
    public static T DeepCopyWithJson<T>(T obj) where T: ISettingsEntity
    {
        string json = JsonSerializer.Serialize(obj);
        return JsonSerializer.Deserialize<T>(json);
    }
}

@MoeHamdan
Copy link

When will this be fixed?

@jasontstone
Copy link

jasontstone commented Apr 2, 2024

The EnableUnsafeBinaryFormatterSerialization workaround is not working on at least the latest .NET 8 SDK, so a fix would be really appreciated.

@rogerbriggen-securiton
Copy link

With dotnet 9 Preview 6, BinaryFormatter is gone.

67.29 14:30:09 [ERR] Target Restore has thrown an exception
67.36 System.PlatformNotSupportedException: BinaryFormatter serialization and deserialization have been removed. See https://aka.ms/binaryformatter for more information.
67.36 at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializationStream, Object graph)
67.36 at Nuke.Common.Tooling.SettingsEntityExtensions.NewInstance[T](T settingsEntity) in //source/Nuke.Tooling/SettingsEntity.NewInstance.cs:line 23
67.36 at Nuke.Common.Tools.DotNet.DotNetRestoreSettingsExtensions.SetProjectFile[T](T toolSettings, String projectFile) in /
/source/Nuke.Common/Tools/DotNet/DotNet.Generated.cs:line 7516

@nsentinel
Copy link

@matkoch As a temporary solution, if migration is difficult to .NET 9, you can use the package System.Runtime.Serialization.Formatters (BinaryFormatter has been moved here)

@rogerbriggen-securiton
Copy link

@matkoch As a temporary solution, if migration is difficult to .NET 9, you can use the package System.Runtime.Serialization.Formatters (BinaryFormatter has been moved here)

Thanks @nsentinel, that seems to work

@nsentinel
Copy link

BinaryFormatter removal from .NET 9 is complete: dotnet/announcements#317

@jjaskulowski-configit
Copy link

+1 cannot use nuke in net9

@nsentinel
Copy link

nsentinel commented Aug 27, 2024

@jjaskulowski-configit As a temporary workaround, you can set .NET 8 target for nuke (_build project), and .NET 9 for everything else. nuke will build such solution fine

@borland
Copy link

borland commented Oct 19, 2024

As a temporary solution, if migration is difficult to .NET 9, you can use the package System.Runtime.Serialization.Formatters (BinaryFormatter has been moved here)

Here is Microsoft's https://learn.microsoft.com/en-us/dotnet/standard/serialization/binaryformatter-migration-guide/compatibility-package

Please note that this package doesn't change the type identity of BinaryFormatter. Existing libraries don't need to be updated to depend on this package in order to use it. The only place that needs to depend on this package is the application project.

I added <PackageReference Include="System.Runtime.Serialization.Formatters" Version="9.0.0-rc.2.24473.5" /> to my nuke build project (which is using an unmodified Nuke 8.1.2) and things work.

The first time I attempted this, I just had <PackageReference Include="System.Runtime.Serialization.Formatters" />, but this does not fix it. The specific reference to Version="9.0.0-rc.2.24473.5" is required or it gets an older version of the package.

Note: If you are reading this in the future when .NET 9 is generally available, please reference the 9.0 packages and not RC2.

As a temporary workaround, you can set .NET 8 target for nuke (_build project), and .NET 9 for everything else. nuke will build such solution fine

I found this also worked for me.

The downside is that it requires both .NET 8 and 9 SDK's to be installed in CI environments.
We had been using nuke's build.sh and build.ps1 bootstrappers to install the .NET sdk -- These pick the version out of global.json which now says dotnet 9 so an extra step is needed to also install dotnet 8

I will proceed with the System.Runtime.Serialization.Formatters workaround until such time as Nuke is ready to remove the reference on BinaryFormatter

@nsentinel
Copy link

Another possible solution to avoid BinaryFormatter is to use Typeless mode in MessagePack - https://github.com/MessagePack-CSharp/MessagePack-CSharp?tab=readme-ov-file#typeless.

If the option of saving to JSON is too inconvenient for some reason

@matkoch matkoch added this to the v8.2.0 milestone Nov 11, 2024
@matkoch matkoch closed this as completed Nov 11, 2024
@jrch-seges
Copy link

@matkoch why would you add it to the milestone and close it? I can't find an "open" task or PR for it, and the binaryformatter is still there

@ScarletKuro
Copy link

ScarletKuro commented Nov 15, 2024

I upgraded package to 8.1.4 and set <TargetFramework>net9.0</TargetFramework> and still get the binaryformatter compilation error.
So seems not to be fixed, at least in the latest public nuget (I didn't find any nightly builds, and it was closed after 8.2.0 milestone, but idk where can I find it).
The BinaryFormatter code is not included in .NET 9.0 SDK anymore, and MSFT talked about it for very long time.
The only workaround is to set:

<EnableUnsafeBinaryFormatterSerialization>true</EnableUnsafeBinaryFormatterSerialization>

and add package

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

to the nuke project.

Off-topic with some criticism: this issue was created about 3 years ago, and it was repeatedly warned that not only should it be migrated away from BinaryFormatter, but it was also mentioned that starting from .NET 9.0, it would be removed from the SDK. There was plenty of time to either migrate or at least include the compatibility packages in the build.csproj.
The problem is that Nuke is not just a third-party library, it’s a build system that users expect to "just run" and if you are a build system then you should be prepared for the next .NET release. This makes me somewhat concerned about the current state of Nuke. I understand this is open source, and if the author doesn't have time to maintain it, critical issues like this should be pinned and marked as “help wanted.” I didn’t even know Nuke was using BinaryFormatter until now.

@matkoch
Copy link
Member

matkoch commented Nov 16, 2024

@ScarletKuro you're concerned about an issue that

  1. has a workaround (I see you quickly found it)
  2. affects a runtime that is not even a week old, and
  3. is marked as completed already?

I'm still following an issue about failing SWA deployments under Microsoft umbrella. It's 2,5 years old and has about 100 "me too" comments without any Microsoft response. The only workaround is recreating the resource. How concerned should I be and where can I escalate this?! :)

users expect to "just run"

FYI, there are plenty of folks more closely connected to me/the project, and I have yet to encounter any serious complaints.

Thanks for your understanding.

PS: I pinned the issue as per your request.

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

No branches or pull requests

16 participants