fix(server-http): buffer url-encoded bodies so the security filter cannot drain them #60
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
iterative-works/support!60
Loading…
Reference in a new issue
No description provided.
Delete branch "SUPP-53-main"
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?
Forward-port of the SUPP-53 fix from
release/0.1.x, where it shipped as 0.1.24(#59, tag
v0.1.24). Same pattern as #45 → #46.The defect
An http4s request body is a single-read stream. The pac4j security filter reads form
parameters to evaluate its authorizers, which consumed the body — so the route behind
the filter received an empty form and still answered 200. Nothing threw, nothing
was logged.
It looked intermittent because it depended on how the server framed the body on the
wire: a large form lost its content roughly one run in six, a small one never. That
read as a flaky test rather than as data loss, which is why it survived so long.
The fix
UrlEncodedBodyBufferreads the body once, up to a configured cap, and rebuilds therequest around the buffered bytes, so the filter and the route read the same content.
The rebuilt request carries framing headers describing what it now holds:
Content-Lengthis set to the measured size,chunkedis dropped.Only
application/x-www-form-urlencodedis buffered; every other content type passesthrough untouched.
Pac4jHttpSecurity.securewires the middleware in ahead of thesecurity filter.
Pac4jSecurityConfiggains a trailingmaxFormBodySize, read fromsecurity.maxformbodysize, default 2 MiB.Two consequences
Pac4jSecurityConfighas a new trailing field — positional destructuring orunapplyover it no longer compiles. Named construction and config-file loading areunaffected.
413, no body. htmx will not swapsuch a response, so the user sees nothing happen.
Contents
The five source files only — identical to
release/0.1.x, verified by diff. Nobuild.millchange:mainalready defines thehttp.testmodule. Theproject-management artifacts stay on the release line, as in #46.
Testing
19 regression tests (17
UrlEncodedBodyBufferSpec, 2Pac4jSecurityConfigSpec).server/httphad no HTTP-level test fixture before this; the harness (a single-readRef-backed body stream, probe routes, anAnonymousClientpac4j config) is new andreusable.
Verified on this branch, on top of
main:./mill http.test12 suites, 0 failed,exit 0;
./mill http.checkFormatclean; repo-wide__.checkFormatand__.testgreen via the pre-commit and pre-push hooks.
Follow-ups
Filed to the tracker rather than fixed here: #55, #56, #57, #58.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BDfb2ghkkKiwTKVrcufmaT