Skip to content

Preserve UTF-8 characters in process log output - #241

Open
bob sira (bobsira) wants to merge 1 commit into
mainfrom
fix/process-monitor-utf8
Open

bob sira (bobsira) wants to merge 1 commit into
mainfrom
fix/process-monitor-utf8

Conversation

@bobsira

Copy link
Copy Markdown
Contributor

Summary

Fixes #121 for UTF-8 child-process output.

The workaround introduced by #122 allowed processing to continue but replaced high-bit bytes with ?. The current standard formatter retained that behavior, corrupting valid multibyte characters in JSON and XML process logs.

  • Preserve complete 2-, 3-, and 4-byte UTF-8 sequences, validating them with the existing Utility::IsTextUTF8 helper.
  • Retain ? replacement for invalid bytes and NUL in the formatter, while preserving valid characters elsewhere in the line.
  • Leave ASCII handling, custom formatting, pipe line framing, and the output schema unchanged.
  • Add regression coverage and document the expected child-process encoding.

This does not introduce legacy Windows code-page detection/conversion or repair text already corrupted by the child application.

Validation

  • Built LogMonitor and LogMonitorTests in x64 Release.
  • Before the fix, the standard-format UTF-8 regression failed; ASCII/NUL handling and custom UTF-8 output passed.
  • After the fix, all 7 selected process-formatting and utility tests passed. Coverage includes mixed 2/3/4-byte characters, long lines, malformed/truncated sequences, and valid characters around invalid bytes.
  • Real child-process checks compared the preserved baseline executable with the candidate. The baseline corrupted UTF-8 as expected; the candidate preserved it in JSON, XML, and custom output, including six split-sequence cases at 4096-byte write boundaries and stderr. JSON/XML also preserved valid characters around malformed bytes while replacing the malformed bytes with ?.
  • The targeted lint run reports the same pre-existing missing <string> include diagnostic on both unchanged main and this branch; no new lint diagnostics were introduced.

Validation was scoped to process formatting and related utilities. The broader EventMonitor replay crash tracked in #238 is outside this change.

Validate and preserve complete UTF-8 sequences in standard JSON and XML process logs instead of replacing every high-bit byte. Retain question-mark replacement for invalid bytes, add regression coverage, and document the supported encoding.

Addresses #121.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Newly preserved U+FFFE and U+FFFF characters produce invalid XML and need XML-specific replacement.

1 open issue
What changed in this PR

This PR replaces the earlier byte-replacement workaround with UTF-8 preservation in standard process logs, addressing #121 while leaving custom formatting and pipe framing unchanged.

Changes:

  • Preserve valid multibyte UTF-8 sequences while replacing invalid bytes and NUL.
  • Add regression coverage for standard and custom formatting.
  • Document the expected child-process encoding and lack of code-page conversion.
File Description
LogMonitor/​src/​LogMonitor/​ProcessMonitor.cpp Validate and preserve UTF-8 sequences in standard output.
LogMonitor/​LogMonitorTests/​LogMonitorTests.cpp Cover UTF-8 preservation, malformed bytes, and existing formatting behavior.
LogMonitor/​docs/​README.md Explain UTF-8 expectations for child-process output.

Review effort: Balanced

if (sequenceLength > 0 &&
inputLine.size() - i >= static_cast<size_t>(sequenceLength) &&
Utility::IsTextUTF8(inputLine.data() + i, sequenceLength)) {
sanitized.append(inputLine, i, sequenceLength);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] process monitor does not support multi-byte characters

2 participants