Remove hardcoded limit in deserialization constructor arguments #75982
Conversation
eiriktsarpalis
requested review from
layomia,
krwq and
steveharter
and removed request for
layomia
|
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsSee #71984 (comment) for an explanation on why this limit is not needed. Fix #71984.
|
...System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitMemberAccessor.cs
Outdated
Show resolved
Hide resolved
| @@ -155,7 +156,7 @@ async Task RunTestAsync<T>() | |||
| DefaultBufferSize = 1 | |||
| }; | |||
|
|
|||
| await Assert.ThrowsAsync<NotSupportedException>(async () => await StreamingSerializer.DeserializeWrapper<T>(stream, options)); | |||
| value = await StreamingSerializer.DeserializeWrapper<T>(stream, options); | |||
There was a problem hiding this comment.
should you add at least some tiny sanitization for couple of properties of value that they look good? (same above)
There was a problem hiding this comment.
The problem is that the test method is generic on the deserialized type, so you can't really check anything other than validate that the JSON is nontrivial. I'm validating that roundtripping works as expected in the new test.
...System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitMemberAccessor.cs
Outdated
Show resolved
Hide resolved
...System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitMemberAccessor.cs
Outdated
Show resolved
Hide resolved
This reverts commit e78ea20c532860d21bd0540a97ebfd1cf02370e3.
This reverts commit 53f7a91.
eiriktsarpalis
force-pushed
the
remove-constructor-argument-limit
branch
from
6ba26ad to
85d39d4
Compare
|
What's the release cycle for this kind of change? I'm curious when I can leverage this change in my code. |
This should be released as part of .NET 8. |
|
Holy crap that's.... quite a ways away...... |
|
.NET 8 Preview 1 is going to be released early next year. If this is a blocking issue, you might consider referencing the .NET 8 prerelease NuGet package as soon as it becomes available. |
|
I appreciate your help! |
|
And if you're feeling adventurous, you might want to try out our daily builds: Given that this was just merged, it should take a day or two before the fix becomes available. |
|
Unfortunately, given the fact that I'm a new hire at my company and we're using .NET 5 sigh, I don't think they'd jump at the idea of using a nightly build in our production backend. Personally, I'd love to use the latest build possible, because that's where I learn the most. When I used to use .NET for personal projects, it was only as far as Unity (game engine) supported 😩 These days I'm using rust for personal projects. However, I really do appreciate the option to do this, and I thank you for your time to implement this fix. |
|
@eiriktsarpalis Thanks for getting this resolved. I will watch out for the Preview and the release next year. |
ghost
locked as resolved and limited conversation to collaborators
See #71984 (comment) for an explanation on why this limit is not needed.
Fix #71984.