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

Developers using JsonSerializer have support for polymorphic types #45189

Closed
3 tasks done
terrajobst opened this issue Nov 25, 2020 · 11 comments
Closed
3 tasks done

Developers using JsonSerializer have support for polymorphic types #45189

terrajobst opened this issue Nov 25, 2020 · 11 comments
Assignees
Labels
area-System.Text.Json Cost:M Work that requires one engineer up to 2 weeks Priority:1 Work that is critical for the release, but we could probably ship without tracking This issue is tracking the completion of other related issues. User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@terrajobst
Copy link
Member

terrajobst commented Nov 25, 2020

Work

@terrajobst terrajobst added area-System.Text.Json Cost:M Work that requires one engineer up to 2 weeks labels Nov 25, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Nov 25, 2020
@terrajobst terrajobst added User Story A single user-facing feature. Can be grouped under an epic. and removed untriaged New issue has not been triaged by the area owner labels Nov 25, 2020
@marek-safar marek-safar added tracking This issue is tracking the completion of other related issues. and removed User Story A single user-facing feature. Can be grouped under an epic. labels Nov 27, 2020
@StingyJack
Copy link

System.Text.Json is not going to be usable until it can handle serialization of objects. Even then, if including all of the properties is not made the default behavior I'm still going to have to avoid using it.

Why would an entire product team think that any data loss would be acceptable? Especially by default.

@layomia
Copy link
Contributor

layomia commented Dec 9, 2020

@StingyJack

Extending polymorphic support has always been on the roadmap for System.Text.Json. We do not consider this feature an edge case. We just couldn't get to it in previous releases due to product scoping and time constraints. As this issue indicates, it is being planned for .NET 6.0.

Polymorphic (de)serialization cannot be made the default behavior because that would be a breaking change from previous versions of System.Text.Json. Polymorphic serialization can also lead to accidental information disclosure when a object is treated as a different type than declared to the serializer. Such behavior needs to be opt-in only.

Not having built-in support for polymorphic is not a ship-blocker as many applications do not need this feature. Following up on #45764: System.Text.Json was not designed to be a drop-in replacement for Newtonsoft.Json. You are free to continue using it if it satisfies your use cases. Please see our migration guide for some clarity about the relationship between the two libraries:

System.Text.Json focuses primarily on performance, security, and standards compliance. It has some key differences in default behavior and doesn't aim to have feature parity with Newtonsoft.Json. For some scenarios, System.Text.Json has no built-in functionality, but there are recommended workarounds. For other scenarios, workarounds are impractical. If your application depends on a missing feature, consider filing an issue to find out if support for your scenario can be added.

@StingyJack
Copy link

@layomia - A Breaking Change means just incrementing the Major Version number I thought. You can say that this is not intended to be a drop in replacement for newtonsoft's package, but being able to serialize an object completely is one of those things that is expected to work in a package like this. What good is the performance going to be to me if its not able to do the primary job? Is getting it to do the complete job going to net worse performance than the existing package?

If you asked 100 random .net developers (that are not already familiar with this shortfall of S.T.J) what Polymorphic Serialization is, you would get maybe one that could correctly answer what it meant, but I would bet $1 that this affects more than 75% of them. The doc page should say "Can serialize base class members? Not supported"

@ericstj ericstj added the Priority:1 Work that is critical for the release, but we could probably ship without label Jan 20, 2021
@layomia layomia changed the title Extend polymorphic support User Story: Developers using JsonSerializer want full polymorphic support Jan 21, 2021
@layomia layomia added the User Story A single user-facing feature. Can be grouped under an epic. label Jan 21, 2021
@danmoseley danmoseley changed the title User Story: Developers using JsonSerializer want full polymorphic support Developers using JsonSerializer have support for polymorphic types Jan 21, 2021
@dmikov
Copy link

dmikov commented Mar 3, 2021

Please, please keep deserializing the shape of the declared type by default. This unintended information disclosure was the thing I hated about Json.Net the most. It is imperative that if your array of type User, it doesn't show President instead. It has to be opt-in. Thank you for making it for us.

@StingyJack
Copy link

Anyone concerned about information disclosure should be using DTOs for the exposed API that don't have the unwanted properties on them.

Asking to have a tool crippled for everyone else because some people are unwilling to model their API data is a request that should not be taken seriously.

@qsdfplkj
Copy link
Contributor

qsdfplkj commented May 30, 2021

Polymorphic serialization can also lead to accidental information disclosure when a object is treated as a different type than declared to the serializer. Such behavior needs to be opt-in only.

this is only valid when the declared property type is object. If you deserialize to an interface or base type you know exactly what to expect. include type information only when the runtime type isn’t the same as the declared type and you will be safe.

@StingyJack
Copy link

naar type

@qsdfplkj Is this a typo or an actual thing?

@qsdfplkj
Copy link
Contributor

Isn’t it just that JavaScript is not really polymorphic but a dynamic language.

@eiriktsarpalis
Copy link
Member

Closing this, since it is specifically tracking polymorphism-related work for .NET 6. Individual pending work items have been reassigned to different milestones.

@StingyJack
Copy link

@eiriktsarpalis - are those other work items something that non-msft users can see?

@eiriktsarpalis
Copy link
Member

All planning and issue tracking happens publicly on github: I was referring to the open issues that are linked in the original post.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json Cost:M Work that requires one engineer up to 2 weeks Priority:1 Work that is critical for the release, but we could probably ship without tracking This issue is tracking the completion of other related issues. User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

No branches or pull requests

9 participants