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 sameRequestBodyTooLargeErrorvalue for one byte over.TestIntegration: exactly the limit and one byte over, each withContent-Lengthand 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
- In
workhorse/, rungo test ./internal/bodylimit/ ./internal/upstream/ -run 'TestCountingReadCloser|TestIntegration$|TestTerraformStateLockUnlockBodyLimit' -count=1 -v. Everything passes. - Run
git checkout origin/master -- internal/bodylimit/bodylimit.goand the same command again. The exact-limit cases fail:TestIntegrationand both route subtests withexpected: 200,actual: 413,TestCountingReadCloserandTestCountingReadCloser_ReadsAgainAfterLimitwith the unexpectedRequestBodyTooLargeError, andTestCountingReadCloser_ReadErrorAtLimitbecause it getsRequestBodyTooLargeErrorinstead of the read error. The one-byte-over cases still pass, except the one inTestCountingReadCloser_ReadsAgainAfterLimit, which reportsNot samebecausemasterbuilds a new error on every read. git checkout HEAD -- internal/bodylimit/bodylimit.gorestores 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 verifyon Go 1.26.9 (theworkhorse:verifyjob): lint (golangci-lint 2.11.4 against the empty baseline), vet,detect-context,detect-external-tests,check-formatting,staticcheck2026.1, andgo mod tidyleavinggo.modandgo.sumunchanged.make test-raceon Go 1.26.9, andmake teston 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-Lengthlowered to match; the tests as first published pass against both. - The commit messages pass
scripts/lint/commit_linter.rbwithgitlab-dangerfiles4.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.