Undocumented breaking change in .NET 5.0 - HashSet moved to another assembly/library

[maciejjarzynski]

Hello everyone,
I've stumbled upon a breaking change which as far as I've searched for was not documented anywhere.
It seems that with that pull request #37180 HashSet was moved from System.Collections to System.Private.CoreLib library.
Let's say we have a fair big distributed system based on microservices with an immutable events used for a synchronization between them which are being stored forever.
Newtonsoft.Json is being used as a serializer/deserializer.
The problem with that breaking change is like in the example below:
that's an example of class which was serialized in a netcoreapp3.1 version (with a collection in a dictionary's value being HashSet):

public class TestClass
    {
        public IReadOnlyDictionary<string, IReadOnlyCollection<string>> Dictionary { get; }

        public TestClass(IReadOnlyDictionary<string, IReadOnlyCollection<string>> dictionary)
        {
            Dictionary = dictionary;
        }
    }

resulting in (with a collection in a dictionary's value being HashSet)

{
  "Dictionary": {
    "DictionaryKey": {
      "$type": "System.Collections.Generic.HashSet`1[[System.String, System.Private.CoreLib]], System.Collections",
      "$values": [
        "string value"
      ]
    }
  }
}

whereas in net5.0 it is being serialized as

{
  "Dictionary": {
    "DictionaryKey": {
      "$type": "System.Collections.Generic.HashSet`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib",
      "$values": [
        "string value"
      ]
    }
  }
}

The TypeNameHandling is set to Auto.

The problem of couse occurs when trying to deserialize old-netcoreapp3.1 events/messages into the application running on net5.0

My questions are basically:

• are there any other such an undocumented breaking changes, like any other types were moved between libraries?
• are you aware that a mentioned change could have caused some serious implications and have some kind of solution or workaround for that?
• @JamesNK maybe you'd have some idea, how could we disable the type name handling for the dictionaries and collections from a framework, so upon deserialization the $type would be ignored for those types?

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Hello everyone,
I've stumbled upon a breaking change which as far as I've searched for was not documented anywhere.
It seems that with that pull request #37180 HashSet was moved from System.Collections to System.Private.CoreLib library.
Let's say we have a fair big distributed system based on microservices with an immutable events used for a synchronization between them which are being stored forever.
The problem with that breaking change is like in the example below:
that's an example of class which was serialized in a netcoreapp3.1 version (with a collection in a dictionary's value being HashSet):

public class TestClass
    {
        public IReadOnlyDictionary<string, IReadOnlyCollection<string>> Dictionary { get; }

        public TestClass(IReadOnlyDictionary<string, IReadOnlyCollection<string>> dictionary)
        {
            Dictionary = dictionary;
        }
    }

resulting in (with a collection in a dictionary's value being HashSet)

{
  "Dictionary": {
    "DictionaryKey": {
      "$type": "System.Collections.Generic.HashSet`1[[System.String, System.Private.CoreLib]], System.Collections",
      "$values": [
        "string value"
      ]
    }
  }
}

whereas in net5.0 it is being serialized as

{
  "Dictionary": {
    "DictionaryKey": {
      "$type": "System.Collections.Generic.HashSet`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib",
      "$values": [
        "string value"
      ]
    }
  }
}

The TypeNameHandling is set to Auto.

The problem of couse occurs when trying to deserialize old-netcoreapp3.1 events/messages into the application running on net5.0

My questions are basically:

• are there any other such an undocumented breaking changes, like any other types were moved between libraries?
• are you aware that a mentioned change could have caused some serious implications and have some kind of solution or workaround for that?
• @JamesNK maybe you'd have some idea, how could we disable the type name handling for the dictionaries and collections from a framework, so upon deserialization the $type would be ignored for those types?

Author:	maciejjarzynski
Assignees:	-
Labels:	area-System.Collections, untriaged
Milestone:	-

[huoyaoyuan]

Moving types among assemblies isn't treated as breaking change for common cases. There's TypeForwardedTo added.

• are there any other such an undocumented breaking changes, like any other types were moved between libraries?

There are, but not frequently. Types will be moved when the implementation structure requires so.

The problem of couse occurs when trying to deserialize old-netcoreapp3.1 events/messages into the application running on net5.0

Such serialization can be more incompatible with .NET Framework, since many types have moved and the corelib name has changed.

[maciejjarzynski]

Moving types among assemblies isn't treated as breaking change for common cases. There's TypeForwardedTo added.

If it isn't, I think it should be treated as such.
As per SemVer specification:

Major version X (X.y.z | X > 0) MUST be incremented if any backwards incompatible changes are introduced to the public API.

And of course assembly name for the given type is the public API.

And my question (let me quote myself) was regarding any other such a breaking changes for .NET 5.0 release:

are there any other such an undocumented breaking changes, like any other types were moved between libraries?

[huoyaoyuan]

If it isn't, I think it should be treated as such.

In public API, it stays remaining in the original assmebly. Serialization using reflection is getting implementation detail.

For example, there is no System.Private.CoreLib in public API. There is only System.Runtime.

Implementation detail can be broken in any release.

[MichalStrehovsky]

The type has moved many times already. Serializers should ideally respect the TypeForwarded**From** attribute on the type and emit that as the owning assembly:

runtime/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs

Line 16 in 0df028b

[TypeForwardedFrom("System.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]

Libraries have type forwarders (using the above mentioned TypeForwardedTo) to ensure apps looking for HashSet in System.Core will find it. This way the serialized data would be compatible all the way to .NET Framework 2.0.

[maciejjarzynski]

Then a question to @JamesNK : does the Newtonsoft.Json respect the TypeForwardedFrom attribute, and if not, why?

[GrabYourPitchforks]

That question should be asked over at the Newtonsoft.Json repo (https://github.com/JamesNK/Newtonsoft.Json). This repo is specifically for .NET.

(Also, standard disclaimer: TypeNameHandling.Auto should be used with extreme caution. It's a code execution equivalent. Your service's threat model should consider whether the entity supplying the JSON contents to deserialize is trustworthy.)

[JamesNK]

Issue about this here - JamesNK/Newtonsoft.Json#1972

This was suggested as a solution - JamesNK/Newtonsoft.Json#1972 (comment)

[danmoseley]

Resolving as by design - we move types with Type Forwarding often. The attribute was specifically created so this could be done transparently (if it is respected)

[SimonCropp]

@danmoseley can you comment on this issue JamesNK/Newtonsoft.Json#1972 ? ie should the fix be included in jsondotnet or in the runtime?