This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
Prevent ImplicitTransform of index getter method (#41418) #41647
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Cherry-pick of #41418 to release/3.1
Fix for the exception thrown in dotnet/coreclr#26890 that is a fallout of the issue being fixed in
DiagnosticSourceEventSource
in conjunction with dotnet/aspnetcore#11730.small repro (click to expand)
Attach
dotnet trace
to this app and turn on theMicrosoft-Diagnostics-DiagnosticSource
provider.Root Cause of Exception:
The getter for the implicit
Item
property for the index operator takes an argument, andDiagnosticSourceEventSource
makes the assumption that property getters don't take arguments. It tries to bind the resulting delegate of type[retval] get_Item(int32 index)
to the typeFunc<TObject, TProperty>
so the binding fails and we get the exception you see in dotnet/coreclr#26890.This change prevents
DiagnosticSourceEventSource
from attempting to serialize the implicitItem
property by blocking index getters.Customer Impact
ASP.NET Core has used
DiagnosticSourceEventSource
for their eventing. Since they changed their event types to inherit fromIReadOnlyCollection
in 3.0, they will all hit the exception in dotnet/coreclr#26890 when turned on causing a bad diagnostic experience if you do not explicitly specify a transform.Regression
I don't believe this is a regression as the code existed in the 2.0 time frame as well, but it was rarely encountered. The difference now being that after dotnet/aspnetcore#11730, the behavior is being hit routinely by ASP.NET Core when diagnosing an MVC app.
Risk
Minimal. This code prevents an exception from being thrown and prevents bad behavior without changing expected behavior.
Tests
A test were added to validate that this change prevents the implicit index getter from being serialized.
CC - @tommcdon @noahfalk