Is Microsoft Planning to fix ImageList?

[SoftCircuits]

There is a lot of documentation about issue dotnet/winforms#9701 and the security risks associated with BinaryFormatter.

But what if we're not using BinaryFormatter directly? Instead, we're using ImageList, which is currently implemented to use BinaryFormatter? Do we need to find a different image list control? Or is Microsoft planning to fix the existing one?

[elachlan]

@SoftCircuits are you currently having a problem with imagelist? If so can you provide your visual studio and .NET version?

[SoftCircuits]

@elachlan Sure.

Visual Studio: 17.12.2
.NET: 9.0

Resource "imageList1.ImageStream" of type "System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" may be deserialized via BinaryFormatter at runtime. BinaryFormatter is deprecated due to known security risks and is removed from .NET 9+. If you wish to continue using it, set property "GenerateResourceWarnOnBinaryFormatterUse" to false.

[elachlan]

@SoftCircuits

Note that an extra step of setting System.Resources.Extensions.UseBinaryFormatter app context switch to true is required to use BinaryFormatter for resources.

See Documentation: https://learn.microsoft.com/en-us/dotnet/standard/serialization/binaryformatter-migration-guide/winforms-applications

[SoftCircuits]

@elachlan I'm still trying to figure out why I need BinaryFormatter. Or, more accurately, why ImageList needs it.

[elachlan]

Images get serialized into the resource files (resx). Winforms handles this internally but you require to enable BinaryFormatter specifically for resources. Enabling the flag doesn't automatically pull in the full BinaryFormatter, that is done via the compatibility package (https://learn.microsoft.com/en-us/dotnet/standard/serialization/binaryformatter-migration-guide/compatibility-package). I don't believe you will need the compatibility package for ImageList to work.

The warning you are getting can be disabled. It says it "MAY" be deserialized by BinaryFormatter, but its a broad catch all message. If it is unable to use the internal winforms deserializer, then it will attempt to use BinaryFormatter. If you haven't pulled in the compat package then it will throw an exception.

[SoftCircuits]

@elachlan I know I can suppress the warning. But why is Microsft not redesigning ImageList such that I don't need to suppress warnings? If there's a problem with it, why doesn't Microsoft just fix it? Why is requiring us to hide warnings okay? I don't understand this.

[elachlan]

I'll leave that to the Winforms team to answer.

[Tanya-Solyanik]

@SoftCircuits - thank you for reporting this issue. MSB3825 does not apply to the image list control in .NET 9 because we had redesigned how we serialize and read image list at the runtime - https://github.com/dotnet/winforms/blob/01cfa36f4c9b6a3f4997129b7b72e2a2c874dd38/src/System.Windows.Forms/src/System/Windows/Forms/BinaryFormat/WinFormsBinaryFormatWriter.cs#L39-L50.
https://github.com/dotnet/winforms/blob/01cfa36f4c9b6a3f4997129b7b72e2a2c874dd38/src/System.Windows.Forms/src/System/Windows/Forms/Nrbf/WinFormsSerializationRecordExtensions.cs#L19-L38
It is not accurate and can be suppressed manually until we fix the error reporting. In fact we deserialize resx resources at the runtime using a safer API, not the BinaryFormatter, unless you opt-in in AppContext switch System.Resources.Extensions.UseBinaryFormatter. The purpose of the warning was to alert you about any unexpected types that might be present in the resx file, but unfortunately the build task does not have enough information to determine that.
We hadn't redesigned how ImageListStream is serialized to the resx resources because that could make resx files incompatible between the .NET versions. However, we handle design time serialization behind the scenes, you should not be required to add dependency on BinaryFormatter to support resources serialization.

[Tanya-Solyanik]

@rainersigwald - is it possible to disable MSB3825 when target framework is NET9+? Or at least change the wording? As is it looks like the app should use BinaryFormatter, while only a small percentage needs the opt-in for the Resource Manager to use BInaryFormatter - https://source.dot.net/#System.Resources.Extensions/System/Resources/Extensions/DeserializingResourceReader.cs,36749eec8744c444,references
related to #8453

[Tanya-Solyanik]

@rainersigwald @Forgind - here are the BF removal workgroup recommendations:

1. remove the MSB3825 warning when targeting .NET9+
2. reword this warning when targeting NET8:
   2.1 may be used -> will be used
   2.2 remove of type {1} - this type is not relevant to the author of the resx file, this is not the type that user serialized to the resx (in the above example, ImageListStreamer, the object inside the binary formatted payload, is not a string)

Resource "imageList1.ImageStream" of type "System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" may be deserialized via BinaryFormatter at runtime. BinaryFormatter is deprecated due to known security risks and is removed from .NET 9+. If you wish to continue using it, set property "GenerateResourceWarnOnBinaryFormatterUse" to false.
More information: https://aka.ms/binaryformatter-migration-guide
-->
Resource "imageList1.ImageStream" will be deserialized via BinaryFormatter at runtime. BinaryFormatter is deprecated due to known security risks and is removed from .NET 9+. If you wish to continue using it, set property "GenerateResourceWarnOnBinaryFormatterUse" to false.
More information: https://aka.ms/binaryformatter-migration-guide

[JanKrivanek]

@Tanya-Solyanik Just to clarify - when targeting lower than NET8 (or full FW) - then we do NOT want to warn - correct?

Not to self - this would be the place to tune the warning for NETx versions: https://github.com/dotnet/sdk/blob/cc260aa9a3fd51e20d66f0186a3da9fcf864b208/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets#L106-L108

[Tanya-Solyanik]

@JanKrivanek

when targeting lower than NET8 (or full FW) - then we do NOT want to warn - correct?

Good point! Is this warning conditional on the target FX right now? I believe so per this bug description - #8453, There is a property GenerateResourceWarnOnBinaryFormatterUse that we set here - https://github.com/search?q=repo%3Adotnet%2Fsdk+GenerateResourceWarnOnBinaryFormatterUse+&type=code

When I moved this bug to msbuild I assumed that this is only about net8+, but now that you posed this question, the warning is applicable to all versions in a sense that BF is bad. I'm not sure why it was implemented conditionally in the first place, probably we didn't want it to be too noisy?

[JanKrivanek]

Yes - I believe it was to limit the update blockers - where just updating the toolchain would suddenly cause builds to fail. In such case we probably do not want to case disruptions to builds targeting NetFx (which we currently do not).

I'll then make sure to adjust wording for NET8 and remove the warning for NET9+

Thank you!