Accept a request body of exactly the enforced limit in Workhorse

What does this MR do and why?

When a Workhorse route enforces a request body limit, a body of exactly the limit size is answered with 413 Request Entity Too Large, although it is within the limit. The routes that enforce a limit on their own are the Terraform state lock and unlock routes (4 KB) and remote_mirrors (100 KB). With WORKHORSE_REQUEST_LIMIT=ENFORCED, the requests proxied on every other route are held to their route's limit too, 100 MB by default.

countingReadCloser.Read returns RequestBodyTooLargeError as soon as the byte count has reached the limit, without checking whether the body has ended. Go's HTTP transport reads the body again at that point when it forwards the request: it copies a Content-Length body through io.LimitReader and then reads once more to detect extra data, and it copies a chunked body until EOF (transferWriter.writeBody). So a Content-Length body of exactly the limit is refused every time, and a chunked one whenever its terminating chunk has not been buffered with the data.

With this change, once the limit is reached the reader reads one more byte from the underlying body. If that read returns no data, its result is returned as it is: the end of the body, so a body of exactly the limit goes through, or a read error. If it returns a byte, the body is over the limit and the request gets 413 as before; the reader keeps that error and returns the same value on every later read, where master built an equal one on each read at the limit. The byte count does not change.

Behavior change: only for a body that reaches the limit exactly, in enforced mode. It is now accepted, and if reading the body fails right at that point, the read error is returned instead of RequestBodyTooLargeError, as it is at any other point of the body. Bodies under or over the limit, and the logging mode, behave as before.

Tests:

  • TestCountingReadCloser: a body of exactly the limit read to the end, and one byte over the limit read past it.
  • TestCountingReadCloser_ReadErrorAtLimit: a read error right after the limit is returned as it is, whether the limit is reached by the first read or by a later one.
  • TestCountingReadCloser_ReadsAgainAfterLimit: reading again after the limit keeps returning EOF for a body of exactly the limit, and the same RequestBodyTooLargeError value for one byte over.
  • TestIntegration: exactly the limit and one byte over, each with Content-Length and chunked.
  • TestTerraformStateLockUnlockBodyLimit: a body of exactly 4 KB through the router, on the lock and the unlock route, and the backend receives all of it.

References

Closes #633771 (closed)

The body limit and its per-route mode were added in !197116 (merged) and !197656 (merged).

Screenshots or screen recordings

Not applicable: no UI change.

How to set up and validate locally

  1. In workhorse/, run go test ./internal/bodylimit/ ./internal/upstream/ -run 'TestCountingReadCloser|TestIntegration$|TestTerraformStateLockUnlockBodyLimit' -count=1 -v. Everything passes.
  2. Run git checkout origin/master -- internal/bodylimit/bodylimit.go and the same command again. The exact-limit cases fail: TestIntegration and both route subtests with expected: 200, actual: 413, TestCountingReadCloser and TestCountingReadCloser_ReadsAgainAfterLimit with the unexpected RequestBodyTooLargeError, and TestCountingReadCloser_ReadErrorAtLimit because it gets RequestBodyTooLargeError instead of the read error. The one-byte-over cases still pass, except the one in TestCountingReadCloser_ReadsAgainAfterLimit, which reports Not same because master builds a new error on every read.
  3. git checkout HEAD -- internal/bodylimit/bodylimit.go restores the branch's file.
What I ran (base f6995bef)

The commands of the Workhorse CI jobs on the Go versions they use, in golang images with exiftool added, with config.toml built from config.toml.example with Redis at redis://redis:6379, and a Redis 7.2 service beside it:

  • make verify on Go 1.26.9 (the workhorse:verify job): lint (golangci-lint 2.11.4 against the empty baseline), vet, detect-context, detect-external-tests, check-formatting, staticcheck 2026.1, and go mod tidy leaving go.mod and go.sum unchanged.
  • make test-race on Go 1.26.9, and make test on Go 1.25.14 and Go 1.27.2: every package passes.
  • The new tests with -race -count=50: they pass.
  • Four mutants of the fix (the probe condition inverted, the over-limit error replaced by a plain return, a read error at the limit reported as the end of the body, the kept error not returned on a later read) each make the new tests fail.
  • The tests added after review fail against a reader that returns a fresh copy of the error on each later read and against a forwarded body cut by one byte with its Content-Length lowered to match; the tests as first published pass against both.
  • The commit messages pass scripts/lint/commit_linter.rb with gitlab-dangerfiles 4.13.0.

The route test covers Content-Length only, which fails on master every time. Through the router, a chunked body is refused on master only when its terminating chunk arrives after the data has been read: in my runs, 20 of 20 when the terminating chunk came 100 ms later, and occasionally (7 of 200 runs) when it followed the data straight away. A route test for that case would depend on timing, so the chunked case is covered by TestIntegration, where it fails on master every time.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by José M. Requena Plens

Merge request reports

Loading
Loading