diff --git a/src/TraceEvent/RegisteredTraceEventParser.cs b/src/TraceEvent/RegisteredTraceEventParser.cs index 7ed71299c..72659ca5c 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,21 @@ 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}"; + string escapedValueName = XmlUtilities.XmlEscape(valueName); + string stringId = XmlUtilities.XmlEscape($"map_{enumName}{valueName}"); + enumWriter.WriteLine(" ", value, stringId); if (emittedStringIds.Add(stringId)) { - enumLocalizations.WriteLine(" ", stringId, valueName); + enumLocalizations.WriteLine(" ", stringId, escapedValueName); } } 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 +460,13 @@ 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}"; + string escapedValue = XmlUtilities.XmlEscape(keyValue.Value); + string stringId = XmlUtilities.XmlEscape($"keyword_{keyValue.Value}"); + manifest.WriteLine(" ", + escapedValue, stringId, keyValue.Key); if (emittedStringIds.Add(stringId)) { - localizedStrings.WriteLine(" ", stringId, keyValue.Value); + localizedStrings.WriteLine(" ", stringId, escapedValue); } } manifest.WriteLine(" "); @@ -473,12 +476,13 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid) foreach (var taskValue in tasks.Keys) { var task = tasks[taskValue]; - manifest.WriteLine(" ", task.Name, task.Name, 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. - string taskStringId = $"task_{task.Name}"; if (emittedStringIds.Add(taskStringId)) { - localizedStrings.WriteLine(" ", taskStringId, task.Name); + localizedStrings.WriteLine(" ", taskStringId, escapedTaskName); } if (task.Opcodes != null) { @@ -486,12 +490,13 @@ 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}"; + string escapedOpcodeName = XmlUtilities.XmlEscape(keyValue.Value); + string opcodeStringId = XmlUtilities.XmlEscape($"opcode_{task.Name}{keyValue.Value}"); + manifest.WriteLine(" ", + escapedOpcodeName, opcodeStringId, keyValue.Key); if (emittedStringIds.Add(opcodeStringId)) { - localizedStrings.WriteLine(" ", opcodeStringId, keyValue.Value); + localizedStrings.WriteLine(" ", opcodeStringId, escapedOpcodeName); } } manifest.WriteLine(" "); diff --git a/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs b/src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs index f182b97df..5bfac1481 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,77 @@ 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 + } } }