Repository navigation
Don't abort the run on an unpaired surrogate in a test name - #16513
Open
Azat Mukhametshin (azat-msft) wants to merge 1 commit into
Open
Azat Mukhametshin (azat-msft) wants to merge 1 commit into
Azat Mukhametshin (azat-msft) wants to merge 1 commit into
Conversation
…t#16512) Jsonite, used by .NET Framework testhosts, escapes every UTF-16 surrogate code unit on its own, so a string truncated in the middle of an astral character reaches the .NET runner as a lone \uD83D escape. System.Text.Json rejects unpaired surrogates by design, and since 18.11 that aborts the whole test run. Repair such escapes to U+FFFD before handing wire text to System.Text.Json, and stop Jsonite from emitting them in the first place. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Cross-target JSON handling changes affect test-run reliability and warrant final human review.
Review effort: Lite
Findings: None
What changed in this PR
Fixes test-run aborts caused by unpaired UTF-16 surrogate escapes in JSON.
Changes:
- Repairs invalid surrogate escapes before System.Text.Json parsing.
- Sanitizes unpaired surrogates in Jsonite output.
- Adds cross-target regression tests.
| File | Description |
|---|---|
test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/JsonDataSerializerSurrogateRegressionTests.cs |
Adds regression and edge-case coverage. |
src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/JsonSurrogates.cs |
Implements surrogate repair logic. |
src/Microsoft.TestPlatform.CommunicationUtilities/JsonDataSerializer.Stj.cs |
Applies repair to STJ parsing paths. |
src/Microsoft.TestPlatform.CommunicationUtilities/Json/Jsonite/Jsonite.cs |
Sanitizes unpaired surrogates when writing JSON. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #16512.
A test run on .NET Framework aborts completely when a single test name, error message or stack trace contains an unpaired UTF-16 surrogate:
This is easy to hit without meaning to. xUnit v2 truncates long theory arguments by UTF-16 length, so any emoji in theory data can be cut in half. Every result after the bad message is lost.
Why it broke in 18.11
.NET Framework testhosts serialize with Jsonite, which escapes every surrogate code unit on its own, so an emoji goes on the wire as \uD83D\uDE00. A truncated one goes as a lone \uD83D. Since 18.11 the .NET runner reads those messages with System.Text.Json, which rejects unpaired surrogates by design.
Before 18.11 the runner used Newtonsoft, which wrote non-ASCII literally, so the UTF-8 encoder on the channel silently turned the lone surrogate into U+FFFD and nothing ever noticed.
The change
Reader (
JsonSurrogates.cs, .NET only). Unpaired surrogate escapes are rewritten to an escaped U+FFFD before the text reaches System.Text.Json. This is the part that actually fixes the reported bug, because testhosts already in the wild emit these escapes and the runner has to cope with them. A vectorizedIndexOf(""\\u"")keeps the cost off the common path, and nothing is allocated unless a replacement is needed.Writer (
Jsonite.cs).WriteStringno longer emits an unpaired surrogate escape at all; complete pairs are still written as two escapes. Without this, a newly patched .NET Framework testhost would still break an older System.Text.Json runner under SDK version skew.The affected character is now reported as U+FFFD instead of killing the run, which matches what the .NET testhost has always done and what the Newtonsoft-based runner effectively did.
Verified
Reproduced end to end with a net48 testhost throwing an exception whose message ends in a lone high surrogate: the run aborts on
mainatTestRequestSender.OnExecutionMessageReceivedand reports1 failed, 7 passedwith this change.