search

dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.6k stars 10.06k forks source link

WebEventData.ParseEventArgsJson has invalid UnconditionalSuppressMessage for JsonSerializer.Deserialize #45851

Open eerhardt opened 1 year ago

eerhardt commented 1 year ago

See the suppression here:

https://github.com/dotnet/aspnetcore/blob/1d04387cf3b68636f8ade0118334f87ae5f56f7d/src/Components/Web/src/WebEventData/WebEventData.cs#L61-L79

This suppression is not valid because there is no guarantee the eventArgsType hasn't been trimmed, and thus the Deserialization may fail, or behave differently, in a trimmed app.

We should remove this suppression and address the warning correctly so this code behaves the same with and without trimming. Potentially, we can use the same approach that we are taking with https://github.com/dotnet/aspnetcore/issues/45527.

This is the root cause of https://github.com/microsoft/fast-blazor/issues/280. In that issue, the Microsoft.Fast.Components.FluentUI.dll assembly is marked as IsTrimmable=true. The assembly contains some event arg classes, like MenuChangeEventArgs and DialogEventArgs. These classes are being trimmed, since their containing assembly is marked as IsTrimmable=true. However, when an app tries to use these events, it fails as described in the issue. This is because the constructors are being trimmed from the classes, causing the JsonSerializer.Deserialize call above to fail.

cc @brunolins16 @javiercn @JamesNK

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost commented 11 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.