Skip to content

OutputHandler.Encoding is silently overridden by a byte order mark, and decode failures are sometimes discarded #76

Description

@matt-edmondson

Summary

Two independent defects make OutputHandler.Encoding unreliable. Both were measured at main
(afa8bc1), .NET SDK 10.0.401, Linux, driving RunCommand.Execute with
new UTF8Encoding(false, throwOnInvalidBytes: true).

1. A byte order mark replaces the requested encoding

CreateStartInfo sets StandardOutputEncoding/StandardErrorEncoding, and
AsyncProcessStreamReader then reads process.StandardOutput. Process builds those
StreamReaders with detectEncodingFromByteOrderMarks: true, so when the output happens to begin
with a BOM the reader switches encodings and ignores what the caller asked for.

bytes on stdout requested actual result
FF FE 68 00 69 00 (UTF-16LE BOM + hi) strict UTF-8 hi — decoded as UTF-16LE, no error
FE FF 00 68 00 69 (UTF-16BE BOM + hi) strict UTF-8 hi — decoded as UTF-16BE, no error
FF FE alone strict UTF-8 exit 0, empty output, no error
FF alone strict UTF-8 DecoderFallbackException (correct)

The third row is the case ktsu-dev/GitBranchStateCache#27 reported as "a decode failure is raised
only sometimes". The cause is not that the failure is intermittent — those two bytes are eaten as a
byte order mark, leaving nothing to decode.

This matters beyond strict encodings: any command whose output begins with FF FE is reinterpreted
as UTF-16 text, which is a silent corruption of captured output rather than a reported error.

2. A faulted read is discarded when the process is still running

AsyncProcessStreamReader.Start loops until the process exits, restarting whichever read has
completed:

if (outputTask.IsCompleted)
{
    outputTask = ReadAndCallback(process.StandardOutput, ...);
}

A faulted task is also a completed one. When a read fails to decode while the process is still
alive, the next pass overwrites the faulted task with a fresh read, and the exception is never
observed — the fresh read then returns 0 at EOF and the call reports success.

Whether the failure surfaces depends purely on whether the process exits before the loop comes
around again. Measured with the repository's own suite, a decode-failure test written against a
short-lived process failed roughly one run in six. Made deterministic with
sh -c "cat bad-bytes; sleep 1", it is silent every time.

This is what makes a genuine decode failure look intermittent, and it applies to any exception a
read or an OutputHandler callback raises, not only decoding.

Acceptance criteria

  • OutputHandler.Encoding decides how output is decoded regardless of any byte order mark
  • A BOM matching the requested encoding is still stripped, so captured output does not gain a
    leading U+FEFF
  • A read that faults while the process is still running surfaces its exception rather than
    being replaced
  • The existing 35 tests still pass

Note

Fixing these does not on its own unblock ktsu-dev/GitBranchStateCache#27, which also needs a way
to redirect or close the child's stdin. That is a separate gap in CommandOptions and is not
covered here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions