Skip to content

Fix XML escaping in GetManifestForRegisteredProvider #2351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brianrob merged 6 commits into from
Jan 16, 2026
Merged

Fix XML escaping in GetManifestForRegisteredProvider #2351

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
afb98b0
Initial plan
Copilot Jan 14, 2026
7780952
Fix XML escaping in GetManifestForRegisteredProvider
Copilot Jan 14, 2026
10a3cd6
Remove unnecessary Assert.True in test as requested
Copilot Jan 14, 2026
7b04510
Use MakeStringId helper to handle angle brackets and XML-escape strin…
Copilot Jan 15, 2026
790ccd5
Simplify MakeStringId to always XML-escape without stripping angle br…
Copilot Jan 15, 2026
a0cfabe
Remove MakeStringId helper and use XmlUtilities.XmlEscape directly wi…
Copilot Jan 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 29 additions & 19 deletions src/TraceEvent/RegisteredTraceEventParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -361,35 +362,35 @@ 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(" <valueMap name=\"{0}\">", enumName);
enumWriter.WriteLine(" <valueMap name=\"{0}\">", XmlUtilities.XmlEscape(enumName));
}
else
{
enumWriter.WriteLine(" <bitMap name=\"{0}\">", enumName);
enumWriter.WriteLine(" <bitMap name=\"{0}\">", XmlUtilities.XmlEscape(enumName));
}

EVENT_MAP_ENTRY* mapEntries = &enumInfo->MapEntryArray;
for (int k = 0; k < enumInfo->EntryCount; k++)
{
int value = mapEntries[k].Value;
string valueName = new string((char*)(&enumBuffer[mapEntries[k].NameOffset])).Trim();
enumWriter.WriteLine(" <map value=\"0x{0:x}\" message=\"$(string.map_{1}{2})\"/>", value, enumName, valueName);
string stringId = $"map_{enumName}{valueName}";
string stringId = $"map_{MakeStringId(enumName)}{MakeStringId(valueName)}";
Copy link
Member

@brianrob brianrob Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concatenate enumName and valueName first, then call MakeStringId on the result.

Copy link
Contributor Author

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to concatenate strings before calling XmlUtilities.XmlEscape in commit a0cfabe.

enumWriter.WriteLine(" <map value=\"0x{0:x}\" message=\"$(string.{1})\"/>", value, stringId);
if (emittedStringIds.Add(stringId))
{
enumLocalizations.WriteLine(" <string id=\"{0}\" value=\"{1}\"/>", stringId, valueName);
enumLocalizations.WriteLine(" <string id=\"{0}\" value=\"{1}\"/>", stringId, XmlUtilities.XmlEscape(valueName));
}
}
if (enumInfo->Flag == MAP_FLAGS.EVENTMAP_INFO_FLAG_MANIFEST_VALUEMAP)
{
enumWriter.WriteLine(" </valueMap>", enumName);
enumWriter.WriteLine(" </valueMap>");
}
else
{
enumWriter.WriteLine(" </bitMap>", enumName);
enumWriter.WriteLine(" </bitMap>");
}

enumIntern[mapName] = enumWriter.ToString();
Expand Down Expand Up @@ -458,12 +459,12 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid)
manifest.WriteLine(" <keywords>");
foreach (var keyValue in keywords)
{
manifest.WriteLine(" <keyword name=\"{0}\" message=\"$(string.keyword_{1})\" mask=\"0x{2:x}\"/>",
keyValue.Value, keyValue.Value, keyValue.Key);
string stringId = $"keyword_{keyValue.Value}";
string stringId = $"keyword_{MakeStringId(keyValue.Value)}";
manifest.WriteLine(" <keyword name=\"{0}\" message=\"$(string.{1})\" mask=\"0x{2:x}\"/>",
XmlUtilities.XmlEscape(keyValue.Value), stringId, keyValue.Key);
if (emittedStringIds.Add(stringId))
{
localizedStrings.WriteLine(" <string id=\"{0}\" value=\"{1}\"/>", stringId, keyValue.Value);
localizedStrings.WriteLine(" <string id=\"{0}\" value=\"{1}\"/>", stringId, XmlUtilities.XmlEscape(keyValue.Value));
}
}
manifest.WriteLine(" </keywords>");
Expand All @@ -473,25 +474,25 @@ public static string GetManifestForRegisteredProvider(Guid providerGuid)
foreach (var taskValue in tasks.Keys)
{
var task = tasks[taskValue];
manifest.WriteLine(" <task name=\"{0}\" message=\"$(string.task_{1})\" value=\"{2}\"{3}>", task.Name, task.Name, taskValue,
string taskStringId = $"task_{MakeStringId(task.Name)}";
manifest.WriteLine(" <task name=\"{0}\" message=\"$(string.{1})\" value=\"{2}\"{3}>", XmlUtilities.XmlEscape(task.Name), taskStringId, taskValue,
task.Opcodes == null ? "/" : ""); // If no opcodes, terminate immediately.
string taskStringId = $"task_{task.Name}";
if (emittedStringIds.Add(taskStringId))
{
localizedStrings.WriteLine(" <string id=\"{0}\" value=\"{1}\"/>", taskStringId, task.Name);
localizedStrings.WriteLine(" <string id=\"{0}\" value=\"{1}\"/>", taskStringId, XmlUtilities.XmlEscape(task.Name));
}
if (task.Opcodes != null)
{
manifest.WriteLine(">");
manifest.WriteLine(" <opcodes>");
foreach (var keyValue in task.Opcodes)
{
manifest.WriteLine(" <opcode name=\"{0}\" message=\"$(string.opcode_{1}{2})\" value=\"{3}\"/>",
keyValue.Value, task.Name, keyValue.Value, keyValue.Key);
string opcodeStringId = $"opcode_{task.Name}{keyValue.Value}";
string opcodeStringId = $"opcode_{MakeStringId(task.Name)}{MakeStringId(keyValue.Value)}";
manifest.WriteLine(" <opcode name=\"{0}\" message=\"$(string.{1})\" value=\"{2}\"/>",
XmlUtilities.XmlEscape(keyValue.Value), opcodeStringId, keyValue.Key);
if (emittedStringIds.Add(opcodeStringId))
{
localizedStrings.WriteLine(" <string id=\"{0}\" value=\"{1}\"/>", opcodeStringId, keyValue.Value);
localizedStrings.WriteLine(" <string id=\"{0}\" value=\"{1}\"/>", opcodeStringId, XmlUtilities.XmlEscape(keyValue.Value));
}
}
manifest.WriteLine(" </opcodes>");
Expand Down Expand Up @@ -572,6 +573,15 @@ private static string MakeLegalIdentifier(string name)
return name;
}

/// <summary>
/// Creates a string ID suitable for use in XML manifest string tables.
/// XML-escapes the value to make it a valid XML attribute value.
/// </summary>
private static string MakeStringId(string value)
Copy link
Member

@brianrob brianrob Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than using this helper, just replace it with calls to XmlUtilities.XmlEscape. This will also allow for simplifying some cases above where we pass the same value multiple times.

Copy link
Contributor Author

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the MakeStringId helper and replaced with direct calls to XmlUtilities.XmlEscape in commit a0cfabe. Also optimized to avoid escaping the same values multiple times.

{
return XmlUtilities.XmlEscape(value);
}

/// <summary>
/// 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.
Expand Down
74 changes: 74 additions & 0 deletions src/TraceEvent/TraceEvent.Tests/Parsing/RegisteredTraceEventParserTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -66,5 +68,77 @@ public void GetManifestForRegisteredProvider_NoDuplicateStringTableEntries()
// Assert no duplicates exist
Assert.Empty(duplicates);
}

/// <summary>
/// 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.
/// </summary>
[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
}
}
}