Geo: Fix SSH proxy hang by closing pull request body after negotiation
Problem
Two separate bugs on the Geo SSH proxy pull path (internal/command/githttp/pull.go).
1. The proxied request body never closes, so slow transfers die. PullCommand.requestSSHUploadPack streamed the client's raw SSH stdin straight into the HTTP request body — a pattern copied from the push path, where it's correct. For pull, the request body carries small negotiation data, and the pack flows back on the response side. The body then sits idle on the primary for the whole transfer and eventually trips nginx's client_body_timeout. Raising that timeout only moves the failure point.
This causes The new Geo SSH proxy implementation hangs inde... (gitlab#584782 - closed).
2. Closing the body right after done breaks protocol v2 outright. Real git clients send a flush-pkt (0000) immediately after done. Under v0, done itself terminates negotiation, so stopping there is harmless. Under v2, done is only an argument line inside the fetch request section, and that section must be closed with a flush-pkt. Without it git upload-pack hits EOF and dies with fatal: the remote end hung up unexpectedly; Gitaly maps that stderr to a Canceled gRPC error; and the client sees every clone through the proxy fail with fatal: protocol error: bad line length character: fata.
Fix
readFromStdin now closes the request body once it has forwarded done, and injects a flush-pkt of its own before closing. Injecting rather than waiting to read the client's own flush is deliberate: under v0/v1 no flush ever follows done, so waiting would hang forever, while the extra flush is harmless there because upload-pack never reads past done.
This MR also extracts pipeUploadPack so requestUploadPack and requestSSHUploadPack share the pipe-and-copy logic, per Duo's review comment.
Why closing on done is sufficient
Only clients that send done hold the request body open. Measured with an instrumented SSH transport (git 2.53, 40 MB pack, throttled response):
| Negotiation shape | Client closes its write side | Pack ends |
|---|---|---|
v0 clone (sends done) |
15.545s | 15.560s |
v2 fetch (server sends ready, no done) |
0.082s | 15.502s |
Under v2, git half-closes as soon as it sees ready, so the request body reaches EOF on its own. Under v0 it stays open for the entire transfer — and v0 always sends done, which is what this fix closes on. Reproduced on a second, independent rig (close at 22.9ms, pack end at 27.0s).
A non-git v2 SSH client that never half-closes would still hold the body open; no such client is known.
Testing
| Setup | Protocol | Result |
|---|---|---|
| 3k Cloud Native Hybrid (Envoy Gateway ingress, Praefect-fronted Gitaly cluster) | v2 | Clone passed — failed before commit 8556cbd2 |
| 3k Cloud Native Hybrid (Envoy Gateway ingress, Praefect-fronted Gitaly cluster) | v0 | Passed — regression check for the injected flush |
| 1k Omnibus (nginx, single Gitaly) | v0 | Passed, including git ls-remote, multi-round have negotiation, --depth 1, and git push |
1k Omnibus (nginx, single Gitaly), AcceptEnv GIT_PROTOCOL on the secondary sshd |
v2 | Clone passed |
| Unit test: v2 fetch fixture | v2 | Forwarded body matches what git sends, byte for byte |
Standalone git upload-pack, with and without the trailing flush |
v2 | Confirms the mechanism behind bug 2: dies with fatal: the remote end hung up unexpectedly without the flush, succeeds with it |
1k Omnibus (nginx, single Gitaly), AcceptEnv GIT_PROTOCOL on the secondary sshd |
v2 | With aincremental fetch (no done) and client_body_timeout at 5s, unpatched 190MB binary → passed |
Slow-transfer verification for bug 1: a 700 MB clone through a throttled client with client_body_timeout forced to 5s failed at ~9s unpatched (unexpected EOF, workhorse logging context canceled at duration_ms: 5555) and completed in 53.4s patched.