Skip to content

[fix] Fix HTML logger exception on invalid XML chars in test display names #16051

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
Jakub JareΕ‘ (nohwnd) merged 2 commits into from
May 26, 2026
Merged

[fix] Fix HTML logger exception on invalid XML chars in test display names #16051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b3a783
Fix HTML logger exception on invalid XML chars in test display names
nohwnd May 22, 2026
82ef8e5
Fix HtmlLogger regex: use compiled static field, handle surrogate pai…
nohwnd May 22, 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
40 changes: 36 additions & 4 deletions src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.IO;
using System.Linq;
using System.Runtime.Serialization;
using System.Text.RegularExpressions;
using System.Threading;

using Microsoft.VisualStudio.TestPlatform.Extensions.HtmlLogger.ObjectModel;
Expand Down Expand Up @@ -36,6 +37,14 @@ public class HtmlLogger : ITestLoggerWithParameters
private readonly IHtmlTransformer _htmlTransformer;
private Dictionary<string, string?>? _parametersDictionary;

// Matches XML 1.0 invalid characters (excluding valid surrogate pairs).
// Valid chars per spec: #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]
// The pattern allows valid high+low surrogate pairs to pass through unchanged;
// lone surrogates are treated as invalid.
private static readonly Regex InvalidXmlCharsRegex = new(
@"[^\x09\x0A\x0D\x20-\uD7FF\uE000-\uFFFD\uD800-\uDFFF]|[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]",
RegexOptions.Compiled);

public HtmlLogger()
: this(new FileHelper(), new HtmlTransformer(), new DataContractSerializer(typeof(TestRunDetails)))
{
Expand Down Expand Up @@ -195,10 +204,10 @@ public void TestResultHandler(object? sender, TestResultEventArgs e)

var testResult = new ObjectModel.TestResult
{
DisplayName = e.Result.DisplayName ?? e.Result.TestCase.FullyQualifiedName,
FullyQualifiedName = e.Result.TestCase.FullyQualifiedName,
ErrorStackTrace = e.Result.ErrorStackTrace,
ErrorMessage = e.Result.ErrorMessage,
DisplayName = RemoveInvalidXmlChars(e.Result.DisplayName ?? e.Result.TestCase.FullyQualifiedName),
FullyQualifiedName = RemoveInvalidXmlChars(e.Result.TestCase.FullyQualifiedName),
ErrorStackTrace = RemoveInvalidXmlChars(e.Result.ErrorStackTrace),
ErrorMessage = RemoveInvalidXmlChars(e.Result.ErrorMessage),
TestResultId = e.Result.TestCase.Id,
Duration = GetFormattedDurationString(e.Result.Duration),
ResultOutcome = e.Result.Outcome
Expand Down Expand Up @@ -454,4 +463,27 @@ private static Guid GetExecutionId(TestPlatform.ObjectModel.TestResult testResul

return time.Count == 0 ? "< 1ms" : string.Join(" ", time);
}

/// <summary>
/// Removes characters that are invalid in XML 1.0 from a string.
/// </summary>
/// <remarks>
/// XML 1.0 valid characters: #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD].
/// Control characters in the range #x00-#x08, #x0B, #x0C, #x0E-#x1F are not valid and
/// will cause <see cref="DataContractSerializer"/> to throw an <see cref="System.Xml.XmlException"/>.
/// Invalid characters are replaced with their Unicode escape representation.
/// </remarks>
Comment on lines +467 to +475
private static string? RemoveInvalidXmlChars(string? str)
{
if (str is null)
{
return null;
}

@nohwnd Jakub JareΕ‘ (nohwnd) May 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

[Performance] Regex is re-instantiated on every call via the string overload

Regex.Replace(str, invalidChar, ...) with a const string pattern looks up (and possibly compiles) the regex on every invocation via .NET's internal cache (size 15). This method is called 4Γ— per test result; for a run with thousands of tests this accumulates.

Since the project targets netstandard2.0 and net48, [GeneratedRegex] isn't available, but a static readonly field with RegexOptions.Compiled eliminates the cache lookup overhead:

private static readonly Regex s_invalidXmlCharsRegex = new Regex(
    @"[^\x09\x0A\x0D\x20-\uD7FF\uE000-\uFFFD]",
    RegexOptions.Compiled);

Minor, but worth fixing given the hot-path nature of TestResultHandler.

@nohwnd Jakub JareΕ‘ (nohwnd) May 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed β€” the regex is now a private static readonly Regex InvalidXmlCharsRegex with RegexOptions.Compiled, eliminating the per-call lookup overhead.

πŸ”§ Iterated by PR Iteration Agent πŸ”§

// From xml spec (http://www.w3.org/TR/xml/#charsets) valid chars:
// #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]

@nohwnd Jakub JareΕ‘ (nohwnd) May 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

[Correctness] Surrogate pairs (supplementary Unicode characters) are incorrectly stripped

The regex [^\x09\x0A\x0D\x20-\uD7FF\uE000-\uFFFD] has a gap between \uD7FF and \uE000 that covers the entire surrogate range \uD800–\uDFFF. In .NET strings, supplementary Unicode characters (U+10000–U+10FFFF, including all emoji) are encoded as surrogate pairs β€” two char values both in that range.

DataContractSerializer / XmlWriter handle surrogate pairs correctly and serialize them as the corresponding supplementary Unicode code point, which is valid XML 1.0 ([#x10000-#x10FFFF]). So DataContractSerializer will not throw on a string like "Test(πŸ˜€)".

The current code will corrupt such strings: "Test(πŸ˜€)" β†’ "Test(\uD83D\uDE00)" β€” two broken escape sequences instead of the emoji. A test whose [DataRow] argument contains an emoji will have its display name silently mangled in the HTML report.

Suggested fix β€” allow valid surrogate pairs to pass through by checking for a complete surrogate pair before replacing:

private static readonly Regex s_invalidXmlCharsRegex =
    new Regex(@"[^\x09\x0A\x0D\x20-\uD7FF\uE000-\uFFFD]|[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]",
              RegexOptions.Compiled);

private static string? RemoveInvalidXmlChars(string? str)
{
    if (str is null)
        return null;

    // Allow valid surrogate pairs (they represent U+10000–U+10FFFF, which are valid XML chars).
    // Only lone surrogates are truly invalid.
    return s_invalidXmlCharsRegex.Replace(str, m =>
        m.Value.Length == 1 ? $@"\u{(ushort)m.Value[0]:x4}" : m.Value);
}

Alternatively, use char.IsHighSurrogate/char.IsLowSurrogate in a manual loop, which is clearer and avoids nested regex complexity.

@nohwnd Jakub JareΕ‘ (nohwnd) May 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed β€” the surrogate range \uD800-\uDFFF is now excluded from the negated character class in the first alternative, so valid surrogate pairs pass through untouched. Two additional alternatives handle lone surrogates (unpaired high or low) using lookahead/lookbehind. Added a test that verifies "Test(πŸ˜€)" is preserved as-is after sanitization.

πŸ”§ Iterated by PR Iteration Agent πŸ”§

// Valid surrogate pairs (representing U+10000–U+10FFFF) are allowed through unchanged;
// lone surrogates are replaced with their Unicode escape representation.
return InvalidXmlCharsRegex.Replace(str, m => $@"\u{(ushort)m.Value[0]:x4}");
}
Comment on lines +483 to +488
}
44 changes: 44 additions & 0 deletions test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,50 @@ public void TestResultHandlerShouldCreateTestResultProperly()
Assert.AreEqual("1s", result.Duration);
}

[TestMethod]
public void TestResultHandlerShouldSanitizeInvalidXmlCharsInDisplayName()
Comment thread
nohwnd marked this conversation as resolved.
{
// Characters like \x01 (SOH) are invalid in XML 1.0 and would cause DataContractSerializer to throw.
var testCase = CreateTestCase("Pass1");
testCase.FullyQualifiedName = "fully";
testCase.Source = "abc/def.dll";

var testResult = new ObjectModel.TestResult(testCase)
{
DisplayName = "TestMethod(\x01value)",
ErrorMessage = "error\x02message",
ErrorStackTrace = "stack\x03trace",
};

_htmlLogger.TestResultHandler(new object(), new Mock<TestResultEventArgs>(testResult).Object);

var result = _htmlLogger.TestRunDetails!.ResultCollectionList!.First().ResultList!.First();

Assert.AreEqual(@"TestMethod(\u0001value)", result.DisplayName);
Assert.AreEqual(@"error\u0002message", result.ErrorMessage);
Assert.AreEqual(@"stack\u0003trace", result.ErrorStackTrace);
}

[TestMethod]
public void TestResultHandlerShouldPreserveValidSurrogatePairsInDisplayName()
{
// Valid surrogate pairs (e.g. emoji U+1F600 = πŸ˜€) are valid XML 1.0 chars and must NOT be mangled.
var testCase = CreateTestCase("Pass1");
testCase.FullyQualifiedName = "fully";
testCase.Source = "abc/def.dll";

var testResult = new ObjectModel.TestResult(testCase)
{
DisplayName = "Test(πŸ˜€)", // πŸ˜€ is U+1F600, encoded as surrogate pair \uD83D\uDE00
};

_htmlLogger.TestResultHandler(new object(), new Mock<TestResultEventArgs>(testResult).Object);

var result = _htmlLogger.TestRunDetails!.ResultCollectionList!.First().ResultList!.First();

Assert.AreEqual("Test(πŸ˜€)", result.DisplayName, "Valid surrogate pairs should pass through unchanged.");
}

[TestMethod]
public void GetFormattedDurationStringShouldGiveCorrectFormat()
{
Expand Down
Loading