-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[release/9.0-staging] [NRBF] Allow the users to decode System.Nullable<UserStruct> #118328
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
[release/9.0-staging] [NRBF] Allow the users to decode System.Nullable<UserStruct> #118328
Conversation
…WithMembersAndTypes in the payload
This reverts commit 3636ebe.
Tagging subscribers to 'binaryformatter-migration': @adamsitnik, @bartonjs, @jeffhandley, @JeremyKuhne |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has my support for backport to .NET 9. Customer-blocking issue.
/ba-g Build analysis showed only one known issue: wasm build failure in CI (dotnet/runtime#117017) |
/ba-g This is a known issue |
/ba-g This is a known issue! |
/ba-g Trying again! This is a known issue! |
Backport of #118276 to release/9.0-staging
/cc @adamsitnik
Customer Impact
Customers using
NrbfDecoder
to decode a very specific payload that was generated by theBinaryFormatter
will fail with an exception about invalid payload.More details: The payload can be generated by the
BinaryFormatter
for a type with a nullable user-defined struct where the first occurrence of such struct is anull.
BinaryFormatter
is then fetching the field type (rather than instance type) and since it'sSystem.Nullable<$UserType>
it's described as BinaryType.SystemClass rather thanBinaryType.Class
. Since NrbfDecoder is restrictive to be secure by default, the input is rejected as for a "system record" the payload contains "non-system record".One of our first party customers is moving away from BinaryFormatter and they are currently blocked as they have hit this limitation during their migration process. The issue was reported internally.
Regression
Testing
A standalone and simple repro was identified and turned into a unit test. The test was failing without the fix, now it's passing.
Risk
Low. The
NrbfDecoder
is now going to decode such payloads, but it's going to keep the record type as it was (SerializationRecordType.ClassWithMembersAndTypes
, notSerializationRecordType.SystemClassWithMembersAndTypes
).