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

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

Open
markgomez opened this issue Jan 6, 2024 · 5 comments

Comments

@markgomez
Copy link

  • [×] I have looked for existing issues (including closed) about this

Bug Report

Version

  • axum v0.7.3
  • axum-core v0.4.2

Platform

  • Darwin Kernel Version 23.0.0: Fri Sep 15 14:41:34 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T8103 arm64

Crates

axum = "0.7.3"
tokio = { version = "1.35.1", features = ["full"] }
tower = "0.4.13"
tower-http = { version = "0.5.0", features = ["limit"] }

Description

With ServiceBuilder I was applying multiple middleware, the first of which was RequestBodyLimitLayer and everything compiled in v0.6 (when the B type parameter was still available). With Axum now having its own body type in v0.7, the following example no longer compiles:

use axum::{
    extract::{Request, State},
    middleware::{self, Next},
    response::{Response, Result},
    routing::get,
    Router,
};
use tower::ServiceBuilder;
use tower_http::limit::RequestBodyLimitLayer;

#[tokio::main]
async fn main() {
    let app = Router::new()
        .route("/", get(|| async { "Hello, Axum!" }))
        .layer(
            ServiceBuilder::new()
                .layer(RequestBodyLimitLayer::new(1_048_576))
                .layer(middleware::from_fn_with_state("state", foo)),
        );

    let listener = tokio::net::TcpListener::bind("0.0.0.0:8080").await.unwrap();

    axum::serve(listener, app).await.unwrap();
}

async fn foo(State(_state): State<&'static str>, req: Request, next: Next) -> Result<Response> {
    Ok(next.run(req).await)
}
error[E0277]:
the trait bound `axum::middleware::FromFn<fn(axum::extract::State<&'static str>,
axum::http::Request<axum::body::Body>, axum::middleware::Next) ->
impl std::future::Future<Output = std::result::Result<axum::http::Response<axum::body::Body>,
axum::response::ErrorResponse>> {foo}, &str, axum::routing::Route, _>:
tower::Service<axum::http::Request<tower_http::body::Limited<axum::body::Body>>>` is not satisfied

I noticed in the "Fewer generics" section from the v0.7 announcement blog post that ServiceBuilder wasn't being used and sure enough things will compile if RequestBodyLimitLayer is moved to its own layer (like in the blog post):

 // compiles
 let app = Router::new()
     .route("/", get(|| async { "Hello, Axum!" }))
     .layer(
         ServiceBuilder::new()
-            .layer(RequestBodyLimitLayer::new(1_048_576))
             .layer(middleware::from_fn_with_state("state", foo)),
     )
+    .layer(RequestBodyLimitLayer::new(1_048_576));

To keep RequestBodyLimitLayer a part of ServiceBuilder, things will also compile if it is moved out of the first position (but I will eventually need it to be first):

 // also compiles, but limits need to be applied before everything else, so ultimately won't work
 let app = Router::new()
     .route("/", get(|| async { "Hello, Axum!" }))
     .layer(
         ServiceBuilder::new()
-            .layer(RequestBodyLimitLayer::new(1_048_576))
             .layer(middleware::from_fn_with_state("state", foo))
+            .layer(RequestBodyLimitLayer::new(1_048_576)),
     );

In #1110 (and mentioned in the blog post) the solution back in v0.6 was to extract Request<http_body::Limited<Body>> instead of Request<Body> in subsequent middleware. But with the body type parameter being removed from v0.7, I'm not sure how to satisfy the trait bound in this situation (other than avoid using RequestBodyLimitLayer with ServiceBuilder).

@markgomez
Copy link
Author

I think I figured it out. I'm using tower-http@0.5.x which depends on http@1.0.x (ok so far), but tower::ServiceBuilder is still on http@0.2.x (old/incompatible Request type). So it looks like I'll have to use RequestBodyLimitLayer in its own layer until the tower crate switches to http@1.0.x.

@davidpdrsn
Copy link
Member

That can’t be it. Tower doesn’t depend on http at all.

@markgomez
Copy link
Author

In that case, does this look like a bug? If so, I’ll reopen.

@markgomez
Copy link
Author

markgomez commented Jan 9, 2024

That can’t be it. Tower doesn’t depend on http at all.

I've been looking into this issue a little more and do see an http dependency here. Could this be related? It's a dev-dependency though, so other than that I'm out of ideas.

@markgomez markgomez reopened this Jan 9, 2024
@davidpdrsn
Copy link
Member

No that’s unrelated since it’s a dev dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants