Middleware that change the request body have poor usability #1110

davidpdrsn · 2022-06-20T13:46:42Z

We've heard from several people who try to use tower_http::limit::RequestBodyLimitLayer that the implications are hard to understand.

For example breaking your Router into functions is quite common. Something like:

#[tokio::main]
async fn main() {
    let app = my_routes();

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    axum::Server::bind(&addr)
        .serve(app.into_make_service())
        .await
        .unwrap();
}

fn my_routes() -> Router {
    Router::new()
}

Adding RequestBodyLimitLayer like so

let app = my_routes().layer(RequestBodyLimitLayer::new(1024));

should just work but produces a rather confusing error:

    Checking example-hello-world v0.1.0 (/Users/davidpdrsn/dev/major/axum/examples/hello-world)
error[E0277]: the trait bound `Route: tower_service::Service<Request<http_body::limited::Limited<_>>>` is not satisfied
  --> hello-world/src/main.rs:13:27
   |
13 |     let app = my_routes().layer(RequestBodyLimitLayer::new(1024));
   |                           ^^^^^ the trait `tower_service::Service<Request<http_body::limited::Limited<_>>>` is not implemented for `Route`
   |
   = help: the following implementations were found:
             <Route<B, E> as tower_service::Service<Request<B>>>
   = note: required because of the requirements on the impl of `tower_service::Service<Request<_>>` for `RequestBodyLimit<Route>`

The solution is instead to write

fn my_routes() -> Router<http_body::Limited<Body>> {
    Router::new()
}

but that isn't obvious and ideally shouldn't be necessary.

We should think about ways to improve that experience.

The text was updated successfully, but these errors were encountered:

davidpdrsn · 2022-08-22T12:08:01Z

I think this is closely related to #1154. If axum had its own body type, and a way to construct that from any B: http_body::Body then we could use that everywhere, even if you applied middleware.

That is something we'll have to explore once the development of hyper 1.0 (and hyper-utils) is further along. Hyper 1.0 will require a breaking release of axum anyway, so I think we should wait with this until we're closer to that happening.

I'm gonna remove this from the 0.6 milestone.

avdb13 · 2022-09-15T16:57:58Z

why does ContentLengthLimit exist in Axum when we have tower_http::limit::RequestBodyLimitLayer in Tower, on which Axum relies anyway?
What are their biggest differences other than that the former has the tradeoff that its argument needs to be a const?

davidpdrsn · 2022-09-18T18:59:35Z

why does ContentLengthLimit exist in Axum when we have tower_http::limit::RequestBodyLimitLayer in Tower, on which Axum relies anyway?
What are their biggest differences other than that the former has the tradeoff that its argument needs to be a const?

Mostly history. ContentLengthLimit was made before RequestBodyLimitLayer existed. It also has a few differences

It doesn't change the type of your Router. Adding RequestBodyLimitLayer changes the Router type to Router<_, http_body::Limited<_>>. Thats what this issue is about.
It doesn't permit streaming requests, which RequestBodyLimitLayer does.

I tried previously to implement ContentLengthLimit on top of http_body::Limited but couldn't do it because of limitations in 0.5. However in 0.6 FromRequest gets an owned Request<B> which it can take a part and change the body type of. So it might be possible to bridge the cap further between the two and make ContentLengthLimit support streaming requests.

Edit: Its possible #1389!

avdb13 · 2022-09-22T12:54:26Z

I'm trying to use tower_http::limit::RequestBodyLimitLayer on a single handler but I get the following error:

error[E0277]: the trait bound `axum::handler::Layered<RequestBodyLimit<IntoService<fn(Extension<Arc<App>>, Multipart) -> impl Future<Output = axum::response::Redirect> {handlers::creat
e_post}, (Extension<Arc<App>>, Multipart), _>>, (Extension<Arc<App>>, Multipart)>: Handler<_, _>` is not satisfied
   --> src/routes.rs:27:32
    |
27  |         .route("/:board", post(create_post))
    |                           ---- ^^^^^^^^^^^ the trait `Handler<_, _>` is not implemented for `axum::handler::Layered<RequestBodyLimit<IntoService<fn(Extension<Arc<App>>, Multipart
) -> impl Future<Output = axum::response::Redirect> {handlers::create_post}, (Extension<Arc<App>>, Multipart), _>>, (Extension<Arc<App>>, Multipart)>`
    |                           |
    |                           required by a bound introduced by this call
    |
    = help: the trait `Handler<T, ReqBody>` is implemented for `axum::handler::Layered<S, T>`
note: required by a bound in `post`
   --> /home/mikoto/.cargo/registry/src/github.com-1ecc6299db9ec823/axum-0.5.13/src/routing/method_routing.rs:400:1
    |
400 | top_level_handler_fn!(post, POST);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `post`
    = note: this error originates in the macro `top_level_handler_fn` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Router<http_body::Limited<axum::body::Body>>: tower_service::Service<Request<axum::body::Body>>` is not satisfied
  --> src/main.rs:71:16
   |
71 |         .serve(router.into_make_service())
   |          ----- ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `tower_service::Service<Request<axum::body::Body>>` is not implemented for `Router<http_body::Limited<axum::body::Body>>`
   |          |
   |          required by a bound introduced by this call
   |
   = help: the trait `tower_service::Service<Request<B>>` is implemented for `Router<B>`
   = note: required because of the requirements on the impl of `hyper::service::http::HttpService<axum::body::Body>` for `Router<http_body::Limited<axum::body::Body>>`
   = note: required because of the requirements on the impl of `hyper::service::make::MakeServiceRef<hyper::server::tcp::addr_stream::AddrStream, axum::body::Body>` for `IntoMakeServic
e<Router<http_body::Limited<axum::body::Body>>>`

error[E0277]: the trait bound `hyper::common::exec::Exec: hyper::common::exec::NewSvcExec<hyper::server::tcp::addr_stream::AddrStream, IntoMakeServiceFuture<Router<http_body::Limited<a
xum::body::Body>>>, Router<http_body::Limited<axum::body::Body>>, hyper::common::exec::Exec, hyper::server::server::NoopWatcher>` is not satisfied
  --> src/main.rs:71:43
   |
71 |           .serve(router.into_make_service())
   |  ___________________________________________^
72 | |         .await
   | |______________^ the trait `hyper::common::exec::NewSvcExec<hyper::server::tcp::addr_stream::AddrStream, IntoMakeServiceFuture<Router<http_body::Limited<axum::body::Body>>>, Route
r<http_body::Limited<axum::body::Body>>, hyper::common::exec::Exec, hyper::server::server::NoopWatcher>` is not implemented for `hyper::common::exec::Exec`
   |
   = help: the trait `hyper::common::exec::NewSvcExec<I, N, S, E, W>` is implemented for `hyper::common::exec::Exec`
   = note: required because of the requirements on the impl of `Future` for `Server<hyper::server::tcp::AddrIncoming, IntoMakeService<Router<http_body::Limited<axum::body::Body>>>>`
   = note: required because of the requirements on the impl of `IntoFuture` for `Server<hyper::server::tcp::AddrIncoming, IntoMakeService<Router<http_body::Limited<axum::body::Body>>>>
`
help: remove the `.await`
   |
71 -         .serve(router.into_make_service())
71 +         .serve(router.into_make_service())
   |

error[E0277]: the trait bound `Router<http_body::Limited<axum::body::Body>>: tower_service::Service<Request<axum::body::Body>>` is not satisfied
  --> src/main.rs:71:10
   |
71 |         .serve(router.into_make_service())
   |          ^^^^^ the trait `tower_service::Service<Request<axum::body::Body>>` is not implemented for `Router<http_body::Limited<axum::body::Body>>`
   |
   = help: the trait `tower_service::Service<Request<B>>` is implemented for `Router<B>`
   = note: required because of the requirements on the impl of `hyper::service::http::HttpService<axum::body::Body>` for `Router<http_body::Limited<axum::body::Body>>`
   = note: required because of the requirements on the impl of `hyper::service::make::MakeServiceRef<hyper::server::tcp::addr_stream::AddrStream, axum::body::Body>` for `IntoMakeServic
e<Router<http_body::Limited<axum::body::Body>>>`

This is what my router looks like:

pub fn routes(app: Arc<App>) -> Router<http_body::Limited<Body>> {
    let create_post = handlers::create_post.layer(RequestBodyLimitLayer::new(
        (app.config.security.validate_upload_limit()).unwrap(),
    ));

    Router::new()
        ...
        .route("/:board", post(create_post))
        .layer(TraceLayer::new_for_http())
        .layer(Extension(app))
        .fallback(handlers::fallback.into_service())
}

avdb13 · 2022-09-22T13:07:52Z

The following compiles fine:

pub fn routes(app: Arc<App>) -> Router {
    let create_post = handlers::create_post;

    Router::new()
        ...
        .route("/:board", post(create_post))
        .layer(TraceLayer::new_for_http())
        // .layer(RequestBodyLimitLayer::new(
        //     (app.config.security.validate_upload_limit()).unwrap(),
        // ))
        .layer(Extension(app))
        .fallback(handlers::fallback.into_service())
}

davidpdrsn · 2022-09-22T15:49:33Z

@avdb13 please open a separate issue/discussion about that and please include a minimal reproducible example that we can run.

davidpdrsn · 2023-03-20T20:33:59Z

I'd say this has been solved by #1751. TLDR: in axum 0.7 the B type parameter has been removed and we instead use our own Body type. So adding new middleware never changes the request body type.

davidpdrsn added C-musings Category: musings about a better world A-axum labels Jun 20, 2022

davidpdrsn added this to the 0.6 milestone Jun 25, 2022

davidpdrsn mentioned this issue Jul 11, 2022

Remove request body type param #1154

Closed

2 tasks

davidpdrsn self-assigned this Jul 11, 2022

davidpdrsn mentioned this issue Jul 11, 2022

feat(body): add Body::wrap_body hyperium/hyper#2913

Closed

davidpdrsn added the E-hard Call for participation: Experience needed to fix: Hard / a lot label Jul 13, 2022

davidpdrsn removed this from the 0.6 milestone Aug 22, 2022

davidpdrsn mentioned this issue Aug 25, 2022

Hyper 1.0 support #1325

Closed

davidpdrsn mentioned this issue Sep 18, 2022

Support streaming/chunked requests in ContentLengthLimit #1389

Merged

davidpdrsn mentioned this issue Sep 24, 2022

Tracking: Improvements to DefaultBodyLimit #1409

Closed

3 tasks

This was referenced Nov 27, 2022

v0.7.0 #1583

Merged

Add axum_core::body::Body #1584

Merged

davidpdrsn mentioned this issue Feb 12, 2023

Remove B type param #1751

Merged

1 task

avdb13 mentioned this issue Mar 5, 2023

the trait Clone is not implemented for http_body::Limited<hyper::Body> #1817

Closed

davidpdrsn closed this as completed Mar 20, 2023

yokomizor mentioned this issue Sep 28, 2023

Axum Integration for BoxBody or impl HttpBody. leptos-rs/leptos#1793

Closed

markgomez mentioned this issue Jan 6, 2024

RequestBodyLimitLayer when applied via ServiceBuilder won't compile in v0.7 #2492

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Middleware that change the request body have poor usability #1110

Middleware that change the request body have poor usability #1110

davidpdrsn commented Jun 20, 2022 •

edited

davidpdrsn commented Aug 22, 2022

avdb13 commented Sep 15, 2022

davidpdrsn commented Sep 18, 2022 •

edited

avdb13 commented Sep 22, 2022

avdb13 commented Sep 22, 2022

davidpdrsn commented Sep 22, 2022

davidpdrsn commented Mar 20, 2023

Middleware that change the request body have poor usability #1110

Middleware that change the request body have poor usability #1110

Comments

davidpdrsn commented Jun 20, 2022 • edited

davidpdrsn commented Aug 22, 2022

avdb13 commented Sep 15, 2022

davidpdrsn commented Sep 18, 2022 • edited

avdb13 commented Sep 22, 2022

avdb13 commented Sep 22, 2022

davidpdrsn commented Sep 22, 2022

davidpdrsn commented Mar 20, 2023

davidpdrsn commented Jun 20, 2022 •

edited

davidpdrsn commented Sep 18, 2022 •

edited