[fix] Fix tilde/exclamation characters corrupted in TerminalLogger test output by nohwnd · Pull Request #16046 · microsoft/vstest

nohwnd · 2026-05-19T14:31:51Z

Caution

Security scanning requires review for Issue Repro Triage & Auto-Fix 🔍

Details

Potential security threats were detected in the agent output. The workflow output should be reviewed before merging.

Review the workflow run logs for details.

Summary

🤖 This is an automated fix for issue #15268.

Fixes #15268 — ~ and ! characters in test output (assertion messages, test display names, stack traces) were being replaced with _ characters when using the TerminalLogger (dotnet test with live logger enabled).

Root Cause

MSBuildLogger.Escape() serializes multi-line test messages to a single line for transport over stdout between vstest.console and the MSBuild VSTestTask2. The protocol used:

|||| as a field separator
~~~~ (4 tildes) as a \r placeholder
!!!! (4 exclamation marks) as a \n placeholder

To prevent user data from being mistaken for these escape sequences, the code pre-sanitized input by replacing ~~~~ and !!!! with ____:

// OLD — corrupts user data
.Replace("||||", "____").Replace("~~~~", "____").Replace("!!!!", "____")
.Replace("\r", "~~~~").Replace("\n", "!!!!");

The problem: a string like ~~~~~ (5 tildes) would have its first 4 tildes replaced with ____, yielding ____~. The user sees ____~ instead of ~~~~~.

Fix

Replace the escape sequences for CR and LF with ASCII control characters \x02 (STX) and \x03 (ETX), which cannot appear in normal test output:

// NEW — safe, no user data corruption
.Replace("||||", "____")
.Replace("\r", "\x02")
.Replace("\n", "\x03");

The VSTestTask2 decoder is updated to match:

// OLD
p?.Replace("~~~~", "\r").Replace("!!!!", "\n")
// NEW
p?.Replace("\x02", "\r").Replace("\x03", "\n")

Tests

Three regression tests added to VSTestTask2RegressionTests.cs:

LogEventsFromTextOutput_TestFailed_TildeCharsInMessage_ShouldNotBeCorrupted — 5 tilde chars survive round-trip
LogEventsFromTextOutput_TestFailed_ExclamationCharsInMessage_ShouldNotBeCorrupted — 4 ! chars survive round-trip
LogEventsFromTextOutput_TestFailed_NewlinesEncodedWithControlChars_ShouldBeRestored — \r\n newlines are correctly round-tripped via the new control-char encoding

🔍 Triaged by Issue Repro Triage & Auto-Fix 🔍

When test output contained sequences of 4+ tilde characters (~~~~) or 4+ exclamation marks ( { echo ___BEGIN___COMMAND_OUTPUT_MARKER___; PS1=;PS2=;unset HISTFILE; EC=0; echo ___BEGIN___COMMAND_DONE_MARKER___0; } { echo ___BEGIN___COMMAND_OUTPUT_MARKER___; PS1=;PS2=;unset HISTFILE; EC=0; echo ___BEGIN___COMMAND_DONE_MARKER___0; }), the MSBuildLogger.Escape method would replace them with underscores (____) before applying the actual CR/LF encoding. This caused any ~~ or { echo ___BEGIN___COMMAND_OUTPUT_MARKER___; PS1=;PS2=;unset HISTFILE; EC=0; echo ___BEGIN___COMMAND_DONE_MARKER___0; } sequences in test names, assertion messages, or stack traces to appear corrupted in the terminal logger. Root cause: the old encoding used ~~~~ as a CR placeholder and { echo ___BEGIN___COMMAND_OUTPUT_MARKER___; PS1=;PS2=;unset HISTFILE; EC=0; echo ___BEGIN___COMMAND_DONE_MARKER___0; } { echo ___BEGIN___COMMAND_OUTPUT_MARKER___; PS1=;PS2=;unset HISTFILE; EC=0; echo ___BEGIN___COMMAND_DONE_MARKER___0; } as an LF placeholder, and sanitized input by pre-replacing those sequences with ____. This sanitization destructively modified user data. Fix: Use ASCII control characters STX (\x02) for CR and ETX (\x03) for LF as the escape sequences. These control characters cannot appear in normal test output (test names, assertion messages, stack traces), so no sanitization of user data is needed. Fixes #15268 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes corruption of ~ and ! characters in TerminalLogger-transported test output by changing the newline escaping protocol between vstest.console and VSTestTask2.

Changes:

Switched CR/LF encoding from printable sequences (~~~~/!!!!) to ASCII control chars (\x02/\x03) in MSBuildLogger.
Updated VSTestTask2 decoding logic to restore CR/LF from \x02/\x03.
Added regression tests covering tilde/exclamation preservation and newline round-tripping.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
test/Microsoft.TestPlatform.Build.UnitTests/VSTestTask2RegressionTests.cs	Adds regression tests to validate decoding and prevent output corruption regressions.
src/vstest.console/Internal/MSBuildLogger.cs	Updates message formatting/escaping to use control characters for newline transport.
src/Microsoft.TestPlatform.Build/Tasks/VSTestTask2.cs	Updates message decoding to match the new control-character encoding.

+            // Cleanup characters that we are using ourselves to delimit the message.
+            .Replace("||||", "____")
            // Replace new line characters that would change how the message is consumed.
-            .Replace("\r", "~~~~").Replace("\n", "!!!!");
+            // Use ASCII control characters STX (\x02) and ETX (\x03) which cannot appear in normal
+            // test output, rather than ~~~~ and !!!! which can appear in test data and get corrupted.
+            .Replace("\r", "\x02")
+            .Replace("\n", "\x03");


+            .Replace("\r", "\x02")
+            .Replace("\n", "\x03");


+            // Use ASCII control characters STX (\x02) and ETX (\x03) which cannot appear in normal
+            // test output, rather than ~~~~ and !!!! which can appear in test data and get corrupted.


nohwnd

Review Summary

The fix is correct and well-scoped. Using ASCII control characters \x02/\x03 (STX/ETX) as newline placeholders instead of ~~~~/!!!! eliminates the data-corruption bug: the old approach would mangle any user input containing those exact 4-character sequences.

IPC Protocol Stability note: This is a wire-protocol change between MSBuildLogger (vstest.console) and VSTestTask2. There is no version negotiation — a mismatched deployment (old VSTestTask2 + new vstest.console) would see literal \x02/\x03 characters in output instead of newlines, with no error. Since both components co-ship in the .NET SDK this is low practical risk, but it's worth being aware of for any scenario where these are installed independently.

One inline comment on test coverage (not blocking).

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

nohwnd · 2026-05-19T15:06:39Z

+
+
    private static TestableVSTestTask2 CreateVSTestTask2()
    {


[Backward Compatibility & Rollback Safety / Test Coverage] The three regression tests bypass MSBuildLogger.FormatMessage() and only exercise the VSTestTask2 decoder side. They feed pre-encoded strings (with \x02/\x03 already in place) directly to TestLogEventsFromTextOutput, so they don't verify that the encoder actually produces the control characters the decoder expects.

A round-trip test along these lines would provide stronger guarantees that the encoder/decoder pair stay in sync:

// Arrange string rawMessage = "Assert failed.\r\n at MyTest()"; string encoded = MSBuildLogger.FormatMessage("test-failed", rawMessage, "TestFile.cs", "42"); // ... feed encoded through task ... // Assert: decoded message matches rawMessage

This is a test-coverage observation — not blocking — but a gap worth filling to guard against future encoder/decoder drift.

The full encoder→decoder round-trip is already covered by MSBuildLoggerEncoderTests.cs (in vstest.console.UnitTests), which calls MSBuildLogger.FormatMessage() for encoding and the local Decode helper for decoding. Added a cross-reference comment to the newlines test in VSTestTask2RegressionTests.cs pointing there.

🔧 Iterated by PR Iteration Agent 🔧

Add MSBuildLoggerEncoderTests with four round-trip tests that call MSBuildLogger.FormatMessage() (encoder) and then decode the result using the same logic as VSTestTask2.TryGetMessage (decoder), verifying that tilde chars, exclamation chars, newlines, and multiple fields all survive the full encode/decode cycle. Make FormatMessage internal (was private) to allow access from the vstest.console.UnitTests project via InternalsVisibleTo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

nohwnd · 2026-05-19T15:31:34Z

Commit pushed: 667f4e7

🔧 Iterated by PR Iteration Agent 🔧

nohwnd

Review Summary (updated commit)

The new commit addresses the round-trip test gap from the prior review by adding MSBuildLoggerEncoderTests.cs and exposing FormatMessage as internal. The core fix remains correct and well-scoped.

One non-blocking observation on the new test file: the Decode helper duplicates VSTestTask2.TryGetMessage decoder logic locally rather than invoking the real decoder, which means encoder/decoder drift could go undetected. See inline comment.

Everything else looks good.

🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

nohwnd · 2026-05-19T15:42:14Z

+{
+    // Mirrors VSTestTask2.TryGetMessage decode logic to keep the round-trip self-contained.
+    private static string?[] Decode(string encoded)
+    {


[Test Coverage] The Decode helper duplicates the decoder logic from VSTestTask2.TryGetMessage rather than calling the real decoder. If someone updates the VSTestTask2 decoder (e.g., changes \x02→\r to something else) without updating this copy, these round-trip tests will still pass — but real round-trips will break.

Consider pulling the decode logic through the actual VSTestTask2 path (e.g., via TestLogEventsFromTextOutput as done in VSTestTask2RegressionTests), or at minimum add a comment warning that this must stay in sync with VSTestTask2.TryGetMessage.

Non-blocking — the VSTestTask2RegressionTests already test the decoder directly, so the gap is partially mitigated.

Added a warning comment to the Decode helper noting it must stay in sync with VSTestTask2.TryGetMessage.

🔧 Iterated by PR Iteration Agent 🔧

… encoder tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

nohwnd · 2026-05-19T16:00:58Z

Commit pushed: 4ad352f

🔧 Iterated by PR Iteration Agent 🔧

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

        return input
-            // Cleanup characters that we are using ourselves to delimit the message
-            .Replace("||||", "____").Replace("~~~~", "____").Replace("!!!!", "____")
+            // Cleanup characters that we are using ourselves to delimit the message.
+            .Replace("||||", "____")
            // Replace new line characters that would change how the message is consumed.
-            .Replace("\r", "~~~~").Replace("\n", "!!!!");
+            // Use ASCII control characters STX (\x02) and ETX (\x03) which cannot appear in normal
+            // test output, rather than ~~~~ and !!!! which can appear in test data and get corrupted.
+            .Replace("\r", "\x02")
+            .Replace("\n", "\x03");


    /// <summary>
    /// Writes message to standard output, with the name of the message followed by the number of
-    /// parameters. With each parameter delimited by '||||', and newlines replaced with ~~~~ and !!!!.
+    /// parameters. With each parameter delimited by '||||', and newlines replaced with STX (\x02) and ETX (\x03).
    /// Such as:
    ///  ||||run-start1||||s:\t\mstest97\bin\Debug\net8.0\mstest97.dll


nohwnd

Review Summary (updated commit)

Both observations from the prior reviews have been addressed:

Round-trip test gap — MSBuildLoggerEncoderTests.cs now calls MSBuildLogger.FormatMessage() directly for encoding, covering the full encoder→decoder path.
Decoder drift risk — The Decode helper in the test file now carries a clear warning comment that it must stay in sync with VSTestTask2.TryGetMessage.

The core fix remains correct and well-scoped. No new issues found.

🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

Copilot AI review requested due to automatic review settings May 19, 2026 14:31

nohwnd added the agentic-threat-detected label May 19, 2026

Copilot AI reviewed May 19, 2026

View reviewed changes

nohwnd commented May 19, 2026

View reviewed changes

Address review: add sync warning to Decode helper and cross-reference…

4ad352f

… encoder tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 19, 2026 16:00

Copilot started reviewing on behalf of nohwnd May 19, 2026 16:03 View session

Copilot AI reviewed May 19, 2026

View reviewed changes

nohwnd commented May 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix] Fix tilde/exclamation characters corrupted in TerminalLogger test output#16046

[fix] Fix tilde/exclamation characters corrupted in TerminalLogger test output#16046
nohwnd wants to merge 3 commits into
mainfrom
fix/issue-15268-fe4cfcd940710ed8

nohwnd commented May 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

nohwnd left a comment

Uh oh!

nohwnd May 19, 2026

Uh oh!

nohwnd May 19, 2026

Uh oh!

nohwnd commented May 19, 2026

Uh oh!

nohwnd left a comment

Uh oh!

nohwnd May 19, 2026

Uh oh!

nohwnd May 19, 2026

Uh oh!

nohwnd commented May 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

nohwnd left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// Use ASCII control characters STX (\x02) and ETX (\x03) which cannot appear in normal
		// test output, rather than ~~~~ and !!!! which can appear in test data and get corrupted.

Conversation

nohwnd commented May 19, 2026

Summary

Root Cause

Fix

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

nohwnd left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

nohwnd May 19, 2026

Choose a reason for hiding this comment

Uh oh!

nohwnd May 19, 2026

Choose a reason for hiding this comment

Uh oh!

nohwnd commented May 19, 2026

Uh oh!

nohwnd left a comment

Choose a reason for hiding this comment

Review Summary (updated commit)

Uh oh!

nohwnd May 19, 2026

Choose a reason for hiding this comment

Uh oh!

nohwnd May 19, 2026

Choose a reason for hiding this comment

Uh oh!

nohwnd commented May 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

nohwnd left a comment

Choose a reason for hiding this comment

Review Summary (updated commit)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants