Prevent ImplicitTransform of index getter method (dotnet/corefx#41418)
authorJohn Salem <josalem@microsoft.com>
Tue, 8 Oct 2019 17:58:42 +0000 (10:58 -0700)
committerGitHub <noreply@github.com>
Tue, 8 Oct 2019 17:58:42 +0000 (10:58 -0700)
* Prevent TransformSpec from attempting to implicitly create a delegate for index properties

* Add tests

Commit migrated from https://github.com/dotnet/corefx/commit/2b8d0577ee97ff312ef074ab36f264ebedcccfbf

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSourceEventSource.cs
src/libraries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs

index b27f9c0..98829e5 100644 (file)
@@ -709,6 +709,9 @@ namespace System.Diagnostics
                 TypeInfo curTypeInfo = type.GetTypeInfo();
                 foreach (PropertyInfo property in curTypeInfo.DeclaredProperties)
                 {
+                    // prevent TransformSpec from attempting to implicitly transform index properties
+                    if (property.GetMethod == null || property.GetMethod!.GetParameters().Length > 0)
+                        continue;
                     newSerializableArgs = new TransformSpec(property.Name, 0, property.Name.Length, newSerializableArgs);
                 }
                 return Reverse(newSerializableArgs);
index abd31a0..847a790 100644 (file)
@@ -734,6 +734,43 @@ namespace System.Diagnostics.Tests
                 }
             }).Dispose();
         }
+
+        [Fact]
+        public void IndexGetters_DontThrow()
+        {
+            RemoteExecutor.Invoke(() =>
+            {
+                using (var eventListener = new TestDiagnosticSourceEventListener())
+                using (var diagnosticListener = new DiagnosticListener("MySource"))
+                {
+                    eventListener.Enable(
+                        "MySource/MyEvent"
+                    );
+                    // The type MyEvent only declares 3 Properties, but actually
+                    // has 4 due to the implicit Item property from having the index
+                    // operator implemented. The Getter for this Item property
+                    // is unusual for Property getters because it takes
+                    // an int32 as an input. This test ensures that this
+                    // implicit Property isn't implicitly serialized by
+                    // DiagnosticSourceEventSource.
+                    diagnosticListener.Write(
+                        "MyEvent",
+                        new MyEvent
+                        {
+                            Number = 1,
+                            OtherNumber = 2
+                        }
+                    );
+                    Assert.Equal(1, eventListener.EventCount);
+                    Assert.Equal("MySource", eventListener.LastEvent.SourceName);
+                    Assert.Equal("MyEvent", eventListener.LastEvent.EventName);
+                    Assert.True(eventListener.LastEvent.Arguments.Count <= 3);
+                    Assert.Equal("1", eventListener.LastEvent.Arguments["Number"]);
+                    Assert.Equal("2", eventListener.LastEvent.Arguments["OtherNumber"]);
+                    Assert.Equal("2", eventListener.LastEvent.Arguments["Count"]);
+                }
+            }).Dispose();
+        }
     }
 
     /****************************************************************************/
@@ -757,6 +794,22 @@ namespace System.Diagnostics.Tests
         public int Y { get; set; }
     }
 
+    /// <summary>
+    /// classes for test data
+    /// </summary>
+    internal class MyEvent
+    {
+        public int Number { get; set; }
+        public int OtherNumber { get; set; }
+        public int Count => 2;
+        public KeyValuePair<string, object> this[int index] => index switch
+        {
+            0 => new KeyValuePair<string, object>(nameof(Number), Number),
+            1 => new KeyValuePair<string, object>(nameof(OtherNumber), OtherNumber),
+            _ => throw new IndexOutOfRangeException()
+        };
+    }
+
     /****************************************************************************/
     // Harness infrastructure
     /// <summary>