Cannot close normally after takebody in middleware #2370

Tim-Paik · 2021-08-21T07:59:41Z

Tim-Paik
Aug 21, 2021

I am trying to use environment variables to control whether a certain directory returns HTTP403, the following is the simplified code.

It seems that everything is normal, but there is a timeout phenomenon during the test, and there is no content-length header, which causes the browser and curl to wait for 5s before ending.

How should I modify my code to make it work? Or, is there any better way to return HTTP403 (without body)?

[dependencies]
actix-web = "3"
actix-files = "0.5"

use actix_web::{App, dev::Service, http::{HeaderMap, StatusCode}, HttpServer};

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    HttpServer::new(|| {
        App::new().wrap_fn(|req, srv| {
            let fut = srv.call(req);
            async move {
                Ok(fut.await?.map_body(|head, mut body| {
                    if std::env::var("FORBIDDEN").unwrap_or("false".to_string()) == "true" {
                        head.status = StatusCode::FORBIDDEN;
                        *head.headers_mut() = HeaderMap::new();
                        let _ = body.take_body();
                    }
                    body
                }))
                
            }
        }).service(actix_files::Files::new("/", "/").show_files_listing())
    })
    .bind("127.0.0.1:8080")?
    .run()
    .await
}

Answered by fakeshadow

Aug 22, 2021

And a side note you may want to use a middleware where you check the env before actix_files service get called. Your current code would call the service regardless and make fs syscall.

View full answer

fakeshadow · 2021-08-22T03:38:54Z

fakeshadow
Aug 22, 2021

This is likely a design flaw as actix-web in general expect you to return something as body even if it's empty.

The take_body API does not make any sense from a high level usage point of view other than actually dealing with the body that is taken out from the request/response. In other word it's a footgun if you just take the body and throw it away.

This is an work around

use actix_web::body::Body;
use actix_web::dev::ResponseBody;
use actix_web::web::Bytes;
use actix_web::{
    dev::Service,
    http::{HeaderMap, HeaderName, HeaderValue, StatusCode},
    App, HttpServer,
};

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    HttpServer::new(|| {
        App::new()
            .wrap_fn(|req, srv| {
                let fut = srv.call(req);
                async move {
                    Ok(fut.await?.map_body(|head, _| {
                        head.status = StatusCode::FORBIDDEN;
                        *head.headers_mut() = HeaderMap::new();
                        head.headers_mut().insert(
                            HeaderName::from_static("content-length"),
                            HeaderValue::from_static("0"),
                        );
                        ResponseBody::<Body>::Other(Body::Bytes(Bytes::new()))
                    }))
                }
            })
            .service(actix_files::Files::new("/", "/").show_files_listing())
    })
    .bind("127.0.0.1:8080")?
    .run()
    .await
}

You can also keep your code as is with extra head.set_connection_type(ConnectionType::Close); This would notify the client to close the current connection which would bypass the keep-alive if that's your goal. The above work around on the other hand would still keep connection alive for future requests.

2 replies

Tim-Paik Aug 22, 2021
Author

i tried to return ResponseBody::<Body>::Other(Body::Bytes(Bytes::new())),but it prompts me that there is an error

mismatched types
expected enum `ResponseBody<actix_web::dev::Body>`
   found enum `ResponseBody<actix_http::encoding::encoder::Encoder<actix_web::dev::Body>>`

Maybe it's because you want to use Either? But there are errors. It seems that the function wrapped by map_body requires that the body of the function parameter and the returned value are of the same type?

type mismatch resolving `for<'r> <[closure@src/main.rs:571:25: 598:26] as FnOnce<(&'r mut ResponseHead, ResponseBody<Encoder<actix_web::dev::Body>>)>>::Output == ResponseBody<_>`
expected enum `Either<ResponseBody<actix_web::dev::Body>, ResponseBody<Encoder<actix_web::dev::Body>>>`
   found enum `ResponseBody<_>`

fakeshadow Aug 22, 2021

Different combination of App could result in different type of response body. My example works as is with the given code. If you have different type of response body you have to annotate it accordingly if you use map_body API

fakeshadow · 2021-08-22T03:41:34Z

fakeshadow
Aug 22, 2021

And a side note you may want to use a middleware where you check the env before actix_files service get called. Your current code would call the service regardless and make fs syscall.

3 replies

Tim-Paik Aug 22, 2021
Author

There are really few documents about wrap_fn, even rust-analyzer in VSCode prompts srv.call(req) to be {unknown}.
What should i do? Where can I get more documentation? (Or should I read the source code?) I found that even the code of https://actix.rs/docs/middleware/ can't be compiled, it seems to be out of date?
Sorry I am a beginner, I really appreciate your answers!

fakeshadow Aug 22, 2021

This is an example of how to use middleware as a guard to prevent the running of actix-files service.
Both of wrap_fn and wrap does the same job so it's dulipcated in function.
In general the typed middleware with App::wrap is suggested. It's more boilerplate but have a nicer API for async {} blocks

Cargo.toml

actix-web = { version = "3", default-features = false }
actix-files = "0.5"
futures = "0.3"

Code

use std::task::{Context, Poll};

use actix_web::dev::{Service, ServiceRequest, ServiceResponse, Transform};
use actix_web::{App, Error, HttpResponse, HttpServer};
use futures::future::{Either, LocalBoxFuture};

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    let forbidden = std::env::var("FORBIDDEN")
        .map(|r| r.contains("true"))
        .unwrap_or(false);

    HttpServer::new(move || {
        App::new()
            .wrap_fn(move |req, srv| {
                let either = if forbidden {
                    Either::Left(req.into_response(HttpResponse::Forbidden().finish()))
                } else {
                    Either::Right(srv.call(req))
                };

                async move {
                    match either {
                        Either::Left(res) => Ok(res),
                        Either::Right(fut) => fut.await,
                    }
                }
            })
            .wrap(Env)
            .service(actix_files::Files::new("/", "/").show_files_listing())
    })
    .bind("127.0.0.1:8080")?
    .run()
    .await
}

struct Env;

impl<S> Transform<S> for Env
where
    S: Service<Request = ServiceRequest, Response = ServiceResponse, Error = Error> + 'static,
{
    type Request = S::Request;
    type Response = S::Response;
    type Error = S::Error;
    type Transform = EnvMiddleware<S>;
    type InitError = ();
    type Future = LocalBoxFuture<'static, Result<Self::Transform, Self::InitError>>;

    fn new_transform(&self, service: S) -> Self::Future {
        Box::pin(async move {
            Ok(EnvMiddleware {
                forbidden: std::env::var("FORBIDDEN")
                    .map(|r| r.contains("true"))
                    .unwrap_or(false),
                service,
            })
        })
    }
}

struct EnvMiddleware<S> {
    forbidden: bool,
    service: S,
}

impl<S> Service for EnvMiddleware<S>
where
    S: Service<Request = ServiceRequest, Response = ServiceResponse, Error = Error> + 'static,
{
    type Request = S::Request;
    type Response = S::Response;
    type Error = S::Error;
    type Future = LocalBoxFuture<'static, Result<Self::Response, Self::Error>>;

    fn poll_ready(&mut self, ctx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        self.service.poll_ready(ctx)
    }

    fn call(&mut self, req: Self::Request) -> Self::Future {
        if self.forbidden {
            let res = req.into_response(HttpResponse::Forbidden().finish());
            Box::pin(async { Ok(res) })
        } else {
            Box::pin(self.service.call(req))
        }
    }
}

Tim-Paik Aug 25, 2021
Author

After reading your code, I know why my code is not working: I added middleware::Compress before wrap_fn, resulting in the parameter srv type not being &mut AppRouting, but srv: &mut <impl ServiceFactory<Config = (), Request = ServiceRequest, Response = ServiceResponse<{unknown}>, Error = Error, InitError = ()> as ServiceFactory>::Service, so there is a problem
Thank you for your patience!

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cannot close normally after takebody in middleware #2370

{{title}}

Replies: 2 comments 5 replies

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{editor}}'s edit

{{editor}}'s edit

Select a reply

Cannot close normally after takebody in middleware #2370

Tim-Paik Aug 21, 2021

Replies: 2 comments · 5 replies

fakeshadow Aug 22, 2021

Tim-Paik Aug 22, 2021 Author

fakeshadow Aug 22, 2021

fakeshadow Aug 22, 2021

Tim-Paik Aug 22, 2021 Author

fakeshadow Aug 22, 2021

Tim-Paik Aug 25, 2021 Author

Tim-Paik
Aug 21, 2021

Replies: 2 comments 5 replies

fakeshadow
Aug 22, 2021

Tim-Paik Aug 22, 2021
Author

fakeshadow
Aug 22, 2021

Tim-Paik Aug 22, 2021
Author

Tim-Paik Aug 25, 2021
Author