From afb98b06b3c6b8be7f14ee1d08b5c7af1f18c207 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Jan 2026 18:40:15 +0000 Subject: [PATCH 1/6] Initial plan From 77809526e5709f5558446b9a7762c7762dd6fb4c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Jan 2026 18:48:30 +0000 Subject: [PATCH 2/6] Fix XML escaping in GetManifestForRegisteredProvider - Add using directive for Microsoft.Diagnostics.Utilities namespace - Escape all user-provided strings when writing to XML attribute values and text content - Properly handle string ID references: use raw IDs in message references but escape when writing id attributes - Ensures consistency between message="$(string.ID)" references and entries Fixed locations: - enumName and valueName in valueMap/bitMap elements and string table - keyValue.Value in keyword elements and string table - task.Name in task elements and string table - keyValue.Value in opcode elements and string table Added comprehensive test for XML escaping using Microsoft-Windows-Ntfs provider that: - Validates manifest is well-formed XML by parsing with XmlDocument - Verifies all elements can be queried via XPath - Provides detailed error context if parsing fails --- src/TraceEvent/RegisteredTraceEventParser.cs | 33 ++++---- .../RegisteredTraceEventParserTests.cs | 75 +++++++++++++++++++ 2 files changed, 92 insertions(+), 16 deletions(-) diff --git a/src/TraceEvent/RegisteredTraceEventParser.cs b/src/TraceEvent/RegisteredTraceEventParser.cs index 7ed71299c..89556425a 100644 --- a/src/TraceEvent/RegisteredTraceEventParser.cs +++ b/src/TraceEvent/RegisteredTraceEventParser.cs @@ -2,6 +2,7 @@ using FastSerialization; using Microsoft.Diagnostics.Tracing.Compatibility; using Microsoft.Diagnostics.Tracing.Session; +using Microsoft.Diagnostics.Utilities; using System; using System.Collections.Generic; using System.Diagnostics; @@ -361,14 +362,14 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) { StringWriter enumWriter = new StringWriter(); string enumName = new string((char*)(&enumBuffer[enumInfo->NameOffset])); - enumAttrib = " map=\"" + enumName + "\""; + enumAttrib = " map=\"" + XmlUtilities.XmlEscape(enumName) + "\""; if (enumInfo->Flag == MAP_FLAGS.EVENTMAP_INFO_FLAG_MANIFEST_VALUEMAP) { - enumWriter.WriteLine(" ", enumName); + enumWriter.WriteLine(" ", XmlUtilities.XmlEscape(enumName)); } else { - enumWriter.WriteLine(" ", enumName); + enumWriter.WriteLine(" ", XmlUtilities.XmlEscape(enumName)); } EVENT_MAP_ENTRY* mapEntries = &enumInfo->MapEntryArray; @@ -376,20 +377,20 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) { int value = mapEntries[k].Value; string valueName = new string((char*)(&enumBuffer[mapEntries[k].NameOffset])).Trim(); - enumWriter.WriteLine(" ", value, enumName, valueName); string stringId = $"map_{enumName}{valueName}"; + enumWriter.WriteLine(" ", value, stringId); if (emittedStringIds.Add(stringId)) { - enumLocalizations.WriteLine(" ", stringId, valueName); + enumLocalizations.WriteLine(" ", XmlUtilities.XmlEscape(stringId), XmlUtilities.XmlEscape(valueName)); } } if (enumInfo->Flag == MAP_FLAGS.EVENTMAP_INFO_FLAG_MANIFEST_VALUEMAP) { - enumWriter.WriteLine(" ", enumName); + enumWriter.WriteLine(" "); } else { - enumWriter.WriteLine(" ", enumName); + enumWriter.WriteLine(" "); } enumIntern[mapName] = enumWriter.ToString(); @@ -458,12 +459,12 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) manifest.WriteLine(" "); foreach (var keyValue in keywords) { - manifest.WriteLine(" ", - keyValue.Value, keyValue.Value, keyValue.Key); string stringId = $"keyword_{keyValue.Value}"; + manifest.WriteLine(" ", + XmlUtilities.XmlEscape(keyValue.Value), stringId, keyValue.Key); if (emittedStringIds.Add(stringId)) { - localizedStrings.WriteLine(" ", stringId, keyValue.Value); + localizedStrings.WriteLine(" ", XmlUtilities.XmlEscape(stringId), XmlUtilities.XmlEscape(keyValue.Value)); } } manifest.WriteLine(" "); @@ -473,12 +474,12 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) foreach (var taskValue in tasks.Keys) { var task = tasks[taskValue]; - manifest.WriteLine(" ", task.Name, task.Name, taskValue, - task.Opcodes == null ? "/" : ""); // If no opcodes, terminate immediately. string taskStringId = $"task_{task.Name}"; + manifest.WriteLine(" ", XmlUtilities.XmlEscape(task.Name), taskStringId, taskValue, + task.Opcodes == null ? "/" : ""); // If no opcodes, terminate immediately. if (emittedStringIds.Add(taskStringId)) { - localizedStrings.WriteLine(" ", taskStringId, task.Name); + localizedStrings.WriteLine(" ", XmlUtilities.XmlEscape(taskStringId), XmlUtilities.XmlEscape(task.Name)); } if (task.Opcodes != null) { @@ -486,12 +487,12 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) manifest.WriteLine(" "); foreach (var keyValue in task.Opcodes) { - manifest.WriteLine(" ", - keyValue.Value, task.Name, keyValue.Value, keyValue.Key); string opcodeStringId = $"opcode_{task.Name}{keyValue.Value}"; + manifest.WriteLine(" ", + XmlUtilities.XmlEscape(keyValue.Value), opcodeStringId, keyValue.Key); if (emittedStringIds.Add(opcodeStringId)) { - localizedStrings.WriteLine(" ", opcodeStringId, keyValue.Value); + localizedStrings.WriteLine(" ", XmlUtilities.XmlEscape(opcodeStringId), XmlUtilities.XmlEscape(keyValue.Value)); } } manifest.WriteLine(" "); diff --git a/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs b/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs index f182b97df..ad1a5a92d 100644 --- a/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs +++ b/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs @@ -1,7 +1,9 @@ using Microsoft.Diagnostics.Tracing.Parsers; +using System; using System.Collections.Generic; using System.Linq; using System.Text.RegularExpressions; +using System.Xml; using Xunit; using Xunit.Abstractions; @@ -66,5 +68,78 @@ public void GetManifestForRegisteredProvider_NoDuplicateStringTableEntries() // Assert no duplicates exist Assert.Empty(duplicates); } + + /// + /// Test that GetManifestForRegisteredProvider properly escapes XML special characters + /// in attribute values and text content. This test uses the Microsoft-Windows-Ntfs provider + /// which is known to have values containing quotes and angle brackets. + /// + [WindowsFact] + public void GetManifestForRegisteredProvider_ProperlyEscapesXmlCharacters() + { + // Microsoft-Windows-Ntfs is a well-known provider with complex metadata + const string providerName = "Microsoft-Windows-Ntfs"; + + // Get the manifest for the provider + string manifest = RegisteredTraceEventParser.GetManifestForRegisteredProvider(providerName); + + Assert.NotNull(manifest); + Assert.NotEmpty(manifest); + + _output.WriteLine($"Generated manifest for {providerName} (length: {manifest.Length} chars)"); + + // Verify the manifest is well-formed XML by parsing it + var xmlDoc = new XmlDocument(); + try + { + xmlDoc.LoadXml(manifest); + _output.WriteLine("✓ Manifest is well-formed XML"); + } + catch (XmlException ex) + { + _output.WriteLine($"✗ Manifest XML parsing failed: {ex.Message}"); + _output.WriteLine($"Line {ex.LineNumber}, Position {ex.LinePosition}"); + + // Show context around the error + var lines = manifest.Split('\n'); + if (ex.LineNumber > 0 && ex.LineNumber <= lines.Length) + { + int start = Math.Max(0, ex.LineNumber - 3); + int end = Math.Min(lines.Length, ex.LineNumber + 2); + _output.WriteLine("\nContext:"); + for (int i = start; i < end; i++) + { + string marker = (i == ex.LineNumber - 1) ? ">>> " : " "; + _output.WriteLine($"{marker}{i + 1}: {lines[i]}"); + } + } + + throw; + } + + // Check that all attribute values with special characters are properly escaped + // by verifying we can successfully query the XML document + var nsmgr = new XmlNamespaceManager(xmlDoc.NameTable); + nsmgr.AddNamespace("e", "http://schemas.microsoft.com/win/2004/08/events"); + nsmgr.AddNamespace("win", "http://manifests.microsoft.com/win/2004/08/windows/events"); + + // Verify we can access various elements that might contain special characters + var keywords = xmlDoc.SelectNodes("//e:keyword", nsmgr); + var tasks = xmlDoc.SelectNodes("//e:task", nsmgr); + var opcodes = xmlDoc.SelectNodes("//e:opcode", nsmgr); + var valueMaps = xmlDoc.SelectNodes("//e:valueMap", nsmgr); + var bitMaps = xmlDoc.SelectNodes("//e:bitMap", nsmgr); + var stringElements = xmlDoc.SelectNodes("//e:string", nsmgr); + + _output.WriteLine($"Found {keywords?.Count ?? 0} keywords"); + _output.WriteLine($"Found {tasks?.Count ?? 0} tasks"); + _output.WriteLine($"Found {opcodes?.Count ?? 0} opcodes"); + _output.WriteLine($"Found {valueMaps?.Count ?? 0} valueMaps"); + _output.WriteLine($"Found {bitMaps?.Count ?? 0} bitMaps"); + _output.WriteLine($"Found {stringElements?.Count ?? 0} string entries"); + + // If we got here, the XML was successfully parsed and queried + Assert.True(true, "XML manifest is well-formed and properly escaped"); + } } } From 10a3cd62d6cadb32dfc9e022a8b5473ce3f9ffb0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Jan 2026 19:08:26 +0000 Subject: [PATCH 3/6] Remove unnecessary Assert.True in test as requested Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com> --- .../TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs b/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs index ad1a5a92d..5bfac1481 100644 --- a/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs +++ b/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs @@ -139,7 +139,6 @@ public void GetManifestForRegisteredProvider_ProperlyEscapesXmlCharacters() _output.WriteLine($"Found {stringElements?.Count ?? 0} string entries"); // If we got here, the XML was successfully parsed and queried - Assert.True(true, "XML manifest is well-formed and properly escaped"); } } } From 7b04510e4888001c7b20e5597e925d2b8f4b907b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 15 Jan 2026 23:47:13 +0000 Subject: [PATCH 4/6] Use MakeStringId helper to handle angle brackets and XML-escape string IDs Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com> --- src/TraceEvent/RegisteredTraceEventParser.cs | 35 +++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/TraceEvent/RegisteredTraceEventParser.cs b/src/TraceEvent/RegisteredTraceEventParser.cs index 89556425a..3638a900d 100644 --- a/src/TraceEvent/RegisteredTraceEventParser.cs +++ b/src/TraceEvent/RegisteredTraceEventParser.cs @@ -377,11 +377,11 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) { int value = mapEntries[k].Value; string valueName = new string((char*)(&enumBuffer[mapEntries[k].NameOffset])).Trim(); - string stringId = $"map_{enumName}{valueName}"; + string stringId = $"map_{MakeStringId(enumName)}{MakeStringId(valueName)}"; enumWriter.WriteLine(" ", value, stringId); if (emittedStringIds.Add(stringId)) { - enumLocalizations.WriteLine(" ", XmlUtilities.XmlEscape(stringId), XmlUtilities.XmlEscape(valueName)); + enumLocalizations.WriteLine(" ", stringId, XmlUtilities.XmlEscape(valueName)); } } if (enumInfo->Flag == MAP_FLAGS.EVENTMAP_INFO_FLAG_MANIFEST_VALUEMAP) @@ -459,12 +459,12 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) manifest.WriteLine(" "); foreach (var keyValue in keywords) { - string stringId = $"keyword_{keyValue.Value}"; + string stringId = $"keyword_{MakeStringId(keyValue.Value)}"; manifest.WriteLine(" ", XmlUtilities.XmlEscape(keyValue.Value), stringId, keyValue.Key); if (emittedStringIds.Add(stringId)) { - localizedStrings.WriteLine(" ", XmlUtilities.XmlEscape(stringId), XmlUtilities.XmlEscape(keyValue.Value)); + localizedStrings.WriteLine(" ", stringId, XmlUtilities.XmlEscape(keyValue.Value)); } } manifest.WriteLine(" "); @@ -474,12 +474,12 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) foreach (var taskValue in tasks.Keys) { var task = tasks[taskValue]; - string taskStringId = $"task_{task.Name}"; + string taskStringId = $"task_{MakeStringId(task.Name)}"; manifest.WriteLine(" ", XmlUtilities.XmlEscape(task.Name), taskStringId, taskValue, task.Opcodes == null ? "/" : ""); // If no opcodes, terminate immediately. if (emittedStringIds.Add(taskStringId)) { - localizedStrings.WriteLine(" ", XmlUtilities.XmlEscape(taskStringId), XmlUtilities.XmlEscape(task.Name)); + localizedStrings.WriteLine(" ", taskStringId, XmlUtilities.XmlEscape(task.Name)); } if (task.Opcodes != null) { @@ -487,12 +487,12 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) manifest.WriteLine(" "); foreach (var keyValue in task.Opcodes) { - string opcodeStringId = $"opcode_{task.Name}{keyValue.Value}"; + string opcodeStringId = $"opcode_{MakeStringId(task.Name)}{MakeStringId(keyValue.Value)}"; manifest.WriteLine(" ", XmlUtilities.XmlEscape(keyValue.Value), opcodeStringId, keyValue.Key); if (emittedStringIds.Add(opcodeStringId)) { - localizedStrings.WriteLine(" ", XmlUtilities.XmlEscape(opcodeStringId), XmlUtilities.XmlEscape(keyValue.Value)); + localizedStrings.WriteLine(" ", opcodeStringId, XmlUtilities.XmlEscape(keyValue.Value)); } } manifest.WriteLine(" "); @@ -573,6 +573,25 @@ private static string MakeLegalIdentifier(string name) return name; } + /// + /// Creates a string ID suitable for use in XML manifest string tables. + /// If the value starts with '<' and ends with '>', strips them off. + /// Otherwise, XML-escapes the value to make it a valid XML attribute value. + /// + private static string MakeStringId(string value) + { + if (value.Length >= 2 && value[0] == '<' && value[value.Length - 1] == '>') + { + // Strip the angle brackets + return value.Substring(1, value.Length - 2); + } + else + { + // XML-escape the value + return XmlUtilities.XmlEscape(value); + } + } + /// /// Generates a space separated list of set of keywords 'keywordSet' using the table 'keywords' /// It will generate new keyword names if needed and add them to 'keywords' if they are not present. From 790ccd5b3f59e963017ed2b2db85618dd148bfda Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 15 Jan 2026 23:48:04 +0000 Subject: [PATCH 5/6] Simplify MakeStringId to always XML-escape without stripping angle brackets Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com> --- src/TraceEvent/RegisteredTraceEventParser.cs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/TraceEvent/RegisteredTraceEventParser.cs b/src/TraceEvent/RegisteredTraceEventParser.cs index 3638a900d..e102259c7 100644 --- a/src/TraceEvent/RegisteredTraceEventParser.cs +++ b/src/TraceEvent/RegisteredTraceEventParser.cs @@ -575,21 +575,11 @@ private static string MakeLegalIdentifier(string name) /// /// Creates a string ID suitable for use in XML manifest string tables. - /// If the value starts with '<' and ends with '>', strips them off. - /// Otherwise, XML-escapes the value to make it a valid XML attribute value. + /// XML-escapes the value to make it a valid XML attribute value. /// private static string MakeStringId(string value) { - if (value.Length >= 2 && value[0] == '<' && value[value.Length - 1] == '>') - { - // Strip the angle brackets - return value.Substring(1, value.Length - 2); - } - else - { - // XML-escape the value - return XmlUtilities.XmlEscape(value); - } + return XmlUtilities.XmlEscape(value); } /// From a0cfabe473d155a0aec507f8044c8d3edfec7e0a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 16 Jan 2026 00:33:52 +0000 Subject: [PATCH 6/6] Remove MakeStringId helper and use XmlUtilities.XmlEscape directly with optimizations Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com> --- src/TraceEvent/RegisteredTraceEventParser.cs | 35 +++++++++----------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/TraceEvent/RegisteredTraceEventParser.cs b/src/TraceEvent/RegisteredTraceEventParser.cs index e102259c7..72659ca5c 100644 --- a/src/TraceEvent/RegisteredTraceEventParser.cs +++ b/src/TraceEvent/RegisteredTraceEventParser.cs @@ -377,11 +377,12 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) { int value = mapEntries[k].Value; string valueName = new string((char*)(&enumBuffer[mapEntries[k].NameOffset])).Trim(); - string stringId = $"map_{MakeStringId(enumName)}{MakeStringId(valueName)}"; + string escapedValueName = XmlUtilities.XmlEscape(valueName); + string stringId = XmlUtilities.XmlEscape($"map_{enumName}{valueName}"); enumWriter.WriteLine(" ", value, stringId); if (emittedStringIds.Add(stringId)) { - enumLocalizations.WriteLine(" ", stringId, XmlUtilities.XmlEscape(valueName)); + enumLocalizations.WriteLine(" ", stringId, escapedValueName); } } if (enumInfo->Flag == MAP_FLAGS.EVENTMAP_INFO_FLAG_MANIFEST_VALUEMAP) @@ -459,12 +460,13 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) manifest.WriteLine(" "); foreach (var keyValue in keywords) { - string stringId = $"keyword_{MakeStringId(keyValue.Value)}"; + string escapedValue = XmlUtilities.XmlEscape(keyValue.Value); + string stringId = XmlUtilities.XmlEscape($"keyword_{keyValue.Value}"); manifest.WriteLine(" ", - XmlUtilities.XmlEscape(keyValue.Value), stringId, keyValue.Key); + escapedValue, stringId, keyValue.Key); if (emittedStringIds.Add(stringId)) { - localizedStrings.WriteLine(" ", stringId, XmlUtilities.XmlEscape(keyValue.Value)); + localizedStrings.WriteLine(" ", stringId, escapedValue); } } manifest.WriteLine(" "); @@ -474,12 +476,13 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) foreach (var taskValue in tasks.Keys) { var task = tasks[taskValue]; - string taskStringId = $"task_{MakeStringId(task.Name)}"; - manifest.WriteLine(" ", XmlUtilities.XmlEscape(task.Name), taskStringId, taskValue, + string escapedTaskName = XmlUtilities.XmlEscape(task.Name); + string taskStringId = XmlUtilities.XmlEscape($"task_{task.Name}"); + manifest.WriteLine(" ", escapedTaskName, taskStringId, taskValue, task.Opcodes == null ? "/" : ""); // If no opcodes, terminate immediately. if (emittedStringIds.Add(taskStringId)) { - localizedStrings.WriteLine(" ", taskStringId, XmlUtilities.XmlEscape(task.Name)); + localizedStrings.WriteLine(" ", taskStringId, escapedTaskName); } if (task.Opcodes != null) { @@ -487,12 +490,13 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) manifest.WriteLine(" "); foreach (var keyValue in task.Opcodes) { - string opcodeStringId = $"opcode_{MakeStringId(task.Name)}{MakeStringId(keyValue.Value)}"; + string escapedOpcodeName = XmlUtilities.XmlEscape(keyValue.Value); + string opcodeStringId = XmlUtilities.XmlEscape($"opcode_{task.Name}{keyValue.Value}"); manifest.WriteLine(" ", - XmlUtilities.XmlEscape(keyValue.Value), opcodeStringId, keyValue.Key); + escapedOpcodeName, opcodeStringId, keyValue.Key); if (emittedStringIds.Add(opcodeStringId)) { - localizedStrings.WriteLine(" ", opcodeStringId, XmlUtilities.XmlEscape(keyValue.Value)); + localizedStrings.WriteLine(" ", opcodeStringId, escapedOpcodeName); } } manifest.WriteLine(" "); @@ -573,15 +577,6 @@ private static string MakeLegalIdentifier(string name) return name; } - /// - /// Creates a string ID suitable for use in XML manifest string tables. - /// XML-escapes the value to make it a valid XML attribute value. - /// - private static string MakeStringId(string value) - { - return XmlUtilities.XmlEscape(value); - } - /// /// Generates a space separated list of set of keywords 'keywordSet' using the table 'keywords' /// It will generate new keyword names if needed and add them to 'keywords' if they are not present.