Skip to content

Bound Git requests by their body and response progress - #580

Merged
andrew merged 2 commits into
git-pkgs:mainfrom
simonchrz:fix/git-request-limits
Oct 25, 2026
Merged

andrew merged 2 commits into
git-pkgs:mainfrom
simonchrz:fix/git-request-limits

Conversation

@simonchrz

Copy link
Copy Markdown
Contributor

Hardens how long a Git request can hold an upload-pack process.

  • The handler extended the read deadline to two hours before the body was read, and upload-pack ran with its stdin attached to the rest of the body. Stateless requests are complete once sent, so the body is now read in full first, within the server's read timeout, and only then are the deadlines extended for the response. Bodies past 1 MiB go to a temporary file rather than memory; the 100 MB limit is unchanged. A body that doesn't arrive in time gets a 400, and one past the limit now gets a 413 instead of a cut-off upload-pack.
  • ls-refs and info/refs took no slot. They now share their own (four per CPU), so they still don't wait behind clones.
  • Each response write is bounded to two minutes. A client that stops reading a pack frees its slot instead of holding it until the two-hour request timeout. Time between writes isn't bounded, so a cold clone or a slow pack-objects before the first byte is unaffected.
  • Full duplex is no longer needed, since the body is read before the response starts.

Lease.ServeUploadPack now takes the body from the new Cache.ReadRequest.

Tests: TestGitSlowRequestBodyStartsNothing (part of a body, then nothing: answered at the read timeout), TestGitStalledClientIsDropped (16 MB pack never read: the handler returns), TestRefListingSlots, TestReadRequest. The first three fail without the change.

@andrew

andrew commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

This conflicts with main in internal/gitcache/cache.go since the flaky test fixes merged. Could you rebase?

upload-pack was started with its stdin attached to the request body, after
the read deadline had been extended to two hours, and ls-refs requests took
no slot. A client that sent part of a request and then nothing kept an
upload-pack process waiting, and any number of them could.

Stateless requests are complete once sent, so the handler now reads the
body in full first, within the server's read timeout, keeping bodies past
1 MiB in a temporary file. Only then are the deadlines extended for the
response. info/refs and ls-refs share their own slots, four per CPU, so they
still never wait behind clones. Each response write is bounded to two
minutes, so a client that stops reading a pack frees its slot instead of
holding it until the request timeout.
@simonchrz
simonchrz force-pushed the fix/git-request-limits branch from 813260d to 77bd66a Compare October 24, 2026 11:03
@simonchrz

Copy link
Copy Markdown
Contributor Author

Rebased onto main, CI is green.

@andrew
andrew merged commit bd9f376 into git-pkgs:main Oct 25, 2026
7 checks passed
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.

2 participants