fix(git): allow large smart HTTP pushes (#2880) - #4401
Draft
MajorTal wants to merge 5 commits into
Draft
Conversation
Signed-off-by: Tal Weiss <major.tal@gmail.com>
This was referenced Aug 2, 2026
added 4 commits
August 2, 2026 20:22
Git sends the same unauthenticated four-byte flush-packet probe before any smart HTTP POST whose body exceeds http.postBuffer — not just large pushes. Fetch negotiations cross the 1 MiB decoded threshold when the client's divergent local history contributes enough have lines (an in-limit repo plus ~21k unpushed divergent branches suffices; measured live at MAX_MANIFEST_REFS = 10_000 with divergent client history: 1.46 MiB decoded negotiation, probe fired, fetch got 401). Parameterize the receive-pack probe middleware over the service's (request MIME, result MIME) pair — the predicate was already otherwise service-agnostic — and mount it on the upload-pack POST route as well. All other predicate legs are unchanged: no Authorization / Content-Encoding / Transfer-Encoding headers, a single exact Content-Length of 4, and an exact 0000 body; near misses fall through to the normal authenticated path. Tests: the existing exactness/path-independence matrix now runs against both services (2 path shapes + 11 near-misses each), plus a cross-service case proving each route rejects the other service's request MIME. Fixes block#4423 Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
The relay previously bounded request *bytes* (RequestBodyLimitLayer) but never request *time*: a client that sent valid headers and then withheld the body parked a relay task indefinitely (measured live with 200 parked sockets). Three narrow deadlines close the measured seam: - API router: 60s tower_http TimeoutLayer (408 on expiry via with_status_code — tower-http's layer returns the configured status directly rather than surfacing Elapsed), ordered outside the body limit so a stalled body is cancelled by the deadline instead of sitting inside body-limit middleware. WebSocket routes are handshake bounded only; the established session escapes the response future after the 101 upgrade (proven by test). - Media router: 300s with the same ordering and semantics — bounds the future until response headers, not streaming response bodies. - Git compatibility probe: 60s bound on the pre-auth four-byte body collection, failing closed into the normal authentication path. No outer deadline is added around authenticated Git/CAS execution: PACK_OPS_TIMEOUT (300s) is not a total request budget — finalize_push and CAS publication carry additional deadlines, and an outer layer could cancel a valid push during publication. Git-wide budgeting and hyper header-read deadlines stay open on block#4424. Scope: core unauthenticated request-body surfaces plus the Git probe. The admin router, git policy router, SPA fallback, and health listener are unchanged. Tests inject millisecond deadlines (no production-constant sleeps): stalled body gets an empty 408 within the bound and never completes the handler; a dropped stalled future never invokes the handler (probe and router variants); an established WebSocket session survives the request deadline; probe near-misses still reach the normal path immediately. Addresses the minimal core of block#4424 Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
…timeout The 300s media TimeoutLayer bounded the whole request future, so any legitimate slow-but-progressing upload needing longer than 300s (a 500 MiB body below ~14 Mbit/s sustained) was cut off with a terminal 408 — the CLI does not retry 408. Found by Sami (RESEARCH/PR4401_MEDIA_DEADLINE_SLOW_UPLOAD.md). Uploads and reads are two timeout semantics, so the media router is split by route class and merged: - Upload routes get a RequestBodyTimeoutLayer(60s): the deadline resets on every body frame, so a progressing upload of any duration completes while a withheld body (the parked-task attack, block#4424) still fails closed — plus a generous 3600s wall-clock ceiling (matching the Blossom auth window) backstopping the post-body phase. The ceiling's trickle-permit cost is measured and documented on the constant. - Read routes (GET/HEAD /media/{sha256_ext}) carry no request body, so they keep a tight 60s wall-clock deadline — without it a hung storage read parks a task forever (measured by Sami: idle-only un-bounds every non-body stall). TimeoutLayer only covers until response headers, so streaming blob downloads are not truncated. The idle timeout surfaces as a body read error, not a synthesized response, so buzz-media gains classify_body_error: a typed check for tower_http::timeout::TimeoutError first, then the length-limit Display patterns. All three body-consumption paths in upload_blob map IdleTimeout -> new MediaError::RequestBodyTimeout (408), LengthLimit -> FileTooLarge (413), Other -> Io (500) — previously an idle trip surfaced as 500 pre-sniff (paging as a storage fault) or 413 on the non-video collect path. Verified live per TESTING.md: a paced 512 B/s upload (387s wall, exceeding the old 300s bound) completes and round-trips byte-exact; withheld upload body answers 408 at exactly 60s with the idle-deadline message; API withheld body still answers 408 at 60s; git push/clone/ ff-push/pull smoke passes at this tree. Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
…d bound Four review-gate items from PR block#4401 round 3 (Wren's binding hold at 0845a09, amended by Sami's alias and read-bound findings): 1. New buzz-media regression test: a real tower_http TimeoutError, wrapped the way axum wraps body errors, driven through process_video_upload's StreamReader read loop must surface as MediaError::RequestBodyTimeout / 408 — never Io / 500. Honest scope per Sami's mutation pass (M3): this pins the conversion chain, not the sniff/routing decision, which lives upstream in upload_blob and is stated as uncovered rather than implied-covered. Mutating either conversion arm (the read-loop TimedOut arm or the stream map's IdleTimeout arm) fails the test. 2. MEDIA_READ_TIMEOUT restored 300s: commit 4 silently tightened media reads 300s -> 60s while relaxing uploads (found by Sami). 300s preserves the bound reads already had and covers the multi-call pre-header path: the read handler awaits several sequential storage calls, each independently allowed up to 60s by rust-s3's per-call default, so a 60s request deadline could cancel a sequence whose individual calls are all within their own budgets. 3. Route-precedence regression test for the legacy /media/upload alias: the literal lives in the upload sub-router while /media/{sha256_ext} lives in the read sub-router, merged. If axum ever resolved the literal under the param capture, the alias would inherit the tight read wall-clock and the commit-3 regression would survive on exactly one route. A slow-but-progressing PUT /media/upload must complete under discriminating bounds (read deadline tighter than the upload duration), and GET /media/upload must answer 405 from the literal, not 408 from the param route's deadline. 4. Comment wording: "any duration"-shaped claims on the idle bound now qualify completion by the 3600s MEDIA_UPLOAD_CEILING instead of contradicting it. No production code changes beyond the MEDIA_READ_TIMEOUT constant and doc comments. Additive on 0845a09, no rebase. Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Yo good people, Tal here, human. This (is another) bug that bugged me while dogfooding Buzz, so I went down the Git smart-HTTP rabbit hole and tried to leave behind a very small fix with a very real test. Hope it is in good order :)
What changed
git-receive-packcompatibility probe.application/x-git-receive-pack-resultresponse without resolving a repository, hydrating Git state, touching storage, or mutating anything.http.postBuffer, verifies the four-byte probe and authenticated chunked request, and clones the bytes back exactly.Root cause
For packs above
http.postBuffer, Git first sends an unauthenticatedPOST /git-receive-packcontaining only the flush packet0000. The relay challenged that probe with401, so Git aborted before sending the real authenticated chunked pack.The shim is deliberately narrow and path-independent. It matches only:
POST .../git-receive-packAuthorization,Content-Encoding, orTransfer-EncodingContent-Type: application/x-git-receive-pack-requestContent-Length: 40000Validation
Content-Length: 4followed byTransfer-Encoding: chunked.401.cargo clippy -p buzz-relay -p buzz-test-client --all-targets -- -D warningscargo fmt --all -- --checkgit diff --check504; its active fix is fix(mesh): frame loss when recv_validated is cancelled mid-validation (#2458) #2745.Fixes #2880