Obsolete BinaryFormatter.Serialize and BinaryFormatter.Deserialize #39324
Conversation
* Initial API obsoletions * Add comments to create tracking issues * Update error messsages & proj settings * Fix nowarns * Fix failing unit test * Put issue links in source * Obsolete both serialize and deserialize * React to Serialize being obsolete * Update obsoletion text
GrabYourPitchforks
added
area-Serialization
breaking-change
...ntModel.TypeConverter/src/System/ComponentModel/Design/DesigntimeLicenseContextSerializer.cs
Outdated
Show resolved
Hide resolved
.../System.Configuration.ConfigurationManager/src/System/Configuration/SettingsPropertyValue.cs
Show resolved
Hide resolved
| @@ -59,6 +60,7 @@ protected virtual long Schedule(object? obj) | |||
| return id; | |||
| } | |||
|
|
|||
| [Obsolete(Obsoletions.InsecureSerializationMessage, DiagnosticId = Obsoletions.InsecureSerializationDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] | |||
There was a problem hiding this comment.
Interesting, it's not just BinaryFormatter but all Formatters. I hadn't comprehended that. Not that it matters much probably, since it's rare to see others these days. And it makes sense they'd all fundamentally have the same issues.
There was a problem hiding this comment.
I hadn't realized it encompassed IFormatter, Formatter, and their derived types either. Have we researched the extent to which either of these two are implemented outside of our own code - eg a search through NuGet?
There was a problem hiding this comment.
Right. Formatter is already essentially a dead class anyway. It was never really used, not even on Full Framework. IFormatter doesn't have much use any more since NetDataContractSerializer was never ported over, and there's no remoting infrastructure. So they're both vestigial types at this point.
There was a problem hiding this comment.
Right, but in theory there could be libraries out there that use either of them (most likely the interface).
@marklio is it possible to construct a query over NuGet to see whether there are libraries out there that implement IFormatter for some reason? It is not in itself dangerous, so in theory we could leave it around, I guess. I am guessing nobody actually uses it.
I think the usage I see in apisof.net is folks accepting/returning it, but that would/could be largely in the service of BinaryFormatter.
There was a problem hiding this comment.
Not pushing back on obsoleting it otherwise
There was a problem hiding this comment.
@danmosemsft We don't have the data indexed in a way to ask this question (implemented vs. referenced) with a quick query. If we thought this was important data to make a decision on, I can definitely get this data in the 2-3 days timeframe (or faster if we really think it is important). In any case, I'll add this sort of index to the Chem backlog for the future.
There was a problem hiding this comment.
@marklio it's not urgent. I don't think it blocks this PR even. probably worth doing, though.
There was a problem hiding this comment.
I think the usage I see in apisof.net is folks accepting/returning it, but that would/could be largely in the service of BinaryFormatter.
This matches what I saw as well. IFormatter isn't a terribly convenient interface to implement. The consumers of the API that I came across were using it to dynamically switch between BinaryFormatter, NetDataContractSerializer, and ObjectStateFormatter, depending on their scenario. But since those last two don't exist in Core, it means the interface is effectively now synonymous with the one remaining implementation.
|
@stephentoub You had recommended removing the comments with links back to GitHub? The issue links in each comment are tailored for the specific project. Is this too much noise / not enough value to keep these links in the source? |
No, they're valuable, but the two places I commented on already have the same comment with the same link a line or two below. |
Co-authored-by: Stephen Toub <stoub@microsoft.com>
|
CI isn't enqueueing certain test runs for some reason. Appears to be affecting nearly every PR at the moment. Given that we didn't make runtime changes, tests compiled successfully, and the tests that did run showed no problems, I'm going ahead with the merge. |
|
Please note that since we changed the diagnostic id prefixes we're going to use, #39269 is going to change the ID of BinaryFormatter's obsoletions to |
|
fyi, https://aka.ms/dotnet-warnings/MSLIB0003 (or SYSLIB0011) and https://aka.ms/binaryformatter are redirecting to microsoft.com. |
|
Yup, thanks for the reminder. We don't have the docs up yet because these bits haven't yet shipped. |
ghost
locked as resolved and limited conversation to collaborators
jkotas
added
the
binaryformatter-migration
Contributes to dotnet/designs#141.
Once this goes through we'll need to update WinForms, WPF, etc. The easiest thing for them to do now would be to suppress the new warning to allow the merge to commence, then
pragma warning suppressthe individual call sites and open individual tracking issues. #39287 can be used to track them.