Skip to content

Stop the admission probe decoding output it never reads, and guard the large non-UTF-8 diff [patch] - #82

Open
matt-edmondson wants to merge 1 commit into
mainfrom
claude/gbsc-50-non-utf8-large-output
Open

matt-edmondson wants to merge 1 commit into
mainfrom
claude/gbsc-50-non-utf8-large-output

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #50

What changed

  • GitInvocation.DiscardStandardOutput: when set, GitRunner still drains standard output but throws it away without decoding it. Standard error is kept and decoded leniently, because it is only shown and searched for git's own ASCII messages.
  • AdmissionGate sets this flag on its ls-remote --heads probe. A reachable repository with a non-UTF-8 branch name is now admitted, instead of every caller getting not-admitted, or a 504 when the bad name is followed by more than a pipe of refs.

The diff-tree stall

This half of the issue no longer reproduces on main. Since #27, GitRunner runs git through ktsu.RunCommand 1.9.5, which kills the command and rethrows when a read fails (ktsu-dev/RunCommand#89, fixed by RunCommand#96). This PR adds a regression test for it: a non-UTF-8 path followed by 200 KB of output must come back as "not valid UTF-8" well inside the timeout, with TimedOut == false. That test already passes without the source change here. It guards against the bug returning, for example through a RunCommand downgrade.

Tests

New tests:

  • GitRunnerTests.RunAsync_OutputThatIsNotUtf8AndLargerThanThePipe_IsReportedPromptlyRatherThanAsATimeout
  • GitRunnerTests.RunAsync_DiscardingStandardOutput_SucceedsWhateverTheOutputHolds
  • GitRunnerTests.RunAsync_DiscardingStandardOutput_StillReportsStandardError
  • AdmissionGateTests.AdmitAsync_DiscardsTheProbesOutput

With the GitRunner/AdmissionGate changes reverted, …SucceedsWhateverTheOutputHolds and AdmitAsync_DiscardsTheProbesOutput fail. With the changes in place, the full suite passes (247/247, Linux).

🤖 Generated with Claude Code

https://claude.ai/code/session_016rdMXULeUT13t6FCwNfocx


Generated by Claude Code

…e large non-UTF-8 diff [patch]

The ls-remote admission probe only asks whether git succeeded, yet its
output was decoded as strict UTF-8, so one branch name git allows but
UTF-8 cannot read refused every caller of the repository. GitInvocation
gains DiscardStandardOutput: the output is still drained, but thrown away
undecoded, and standard error, which classifies a refusal, is kept. The
probe sets it.

The diff-tree stall itself no longer reproduces: git invocation now goes
through ktsu.RunCommand 1.9.5, which kills the command and rethrows when
a read fails (ktsu-dev/RunCommand#89). A regression test with a non-UTF-8
path ahead of 200 KB of output now guards that it stays reported as
"not valid UTF-8" rather than a timeout.

Fixes #50

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_016rdMXULeUT13t6FCwNfocx
@sonarqubecloud

sonarqubecloud Bot commented Oct 8, 2026

Copy link
Copy Markdown

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.

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"

2 participants