The hop cap bounds the walk but not the Split: a 1 MB X-Forwarded-For still allocates ~8 MB per request #133
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Residual disclosed by the independent review of #129, which correctly treated it as outside that PR's definition of done. Milestoned 1.0.0 because the endpoint is the unauthenticated receiver and the amplification is in memory, not just CPU.
#124 capped the hop WALK at 64 entries, but the truncation happens after
strings.Split. So a ~1 MBX-Forwarded-Forof"1,"entries still builds a ~500k-element[]string— roughly 8 MB — on every request, before anything is capped.Measured in that review: ~5 ms/req remaining versus ~38 ms/req uncapped, so the hop cap removed about 87% of the CPU burn. The allocation is the part it did not remove, and the allocation is the bigger problem: bounded only by
MaxHeaderBytes, it is ~8 MB per concurrent request. A few hundred concurrent requests is enough to matter on a small host, and the receiver requires no authentication.Definition of done
strings.LastIndexByte) and stop after at most 64 entries, so allocation is bounded by the cap rather than by header length.testing.AllocsPerRunor a bounded-allocation assertion, not a timing assertion.Implementation requirements
internal/middleware/ratelimit.gohas now passed two adversarial reviews; do not restructure it.next, PR based onnext, single commit, title ending(closes #N).TODO.md(see #112).make checkplus the Docker lint path with the cache defeated (#119).PR: #136
forwardedClientAddrnow cuts entries off the right end of eachX-Forwarded-Forvalue withstrings.LastIndexByteand walks multiple values in reverse, instead of joining and splitting. The cap, the skip rules and the fail-closed fallbacks are unchanged, so no existing rate-limit assertion moved.Verified: new test measures
TotalAllocover 50 keyings of a 1,000,007-byte chain — 1,606,043 bytes/call before, 16 bytes/call after, asserted < 4096. Reverting the implementation tostrings.Splitfails that test.make checkexits 0, anddocker build --no-cache-filter=lint,builderexits 0 with both stages executed (0 issues.from the lint stage, middleware tests passing in the builder stage).