Fix culture-specific string comparisons in EventSource (#83319)
authorJan Kotas <jkotas@microsoft.com>
Mon, 13 Mar 2023 03:41:35 +0000 (20:41 -0700)
committerGitHub <noreply@github.com>
Mon, 13 Mar 2023 03:41:35 +0000 (20:41 -0700)
* Fix culture-specific string comparisons in EventSource

These string comparisons are clearly not meant to be culture-specific. They were one of the reasons for loading ICU in Console.WriteLine("Hello world"); app.

Context https://github.com/dotnet/aspnetcore/issues/47029#issuecomment-1455553729

* Update src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/ActivityTracker.cs
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs

index 52212d2..fdca884 100644 (file)
@@ -254,13 +254,13 @@ namespace System.Diagnostics.Tracing
         {
             // We use provider name to distinguish between activities from different providers.
 
-            if (activityName.EndsWith(EventSource.s_ActivityStartSuffix, StringComparison.Ordinal))
+            if (activityName.EndsWith(EventSource.ActivityStartSuffix, StringComparison.Ordinal))
             {
-                return string.Concat(providerName, activityName.AsSpan(0, activityName.Length - EventSource.s_ActivityStartSuffix.Length));
+                return string.Concat(providerName, activityName.AsSpan()[..^EventSource.ActivityStartSuffix.Length]);
             }
-            else if (activityName.EndsWith(EventSource.s_ActivityStopSuffix, StringComparison.Ordinal))
+            else if (activityName.EndsWith(EventSource.ActivityStopSuffix, StringComparison.Ordinal))
             {
-                return string.Concat(providerName, activityName.AsSpan(0, activityName.Length - EventSource.s_ActivityStopSuffix.Length));
+                return string.Concat(providerName, activityName.AsSpan()[..^EventSource.ActivityStopSuffix.Length]);
             }
             else if (task != 0)
             {
index 947d344..cdadc67 100644 (file)
@@ -2332,11 +2332,11 @@ namespace System.Diagnostics.Tracing
         {
             if (opcode == EventOpcode.Info && eventName != null)
             {
-                if (eventName.EndsWith(s_ActivityStartSuffix, StringComparison.Ordinal))
+                if (eventName.EndsWith(ActivityStartSuffix, StringComparison.Ordinal))
                 {
                     return EventOpcode.Start;
                 }
-                else if (eventName.EndsWith(s_ActivityStopSuffix, StringComparison.Ordinal))
+                else if (eventName.EndsWith(ActivityStopSuffix, StringComparison.Ordinal))
                 {
                     return EventOpcode.Stop;
                 }
@@ -3199,10 +3199,10 @@ namespace System.Diagnostics.Tracing
                             {
                                 if (eventAttribute.Opcode == EventOpcode.Start)
                                 {
-                                    string taskName = eventName.Substring(0, eventName.Length - s_ActivityStartSuffix.Length); // Remove the Stop suffix to get the task name
-                                    if (string.Compare(eventName, 0, taskName, 0, taskName.Length) == 0 &&
-                                        string.Compare(eventName, taskName.Length, s_ActivityStartSuffix, 0, Math.Max(eventName.Length - taskName.Length, s_ActivityStartSuffix.Length)) == 0)
+                                    if (eventName.EndsWith(ActivityStartSuffix, StringComparison.Ordinal))
                                     {
+                                        string taskName = eventName[..^ActivityStartSuffix.Length]; // Remove the Start suffix to get the task name
+
                                         // Add a task that is just the task name for the start event.   This suppress the auto-task generation
                                         // That would otherwise happen (and create 'TaskName'Start as task name rather than just 'TaskName'
                                         manifest.AddTask(taskName, (int)eventAttribute.Task);
@@ -3219,10 +3219,11 @@ namespace System.Diagnostics.Tracing
 
                                         // If you remove the Stop and add a Start does that name match the Start Event's Name?
                                         // Ideally we would throw an error
-                                        string taskName = eventName.Substring(0, eventName.Length - s_ActivityStopSuffix.Length); // Remove the Stop suffix to get the task name
                                         if (startEventMetadata.Descriptor.Opcode == (byte)EventOpcode.Start &&
-                                            string.Compare(startEventMetadata.Name, 0, taskName, 0, taskName.Length) == 0 &&
-                                            string.Compare(startEventMetadata.Name, taskName.Length, s_ActivityStartSuffix, 0, Math.Max(startEventMetadata.Name.Length - taskName.Length, s_ActivityStartSuffix.Length)) == 0)
+                                            startEventMetadata.Name.EndsWith(ActivityStartSuffix, StringComparison.Ordinal) &&
+                                            eventName.EndsWith(ActivityStopSuffix, StringComparison.Ordinal) &&
+                                            startEventMetadata.Name.AsSpan()[..^ActivityStartSuffix.Length].SequenceEqual(
+                                                eventName.AsSpan()[..^ActivityStopSuffix.Length]))
                                         {
                                             // Make the stop event match the start event
                                             eventAttribute.Task = (EventTask)startEventMetadata.Descriptor.Task;
@@ -3823,8 +3824,8 @@ namespace System.Diagnostics.Tracing
         // We use a single instance of ActivityTracker for all EventSources instances to allow correlation between multiple event providers.
         // We have m_activityTracker field simply because instance field is more efficient than static field fetch.
         private ActivityTracker m_activityTracker = null!;
-        internal const string s_ActivityStartSuffix = "Start";
-        internal const string s_ActivityStopSuffix = "Stop";
+        internal const string ActivityStartSuffix = "Start";
+        internal const string ActivityStopSuffix = "Stop";
 
         // This switch controls an opt-in, off-by-default mechanism for allowing multiple EventSources to have the same
         // name and by extension GUID. This is not considered a mainline scenario and is explicitly intended as a release
@@ -5636,7 +5637,7 @@ namespace System.Diagnostics.Tracing
             }
             else
             {
-                // Work around a cornercase ETW issue where a manifest with no templates causes
+                // Work around a corner-case ETW issue where a manifest with no templates causes
                 // ETW events to not get sent to their associated channel.
                 sb.AppendLine("    <template tid=\"_empty\"></template>");
             }
@@ -5651,7 +5652,7 @@ namespace System.Diagnostics.Tracing
 
             var sortedStrings = new string[stringTab.Keys.Count];
             stringTab.Keys.CopyTo(sortedStrings, 0);
-            Array.Sort<string>(sortedStrings, 0, sortedStrings.Length);
+            Array.Sort<string>(sortedStrings, StringComparer.Ordinal);
 
             CultureInfo ci = CultureInfo.CurrentUICulture;
             sb.Append(" <resources culture=\"").Append(ci.Name).AppendLine("\">");