git@cosmocr.at/gitweb.git @ HEAD · summary refs log

HEAD / docs / FR-large-push-400.md /

docs/FR-large-push-400.md · 12.4 KiB · raw

FR: pushes over ~1 MiB fail with an undiagnosable `400 bad request`

Status: fixed (2026-10-18). Root cause identified; see Resolution. Filed: 2026-09-19 Observed on: git.cosmocr.at (deployed), pushing rpg137@gmail.com/einkdashboards.git Reproduced on: git@cosmocr.at/demo.git with a 3 MiB payload (see below)

Resolution (2026-10-18)

Root cause: gitweb answered git's probe request with 400. When a push is larger than http.postBuffer, git (remote-curl.c, probe_rpc) first sends a receive-pack POST whose entire body is a flush-pkt, 0000 (Content-Length: 4). It checks the endpoint (and auth) that way before streaming the real body chunked, and it needs a 200 back. gitweb passed the probe to DecodeReceivePack, which fails on a request with no commands, and the handler returned a bare 400. git gave up before sending any of the pack.

That explains every observation below. Pack size alone triggers it, because the probe is only sent over postBuffer. Ancestry makes no difference. Raising postBuffer makes the push work, because then there is no probe. Chunked decoding tests fine, because the pack never got as far as being decoded. Expect: 100-continue (hypothesis 2) is not involved. A GIT_CURL_VERBOSE trace shows the failing request is the 4-byte POST.

Fix:

githttp.DecodeReceivePack reads the first 4 bytes and returns ErrProbe when they are a flush. Reading them can't block, because git sends the whole first pkt-line before it waits. A real push starts with a command.
The handler answers ErrProbe with 200 and an empty result. The probe runs after authentication and authorization, as before, so a non-writer is still refused at the probe.
Decode failures are now logged (gitweb: receive-pack decode …). A body over the cap gets 413 with the limit named; anything else gets 400 with the reason.

Tests:

TestCloneAndPush/large_push_over_http.postBuffer pushes 3 MiB of random data with real git at http.postBuffer=1048576 (the default) and re-clones it. It failed with HTTP 400 before the fix.
…/large_push_as_reader_denied checks that the probe is still authorized.
TestDecodeReceivePackProbe, TestDecodeReceivePackProbeGzip and TestDecodeReceivePackChunked (the regression test proposed below) cover the decode path.

The investigation notes below are kept as filed.

Symptom

Pushing a repository whose pack exceeds git's default http.postBuffer (1 MiB) fails:

$ git push --force origin master multi-dashboard
error: RPC failed; HTTP 400 curl 22 The requested URL returned error: 400
fatal: the remote end hung up unexpectedly
Everything up-to-date

git fetch against the same repo, over the same connection, with the same credentials, succeeds immediately beforehand — so this is not auth, TLS, routing, or repo existence.

The workaround is client-side:

git config http.postBuffer 524288000   # 500 MiB

After which the identical push succeeds. Every user has to discover and apply this per-repo, and nothing in the server's response hints at it.

What is actually wrong

The primary defect is that the failure is unattributable. In internal/server/githttp.go:140-143:

case gitReceivePack:
	req, err := githttp.DecodeReceivePack(r)
	if err != nil {
		http.Error(w, "400 bad request", http.StatusBadRequest)
		return
	}

Every possible decode failure — malformed pkt-line, truncated body, a pack go-git cannot resolve, a body over maxBody, a read error mid-stream — collapses into the same opaque string, and err is discarded without being logged. Every other branch in this handler logs (log.Printf("gitweb: upload-pack %s: %v", ...)); this one does not.

Consequence: the server operator cannot tell a client-side protocol error from a server-side bug, and the client is told nothing actionable. Diagnosing the incident above took roughly an hour and still did not reach a root cause.

What has been ruled out

Investigated and eliminated, so nobody repeats the work:

Hypothesis	Verdict
nginx rejecting the body	No. nginx does SNI passthrough (`docs/ARCHITECTURE.md:53`); it routes TCP and never parses HTTP, so `client_max_body_size` cannot apply.
`maxBody` cap in `requestBody`	No. 256 MiB (`internal/githttp/githttp.go:34`); the failing pack was ~5.7 MiB.
`MaxRepoBytes`	No. 256 MiB default (`main.go:57`), and it is enforced after decode.
Server read timeout cutting off a slow upload	No. Only `ReadHeaderTimeout` is set (`main.go:112`); there is no `ReadTimeout`.
Chunked transfer-encoding decoding	No — tested. `DecodeReceivePack` handles a chunked body correctly, at both minimal size and 6 MiB. See the test below.

That last row matters: the obvious theory ("gitweb can't parse chunked requests") is false, and an FR that asserted it would have sent the fix in the wrong direction. The chunked path decodes; something else about a real large push does not.

Controlled reproduction (2026-09-19)

Reproduced on git@cosmocr.at/demo.git — a 688 KiB repo unrelated to the original incident — using two branches identical except for ancestry, each carrying a 3 MiB incompressible payload (random data; zeros would deflate away and never exceed postBuffer on the wire).

Branch	Common ancestor with `main`	Pack	`http.postBuffer`	Result
`size-only`	yes	3.00 MiB	default (1 MiB)	HTTP 400
`no-ancestor` (orphan)	no	3.00 MiB	default (1 MiB)	HTTP 400
`size-only`	yes	3.00 MiB	500 MiB	succeeded

Client error is byte-identical to the original incident:

error: RPC failed; HTTP 400 curl 22 The requested URL returned error: 400
fatal: the remote end hung up unexpectedly

Conclusion: pack size alone is sufficient. The size-only branch shares an ancestor with main, so no external delta bases and no thin-pack resolution are involved, and it fails anyway. Ancestry makes no difference — both shapes fail identically, and the same objects push cleanly once the client stops using chunked encoding.

This demotes the thin-pack hypothesis below. The trigger is a pack exceeding http.postBuffer, and the fix belongs in request handling rather than the pack resolution path.

Also observed in the same session: a force-push rewinding main succeeded at the default postBuffer (it sends a ref update and no new objects). Ref update handling is therefore not implicated; only the large-pack path fails.

Remaining hypotheses

In rough order of likelihood, after the reproduction above:

~~Thin packs.~~ Ruled out by the controlled reproduction — a push sharing an ancestor, with no external delta bases, fails identically.
Expect: 100-continue handling. Now the leading suspect. curl adds the header for large streamed bodies; Go sends 100 Continue only when the handler first reads the body, so any path that writes a response before reading would strand the client. This fits the evidence: it is specific to the large/streamed request path and independent of pack contents.
Deployed binary drift. HEAD here is 5444040 (2026-07-12) on branch sso-browser-login. Whether the running service matches this source is unconfirmed — the bug may already be fixed in tree. Worth eliminating cheaply before any code change, since the unit tests in this repo show the decode path handling chunked bodies correctly.

Proposed change

1. Log the error (do this first, independently of any fix)

req, err := githttp.DecodeReceivePack(r)
if err != nil {
	log.Printf("gitweb: receive-pack decode %s: %v", repoPath, err)
	http.Error(w, "400 bad request", http.StatusBadRequest)
	return
}

One line. It converts this class of incident from guesswork into a log grep, and it is what tells us which of the hypotheses above is real.

2. Return a useful body to the client

git surfaces the response body on remote: lines. Distinguish the cases that have distinct remedies — at minimum, separate "body too large" (413, with the limit named) from "malformed request" (400). A client seeing remote: pack exceeds 256 MiB limit needs no server log to self-serve.

3. Fix the underlying failure

Only once (1) identifies it. If it is thin packs, the fix belongs in the pack resolution path, not in request handling — and no amount of postBuffer tuning is the right answer for users.

How to test

Regression test for the chunked path (passes today — keep it that way)

Add to internal/githttp/githttp_test.go. This is the test that disproved the original theory; it is worth keeping so the chunked path does not silently regress:

// TestDecodeReceivePackChunked drives a chunked body through a real
// http.Server, which is what git sends once a request exceeds
// http.postBuffer (default 1 MiB).
func TestDecodeReceivePackChunked(t *testing.T) {
	const (
		oldOID = "0000000000000000000000000000000000000000"
		newOID = "1111111111111111111111111111111111111111"
	)
	cmd := fmt.Sprintf("%s %s refs/heads/master\x00report-status\n", oldOID, newOID)
	var body bytes.Buffer
	fmt.Fprintf(&body, "%04x%s", len(cmd)+4, cmd)
	body.WriteString("0000")
	body.WriteString("PACK")
	body.Write([]byte{0, 0, 0, 2, 0, 0, 0, 0})
	body.Write(bytes.Repeat([]byte{0xAB}, 6<<20)) // past postBuffer
	body.Write(make([]byte, 20))

	errCh := make(chan error, 1)
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if r.ContentLength != -1 {
			t.Errorf("ContentLength = %d, want -1 (chunked)", r.ContentLength)
		}
		req, err := DecodeReceivePack(r)
		if err == nil && req.Packfile != nil {
			_, err = io.Copy(io.Discard, req.Packfile)
		}
		errCh <- err
	}))
	defer srv.Close()

	hreq, _ := http.NewRequest(http.MethodPost, srv.URL+"/git-receive-pack",
		io.NopCloser(bytes.NewReader(body.Bytes())))
	hreq.ContentLength = -1 // force chunked
	resp, err := http.DefaultClient.Do(hreq)
	if err != nil {
		t.Fatalf("request failed: %v", err)
	}
	defer resp.Body.Close()
	if err := <-errCh; err != nil {
		t.Fatalf("chunked body rejected: %v", err)
	}
}

Run: go test ./internal/githttp/ -run Chunked -v

End-to-end reproduction (the one that actually fails)

A unit test cannot produce a real git pack. This recipe reproduces it on the demo repo in under a minute; it is the one used for the results above.

git clone https://git.cosmocr.at/git@cosmocr.at/demo.git /tmp/repro
cd /tmp/repro

# Push to a scratch ref with an EXPLICIT refspec. This clone ships
# push.default=upstream, so a bare `git push origin <branch>` targets the
# branch's upstream (main) rather than creating a same-named branch —
# that mistake polluted main once already.
git checkout -b fr-repro

# Incompressible payload: zeros would deflate away and never exceed
# postBuffer on the wire.
mkdir -p fr-repro && head -c 3145728 /dev/urandom > fr-repro/payload-3mb.bin
git add fr-repro && git commit -qm "FR-large-push-400 repro payload"

git config --unset http.postBuffer 2>/dev/null   # ensure the 1 MiB default

# Expected: HTTP 400.
GIT_CURL_VERBOSE=1 git push origin fr-repro:refs/heads/fr-repro 2>&1 | tee /tmp/push.log

# With the logging change in place, the server log now names the cause:
sudo docker logs --tail 50 patrick_gitweb_1 | grep 'receive-pack decode'

# Control: identical objects, non-chunked request. Expected: success.
git -c http.postBuffer=524288000 push origin fr-repro:refs/heads/fr-repro

# Clean up the scratch ref.
git push origin :refs/heads/fr-repro

To also confirm ancestry is irrelevant, repeat with git checkout --orphan instead of git checkout -b; both shapes fail identically.

GIT_CURL_VERBOSE=1 shows whether the request went out chunked and whether Expect: 100-continue was used — which settles hypothesis 2 directly.

Acceptance criteria

A failing push writes a specific, actionable reason to the server log.
The client receives a response body distinguishing "too large" from "malformed".
A >1 MiB push succeeds at the default http.postBuffer, with no client-side configuration, for both normal and orphan history.
go test ./... green, chunked regression test included.

Impact

Low frequency, high friction. It only bites repos over ~1 MiB, but when it does the failure is opaque, the fix is non-obvious, and the workaround has to be repeated per-repo per-clone. Of the repos currently hosted, einkdashboards (5.7 MiB) trips it; mtadash (328 KiB) does not, which is why this went unnoticed until now.

FR: pushes over ~1 MiB fail with an undiagnosable 400 bad request