Skip to content

Commit

Permalink
Fix culture-specific string comparisons in EventSource (#83319)
Browse files Browse the repository at this point in the history 
* 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 dotnet/aspnetcore#47029 (comment)

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

Co-authored-by: Stephen Toub <stoub@microsoft.com>
  • Loading branch information
@jkotas @stephentoub
jkotas and stephentoub authored Mar 13, 2023
1 parent 8599161 commit 541347c
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 16 deletions.
8 changes: 4 additions & 4 deletions src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/ActivityTracker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,13 +254,13 @@ private static string NormalizeActivityName(string providerName, string activity
{
// 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)
{
Expand Down
25 changes: 13 additions & 12 deletions src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2332,11 +2332,11 @@ internal static EventOpcode GetOpcodeWithDefault(EventOpcode opcode, string? eve
{
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;
}
Expand Down Expand Up @@ -3199,10 +3199,10 @@ private static bool AttributeTypeNamesMatch(Type attributeType, Type reflectedAt
{
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);
Expand All @@ -3219,10 +3219,11 @@ private static bool AttributeTypeNamesMatch(Type attributeType, Type reflectedAt

// 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;
Expand Down Expand Up @@ -3823,8 +3824,8 @@ private bool SelfDescribingEvents
// 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
Expand Down Expand Up @@ -5636,7 +5637,7 @@ static FieldInfo[] GetEnumFields(Type localEnumType)
}
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>");
}
Expand All @@ -5651,7 +5652,7 @@ static FieldInfo[] GetEnumFields(Type localEnumType)

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("\">");
Expand Down

0 comments on commit 541347c

Please sign in to comment.