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

Cannot read request body in middleware #373

Closed
vbrandl opened this issue Jul 7, 2018 · 5 comments
Closed

Cannot read request body in middleware #373

vbrandl opened this issue Jul 7, 2018 · 5 comments

Comments

@vbrandl
Copy link

vbrandl commented Jul 7, 2018

I'm trying to validate a request by calculating the HMAC over the body and comparing against a value passed as an HTTP header. I'm trying to do this using a middleware:

impl Middleware<Arc<Config>> for VerifySignature {
    fn start(&self, req: &mut HttpRequest<Arc<Config>>) -> Result<Started> {
        let mut body = Vec::new();
        info!("Loading body");
        req.read_to_end(&mut body).map_err(|e| {
            info!("{:?}", e);
            e
        })?;
        let sig = req.headers()
            .get(data::TOKEN_HEADER)
            .as_ref()
            .ok_or(ErrorUnauthorized(ParseError::Header))?
            .to_str()
            .map_err(ErrorUnauthorized)
            .and_then(|s| {
                crypto::hex_str_to_bytes(s).map_err(|_| ErrorUnauthorized(ParseError::Header))
            })
            .map_err(ErrorUnauthorized)?;
        let secret = &req.state().token;
        if crypto::verify_signature(secret.as_bytes(), &body, &sig) {
            info!("Valid signature");
            Ok(Started::Done)
        } else {
            error!("Invalid signature");
            Err(ErrorUnauthorized(ParseError::Header))
        }
    }
}

This will fail after the call to read_to_end with Error occured during request handling, status: 500 Internal Server Error Not ready. It seems like read_to_end does not block to wait for the future being finished. What would be the correct way, to get the request body as a Vec<u8> or &[u8]?

@ghost
Copy link

ghost commented Jul 7, 2018

I think you should use req.body().concat2().and_then(..) or something like that

@vbrandl
Copy link
Author

vbrandl commented Jul 7, 2018

The problem is, that the current version on crates.io will take ownership of a request, when calling body() but in a middleware, I only got a mutable reference to the request.
https://github.com/actix/actix-web/blob/v0.6.14/src/httpmessage.rs#L137

Cloning the request before calling the methods of Stream or Future will result in an empty body.

@fafhrd91
Copy link
Member

fafhrd91 commented Jul 8, 2018

you can clone request, but if you read body in middleware, handle won't be able to get body, you would need to pass body from middleware to handler somehow.

if you need to verify body, you have to combine verification with handling.
Another option is to create custom extractor, check FromRequest trait.

@fafhrd91 fafhrd91 closed this as completed Jul 9, 2018
@ChriFo
Copy link

ChriFo commented Jul 9, 2018

I think I am at the same point. I want to send parts of the request out of the middleware to validate them. Currently I am able to extract the parts by using a String extractor. But when using the crossbeam_channel::Sender::send I am in the lifetime hell :-(

cannot infer an appropriate lifetime due to conflicting requirements

expected std::boxed::Box<futures::Future<Item=std::option::Option<actix_web::HttpResponse>, Error=actix_web::Error> + 'static>
found std::boxed::Box<futures::Future<Item=std::option::Option<actix_web::HttpResponse>, Error=actix_web::Error>>

#[derive(Debug)]
pub struct Request {
    body: String,
    method: String,
    path: String,
}

pub struct SendRequest {
    tx: channel::Sender<Request>,
}

impl<S: 'static> Middleware<S> for SendRequest {
    fn start(&self, req: &mut HttpRequest<S>) -> actix_web::Result<Started> {
        let method = req.method().as_str().to_string();
        let path = req.path().to_string();

        let fut = <String>::extract(&req)
            .expect("Failed to extract body")
            .and_then(move |body| {
                println!("Body: {}", body);
                println!("Method: {}", method);
                println!("Path: {}", path);

                self.tx.send(Request { body, method, path });

                future::ok(None)
            });

        Ok(Started::Future(Box::new(fut)))
    }
}

I am very thankful for some kind of hint.

@DoumanAsh
Copy link
Contributor

DoumanAsh commented Jul 9, 2018

I think the problem is that String::extract returns future which hooked up on &mut HttpRequest lifetime.

Everything is fine with your self.tx.send but not with your future.
as &mut HttpRequest is not 'static so is the future produced by String::extract

I guess you may need to clone HttpRequest for it to retain 'static lifetime

Feel free to join gitter for some real-time Q&A 😄

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

4 participants