Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaxContentLength support for Netty pt 2 #3337

Merged
merged 36 commits into from
Dec 4, 2023
Merged

Conversation

kciesielski
Copy link
Member

@kciesielski kciesielski commented Nov 23, 2023

Partially addresses #2971
This is a follow-up to PR #3319

Add raw body content length limiting for all Netty backends.

After enabling body length limit on an endpoint, both streaming and non-streaming (raw) requests will be checked. If exceeded, HTTP 413 Payload Too Large response is sent.
Usage:

import sttp.tapir._
import sttp.tapir.server.model.EndpointExtensions._

val limitedEndpoint = endpoint.maxRequestBodyLength(maxBytes = 163484L)

Design notes and open questions:

  1. That EndpointExtensions and MaxContentLength types exposed as public API are in the sttp.tapir.server.model package, in the tapir-server core module. I'm happy do discuss better alternatives.

  2. I heavily refactored all Netty backends to exrtact some repetitions into NettyRequestBody, NettyStreamingRequestBody, NettyToResponseBody, NettyToStreamsResponseBody, Fs2StreamsCompatible, ZioStreamsCompatible.

  3. A critical change was to switch our base Netty pipeline to always use Reactive Streams. This allows to process incoming bytes for raw requests chunk by chunk, and potentially react if total accumulated length is too large. While cats and zio offer simple converters to/from Subscriber/Publisher for their streams, in the Future and Loom backends I had to write manual integrations. They deal with concurrency using lower-level mechanisms, please review them carefully. This includes publishers and subscribers located in netty-server/src/main/scala/sttp/tapir/server/netty/internal/reactivestreams/.

@kciesielski kciesielski changed the title MaxContentLength support (non-streaming) [WIP] MaxContentLength support for Netty (non-streaming) Nov 23, 2023
@kciesielski kciesielski changed the base branch from 3056-max-content-length to master November 23, 2023 09:44
@kciesielski kciesielski linked an issue Dec 1, 2023 that may be closed by this pull request
@kciesielski kciesielski changed the title [WIP] MaxContentLength support for Netty (non-streaming) [WIP] MaxContentLength support for Netty Dec 1, 2023
@kciesielski kciesielski changed the title [WIP] MaxContentLength support for Netty MaxContentLength support for Netty pt 2 Dec 1, 2023
@kciesielski kciesielski marked this pull request as ready for review December 1, 2023 18:38
@kciesielski kciesielski requested a review from adamw December 1, 2023 18:38
doc/endpoint/security.md Outdated Show resolved Hide resolved
doc/endpoint/security.md Outdated Show resolved Hide resolved
@adamw
Copy link
Member

adamw commented Dec 4, 2023

Looks good, let's ship it :)

@kciesielski kciesielski mentioned this pull request Dec 4, 2023
@kciesielski kciesielski merged commit abeb5d7 into master Dec 4, 2023
17 checks passed
@Pask423 Pask423 deleted the 3056-non-streaming branch May 7, 2024 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Extended maxContentLength for Netty
2 participants