Allow reading the request body in both security and main server logic

[magdzikk]

Fixes #4442.

The problem

An endpoint that reads the request body in serverSecurityLogic and again in the main logic crashes on streaming backends:

val secureBase = endpoint
  .securityIn(stringBody)                    // read #1
  .serverSecurityLogic { body => ... }

secureBase.in(jsonBody[Person])              // read #2
  .serverLogic(user => person => ...)

java.lang.IllegalStateException: This publisher only supports one subscriber
  at org.playframework.netty.HandlerPublisher.subscribe(HandlerPublisher.java:167)

The failure is backend-dependent: vertx and armeria materialise the body eagerly and happen to work, so today the DSL permits something whose success is accidental per interpreter.

The approach

This PR adds an extractBodyFromRequest wrapper - it allows reading the body second time, but is ignored during client generation:

val secureBase = endpoint
  .securityIn(auth.bearer[String]())
  .securityIn(extractBodyFromRequest(stringBody))   // server-side only
  .serverSecurityLogic { case (token, raw) => verifyHmac(token, raw) }

secureBase.post
  .in("people")
  .in(jsonBody[Person])                             // the documented body
  .serverLogic(user => person => ...)

We only support byte-like types: String, Array[Byte], ByteBuffer, InputStream, InputStreamRange. Other types are rejected at compile time (File, multipart, streaming and oneOfBody).

⚠️ Breaking change

An endpoint declaring two ordinary bodies now throws IllegalArgumentException at route construction:

endpoint.securityIn(stringBody).in(stringBody)   // now rejected at startup

This previously "worked" on eagerly-buffering backends (vertx, armeria, play) and failed confusingly on streaming ones. No endpoint in tapir itself does this, but user code may. Migration is one line: wrap either declaration in extractBodyFromRequest.

[adamw]

I understand that the extractBodyFromRequest(stringBody) syntax mirrors extractFromRequest, but I'm wondering if there it wouldn't be more readable if this used the usual attribute-adding syntax using an extension method, which could explicitly say that this is a second body definition. E.g. stringBody.extractAsSecondary or stringBody.secondaryForServer or sth like this

[magdzikk]

I agree. What do you think about stringBody.asSecondary? I now pushed such rename (and b.isSecondary instead of b.isExtracted), but let me know if you think it's too short and we can try to find something more descriptive.

[adamw]

both SecondaryBody & ReplayableRawBody aren't really directly used by users. So maybe they don't have to be in the main namespace, which is auto-imported by import sttp.tapir.*. Instead, maybe we could put them e.g. inside EndpointIO.Body companion object? Or maybe there's a better place existing in the hierarchy?

[adamw]

so do we need the @volatile? even if F-s operations cross threads, there will be memory barriers between thread switches, relating to operations on instances of this class?

[adamw]

these tests look very similar to the ones in ServerSecurityTests. Aren't the ones over there - which run with real interpreters - sufficient?

[adamw]

🤖 Claude-generated review. Findings below were verified against the code; I've left out anything I couldn't confirm.

The mechanism itself holds up: the cache is per-request and can't be mutated by callers, decode ordering is right in both directions, both phases share the same maxBytes (ServerEndpoint.info delegates to endpoint.info), decodeStreamingBody correctly stays on the undecorated body, and every server and client interpreter is covered.

Three things worth deciding before merge:

1. CachingRequestBody ignores the request's Content-Type charset, which changes how the primary body decodes on http4s, play, akka, pekko and finatra.
2. Armeria runs the verification per request rather than at route construction.
3. The hidden-schema pattern from #3820 now throws at startup — it's the existing workaround for this exact problem, so it needs either support or a release note.

[adamw]

🤖 Claude-generated review

The charset here is the codec's default. Several backends instead prefer the charset from the request's Content-Type and fall back to the codec default: http4s Http4sRequestBody.scala:42, play/play29 PlayRequestBody.scala:65, finatra FinatraRequestBody.scala:44, akka/pekko via FromEntityUnmarshaller[String].

The wrapper is installed for the whole endpoint (ServerInterpreter.scala:116), so this changes how the primary body decodes too. With securityIn(stringBody.asSecondary).in(stringBody), a request with Content-Type: text/plain; charset=ISO-8859-1 and body café decodes correctly today and as mojibake after. Adding a secondary body silently breaks an unrelated one.

Deriving it the way the backends do:

case RawBodyType.StringBody(defaultCharset) =>
  val cs = serverRequest.contentTypeParsed.flatMap(_.charset).map(Charset.forName).getOrElse(defaultCharset)
  bytes(serverRequest, maxBytes).map(bs => RawValue(new String(bs, cs)))

No test covers a non-default request charset.

[adamw]

🤖 Claude-generated review

serve runs per request, so the whole verification now runs on every request: two asVectorOfBasicInputs() traversals per endpoint, plus endpoint.showShort, which EndpointBodyVerifier.scala:56 builds eagerly even when there are no problems. This line used to be a cheap FilterServerEndpoints wrap.

It also changes the failure mode: an invalid endpoint set throws per request (a 500 each time) rather than at startup, which contradicts security.md ("rejected ... when routes are constructed") and the InvalidBodyDefinitionError scaladoc.

serverEndpoints is a constructor field, so a class-level private val fixes it. Same in TapirCatsService.scala:44 and TapirZioService.scala:39, and the same shape (test-only, lower impact) in stub/StubServerInterpreter.scala:28 and stub4/StubServerInterpreter.scala:29.

Making shown a lazy val is worth doing regardless.

[adamw]

🤖 Claude-generated review

forIO is reached from forOutput (line 68) as well as forInput, and asSecondary is defined on EndpointIO.Body — so out(jsonBody[Person].asSecondary) compiles. The schema is then never registered, but EndpointToOperationResponse.scala:132 still emits the media type, so ToSchemaReference falls into its "assuming external reference" branch and the spec gets a dangling $ref.

EndpointBodyVerifier only inspects securityInput/input, so nothing catches it. Either skip secondary bodies in EndpointToOperationResponse too, or have the verifier reject a secondary body found in output/errorOutput.

Unrelated, same file: the import on line 6 was widened from internal.IterableToListMap to internal._, but this case uses b.isSecondary, which is a member of Body — the widening isn't needed.

[adamw]

🤖 Claude-generated review

SecondaryBody and attributeKey are public, and so is Body.attribute — so fileBody.attribute(SecondaryBody.attributeKey, SecondaryBody()) compiles and produces exactly the state ReplayableRawBody exists to prevent. asSecondary/isSecondary are the whole intended surface; both can be private[tapir]. The key is identified by its string, so lookup is unaffected.

[adamw]

🤖 Claude-generated review

The module is right — testing depends on core only — but the package makes this permanently MiMa-tracked: enableMimaSettings runs on core and excludes only sttp.tapir.internal.*, generic.internal.* and typelevel.internal.* (build.sbt:114-118), and private[tapir] is public in bytecode. Any later change to EndpointBodyProblems, or a rename of verifyOne, will need a new exclusion.

sttp.tapir.internal gets the exclusion for free, and is where the PR already put isSecondaryBodyInput.

Two smaller things: throwOnErrors (line 24) is private[tapir] inside an already-private[tapir] object, and all six call sites spell out throwOnErrors(verify(...)) / throwOnErrors(verifyOne(...)) — one verifyOrThrow with an overload would read better.

[adamw]

🤖 Claude-generated review

Some trimming:

• line 11 duplicates line 7 — same attribute(...) call, different body constructor, no distinct code path.
• line 25 "preserve the codec and body type" — asSecondary is a case-class copy; this tests Scala.
• lines 7, 21 and 51 assert the same fact through three accessors, and line 16 already asserts two of them together.

Worth a comment on line 39: assertDoesNotCompile("oneOfBody(...).asSecondary") passes because OneOfBody has no asSecondary member at all, not because of the ReplayableRawBody constraint — a future reader could easily take it as evidence the implicit is doing the work.

[adamw]

🤖 Claude-generated review

"accept an secondary body alongside an ordinary one" is redundant with EndpointBodyVerifierTest; the EndpointVerifier wiring is already proven by the two tests above it.

Typo in the test names: "an secondary" → "a secondary", here and on line 345, and in DecodeBasicInputsValuesTest.scala:11.

[adamw]

🤖 Claude-generated review

Unused — and it's the only change in this file.

[adamw]

🤖 Claude-generated review

"ignored by client interpreters" is true of what gets sent, but the value is still required: the secondary body stays in the client's input tuple and is discarded. SecondaryBodyClientTest shows it as (...)("sent")("ignored"). Worth a sentence, since it's the first thing a caller hits.

[adamw]

🤖 Claude-generated review

Two things for the release notes, since they change behaviour for existing users:

• two ordinary bodies moves from a request-time IllegalStateException("Double body definition") to a startup IllegalArgumentException;
• EndpointVerifier now reports endpoint shapes that previously passed, so existing verification tests can start failing.

The hidden-schema case (see the comment on EndpointBodyVerifier) belongs there too.

Related: DecodeBasicInputs.verifyNoBody is now a backstop only for hand-built ServerInterpreters — worth a comment saying so.