Skip to content

A diff containing one non-UTF-8 path stalls for the full DiffTimeout and is reported as a timeout instead of "not valid UTF-8" #50

Description

@matt-edmondson

What's wrong

GitRunner.RunAsync (GitBranchStateCache/Git/GitRunner.cs) starts both reads with a strict UTF-8 decoder:

Task<string> standardOutput = process.StandardOutput.ReadToEndAsync(CancellationToken.None);
Task<string> standardError  = process.StandardError.ReadToEndAsync(CancellationToken.None);
...
await process.WaitForExitAsync(linked.Token);

When git writes bytes that aren't valid UTF-8, ReadToEndAsync throws DecoderFallbackException and the task faults. After that, nothing reads the pipe. The fault is only observed after the process exits, but if git still has more than one pipe buffer (about 64 KB) to write, git blocks on the full pipe and never exits. WaitForExitAsync then runs until invocation.Timeout, the process is killed, and the catch (OperationCanceledException) branch returns "The git command exceeded its timeout.". The catch (DecoderFallbackException) branch, which was written for exactly this case, is never reached.

Why it matters

  • diff-tree on a branch with one non-UTF-8 path (for example an asset named with Latin-1 bytes) plus more than about 64 KB of other changes:
    • Each request stalls for the whole DiffTimeout (2 minutes) and returns DiffOutcome.Timeout() rather than a clear "cannot be read" failure.
    • Failures aren't cached, so every heartbeat repeats the stall.
    • DiffCache followers wait DiffTimeout and then recompute, which stalls again.
    • The operator sees what looks like a slow git rather than a data problem.
  • A single ref name with bytes ≥ 0x80 (git allows these) has the same root cause and a second symptom:
    • ls-remote --heads and for-each-ref return the UTF-8 failure even when their output is small.
    • The AdmissionGate probe never looks at the probe's output, only whether it succeeded, yet the probe fails. So every caller of that repository gets not-admitted, or a timeout (504) if more than 64 KB of refs follow the bad one.
    • RefResolver returns null, and /branches and /state return refs-unreadable.

Reproduction

I built a repository with a path b\xff.uasset plus 1,500 further files (219 KB of -z output). I then called the real GitRunner with a 10 s timeout:

diff-tree: exit=-1 timedOut=True ok=False elapsed=10.0s summary='The git command exceeded its timeout.'

With the bad path and only a few other files, the call returns immediately with the intended not valid UTF-8 summary. The difference is purely whether git outlives a full pipe buffer.

Suggested fix

  • Read stdout and stderr as raw bytes (process.StandardOutput.BaseStream.CopyToAsync(memoryStream)) and decode strictly only after the process has exited. The pipes are always drained, git exits promptly, and the DecoderFallbackException branch reports the real cause. Alternatively, observe a faulted read task and kill the process immediately, but draining bytes is simpler and cannot deadlock.
  • Separately, the admission probe (ls-remote) only needs the exit code. It shouldn't decode its output at all, so an unusual ref name can't deny admission to a whole repository.

Acceptance criteria

  • A diff-tree whose output contains invalid UTF-8 and is larger than the pipe buffer returns the "not valid UTF-8" result well inside DiffTimeout, with TimedOut == false.
  • A regression test covers large output with a non-UTF-8 path, for example a fake git script that writes \xff followed by 200 KB.
  • Admission succeeds for a reachable repository that has a non-UTF-8 branch name.

Activity

  1. matt-edmondson commented on Sep 28, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    Category: Bug
    Priority: High

    Why High: A single non-UTF-8 path or ref name fails in the wrong way, and the failure repeats on every request.

    • Each diff-tree request stalls for the full DiffTimeout (2 minutes). Failures aren't cached, so every heartbeat and every DiffCache follower repeats the stall.
    • The admission probe decodes output it never reads. One unusual branch name can therefore deny admission to a whole repository, or return 504.

    Both paths are in core request handling, the repro is concrete, and the misleading "timeout" message sends operators looking in the wrong place.

    Suggested area: GitBranchStateCache/Git/GitRunner.cs (process I/O). Secondary: the AdmissionGate probe and RefResolver.

    Duplicates / related:

    Next steps: Take the suggested byte-draining approach: copy the raw streams, then decode after exit. Stop the ls-remote admission probe decoding output. Add the proposed fake-git regression test (\xff followed by 200 KB).


    Generated by Claude Code

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

Metadata

Metadata

Labels

bugSomething isn't workingreadyFully specified; implement as written

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions