fix(server-http): buffer url-encoded bodies so the security filter cannot drain them #60

Merged
mph merged 1 commit from SUPP-53-main into main 2026-09-15 09:39:28 +00:00
Member

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

UrlEncodedBodyBuffer reads the body once, up to a configured cap, and rebuilds the
request 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-Length is set to the measured size, chunked is dropped.

Only application/x-www-form-urlencoded is buffered; every other content type passes
through untouched. Pac4jHttpSecurity.secure wires the middleware in ahead of the
security filter.

Pac4jSecurityConfig gains a trailing maxFormBodySize, read from
security.maxformbodysize, default 2 MiB.

Two consequences

  • Pac4jSecurityConfig has a new trailing field — positional destructuring or
    unapply over it no longer compiles. Named construction and config-file loading are
    unaffected.
  • A body over the cap is rejected with a bare 413, no body. htmx will not swap
    such a response, so the user sees nothing happen.

Contents

The five source files only — identical to release/0.1.x, verified by diff. No
build.mill change: main already defines the http.test module. The
project-management artifacts stay on the release line, as in #46.

Testing

19 regression tests (17 UrlEncodedBodyBufferSpec, 2 Pac4jSecurityConfigSpec).
server/http had no HTTP-level test fixture before this; the harness (a single-read
Ref-backed body stream, probe routes, an AnonymousClient pac4j config) is new and
reusable.

Verified on this branch, on top of main: ./mill http.test 12 suites, 0 failed,
exit 0; ./mill http.checkFormat clean; repo-wide __.checkFormat and __.test
green 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

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 `UrlEncodedBodyBuffer` reads the body once, up to a configured cap, and rebuilds the request 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-Length` is set to the measured size, `chunked` is dropped. Only `application/x-www-form-urlencoded` is buffered; every other content type passes through untouched. `Pac4jHttpSecurity.secure` wires the middleware in ahead of the security filter. `Pac4jSecurityConfig` gains a trailing `maxFormBodySize`, read from `security.maxformbodysize`, default 2 MiB. ## Two consequences - `Pac4jSecurityConfig` has a **new trailing field** — positional destructuring or `unapply` over it no longer compiles. Named construction and config-file loading are unaffected. - A body over the cap is rejected with a **bare `413`, no body**. htmx will not swap such a response, so the user sees nothing happen. ## Contents The five source files only — identical to `release/0.1.x`, verified by diff. No `build.mill` change: `main` already defines the `http.test` module. The project-management artifacts stay on the release line, as in #46. ## Testing 19 regression tests (17 `UrlEncodedBodyBufferSpec`, 2 `Pac4jSecurityConfigSpec`). `server/http` had no HTTP-level test fixture before this; the harness (a single-read `Ref`-backed body stream, probe routes, an `AnonymousClient` pac4j config) is new and reusable. Verified on this branch, on top of `main`: `./mill http.test` 12 suites, 0 failed, exit 0; `./mill http.checkFormat` clean; repo-wide `__.checkFormat` and `__.test` green 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.com/claude-code) https://claude.ai/code/session_01BDfb2ghkkKiwTKVrcufmaT
fix(server-http): buffer url-encoded bodies so the security filter cannot drain them
All checks were successful
CI / Compile (pull_request) Successful in 2m24s
CI / Check Formatting (pull_request) Successful in 55s
CI / Check Linting (pull_request) Successful in 2m36s
CI / Run Tests (pull_request) Successful in 3m37s
39aaa71af7
An http4s request body is a single-read stream. The pac4j security filter needs form
parameters to evaluate its authorizers, and reading them consumed the body, so the
route behind the filter received an empty form and still answered 200. Nothing threw
and nothing was logged — the data was simply gone.

The behaviour was 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.

UrlEncodedBodyBuffer reads the body once, up to a configured cap, and rebuilds the
request around the buffered bytes, so the filter and the route read the same content.
The rebuilt request carries framing headers that describe what it now holds:
Content-Length is set to the measured size and chunked is dropped. Only
application/x-www-form-urlencoded requests are buffered; every other content type
passes through untouched. Pac4jHttpSecurity.secure wires the middleware in ahead of
the security filter.

Pac4jSecurityConfig gains a trailing maxFormBodySize, read from
security.maxformbodysize with a 2 MiB default. Positional destructuring over the case
class no longer compiles; named construction and config-file loading are unchanged. A
body over the cap is rejected with a bare 413, which htmx will not swap.

19 regression tests: 17 in UrlEncodedBodyBufferSpec, 2 in Pac4jSecurityConfigSpec.
server/http had no HTTP-level test fixture before this; the harness (a single-read
Ref-backed body stream, probe routes, an AnonymousClient pac4j config) is new and
reusable.

Forward-ported from release/0.1.x, released as 0.1.24 (387783cc, 8b771a63).

Closes #53.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BDfb2ghkkKiwTKVrcufmaT
mph merged commit 9f04c3bc25 into main 2026-09-15 09:39:28 +00:00
mph deleted branch SUPP-53-main 2026-09-15 09:39:28 +00:00
Sign in to join this conversation.
No description provided.