hyperium / http-body Goto Github PK

View Code? Open in Web Editor NEW

115.0 12.0 45.0 107 KB

Asynchronous HTTP body trait

License: MIT License

Rust 100.00%

http-body's Introduction

HTTP Body

A trait representing asynchronous operations on an HTTP body.

License

This project is licensed under the MIT license.

Contribution

Unless you explicitly state otherwise, any contribution intentionally submitted for inclusion in http-body by you, shall be licensed as MIT, without any additional terms or conditions.

http-body's People

Contributors

Stargazers

Watchers

Forkers

mm4nn isgasho dekellum doumanash forkkit vorot93 hawkw benxiangge stanislav-tkach skeet70 olix0r agemanning fastly davidpdrsn tesaguri inikulin zazaulola g2p nmoutschen programatik29 neoeinstein indietyp lperlaki raw-http coolreader18 oddgrd wesleyac tottoto meowjesty sfackler nitsky f0rki inq chrislearn seanpm2001 aloxaf zuzmic hatoo daheige yanns mladedav lexe-app swlynch99 rtimush

http-body's Issues

rc1 vs rc.2 causes sorting issues on crates.io

Not sure if this can be fixed, but I found myself ending up on rc1 multiple times even though rc.2 is younger.

https://docs.rs/http-body/1.0.0-rc.2/http_body/index.html

Question: can frame.into_data() be incomplete?

I use http-body to parse the body of an endless Transfer-Encoding: Chunked stream.

let frame = response.frame().await.expect("Stream ended").expect("Failed to read frame");

let Ok(data) = frame.into_data() else {
    // frame is trailers, ignored
    continue;
};

let decoded = serde_json::from_slice(&data)?;

// ...

But as I've discovered, under certain conditions data is incomplete. When complete it ends in \n.

To fix it I have a buffer that I only parse out the part of [0..(index of first b'\n'] and remove it from the buffer.

This leaves me with the following questions:

Is this expected behavior from Frame? Having a partial piece in there?
Is the \n a left-over from the Chunked separator \r\n?

Adapters for AsyncRead and Stream?

In tower-http we have an adapter that makes an AsyncRead into a Body https://github.com/tower-rs/tower-http/blob/serve-file/tower-http/src/serve_file.rs#L107. Is that something you'd be interested in upstreaming to http-body? Might be useful for others.

Adapters for Stream<Item = Bytes> and Stream<Item = Vec<u8>> might also be nice.

If you're interested I can submit a PR.

Should `Request<B>` and `Response<B>` implement `Body`?

Today I discovered an unfortunate interaction between axum and http-body.

In axum you're able to write this:

use axum::{
    body::{box_body, Body, BoxBody},
    handler::get,
    http::Response,
    Router,
};

let app = Router::new().route(
    "/",
    get(|| async {
        // Build a response with a header.
        let response: Response<Body> = Response::builder()
            .header("x-foo", "foo")
            .body(Body::empty())
            .unwrap();

        // Since `Response<B>` implements `Body`, we can use `box_body` to
        // convert it into a `BoxBody`.
        let body: BoxBody = box_body(response);

        // And because `BoxBody` implements `IntoResponse` we can return it
        // from handlers.
        //
        // However `impl IntoResponse for BoxBody` simply does
        // `Response::new(self)` thus removing headers, status, etc.
        body
    }),
);

So because Response<B> implements Body it can be used with box_body but that ends up removing everything from the response except the body. I think being able to return bodies directly from axum handlers is a nice feature its just unfortunate that this particular thing compiles.

BoxBody's IntoResponse impl is here.

One could say "well just don't do this" but would be nice if it didn't compile at all, which could be done by removing impl Body for {Response, Request}<B>. Thats of course a breaking change 😞

Backport `Either` to 0.4

With the latest changes, moving types to http-body-util, I think this will require a breaking change release for a lot of crates.

I have a use-case where I require the recently introduced Either type from #64, currently I basically just copied the file. Would it be possible to backport #64 to a 0.4 branch and make a release?

Happy to make the PR of course.

Split trait into `http-body-core` crate

The http-body crate has been gradually accumulating more utilities in it. This is a good thing, but it runs into the problem that breaking changes to any one of these utilities can cause a major ecosystem breakage, since even if trait Body defined in http-body 0.4 and 0.5 are the exact same trait signature-wise, Rust still treats them as entirely separate traits and will not allow them to interact. This also results in very confusing error messages that usually say something like "expected http_body::Body, found http_body::Body".

To solve this, this crate can be split into two (à la tower-service/tower-layer/tower, rand-core/rand and futures-core/futures-io/futures-sink/futures): http-body-core that contains only the definition of the trait itself, and http-body that contains BodyExt and all the useful utilities. With this design, if we were to release http-body v0.5 that contains breaking changes to the utilities but not the trait, it could re-export the old trait from the same version of http-body-core and there would be basically no breakage at all - both versions of http-body would be able to seamlessly coëxist and interoperate.

Reimplement `hyper::body::to_bytes()`

This function was quite useful in tests: https://docs.rs/hyper/0.14.27/hyper/body/fn.to_bytes.html

Maybe I am not searching the correct places but I can't see where it ended up after hyper 1.0 and I assume it should probably be part of this crate.

how to check `Body::data()` has reached the end?

Is there a way in this crate to check if Body::data() returns no more data?

i.e. in something like:

match body.data().await {
// what match arm to use for the end of data? Any generic way?
}

Do I have to implement a concrete type of Body first and rely on its implementation-specific Data type?

Should there be a blanket impl for BufStream?

The current code on master has this impl:

impl<T: BufStream> Body for T {
    // ...
}

I'm worried this can cause problems, but it does have some benefits too.

Pro Ease of use for types that already implement BufStream. Some of those are String, Vec<u8>, &'static str, &'static [u8], Bytes. By providing this blanket impl, any bounds requiring B: Body will Just Work for those types.
Con Complicated for implementors since one cannot implement Body and BufStream. If someone implements Body, and then an API has B: BufStream, the type doesn't fit. This could be solved with into_buf_stream()/as_buf_stream() adapters...
Con Ties breaking changes to BufStream.

An alternative would be to remove the blankt impl, and instead having people use wrapping adapters (like struct BufStreamAsBody<B: BufStream>(B), etc).

Empty frame cause panic

http-body/http-body-util/src/collected.rs

Line 44 in 619ba28

self.bufs.push(data);

http-body/http-body-util/src/util.rs

Line 14 in 619ba28

debug_assert!(buf.has_remaining());

When frame is empty, this cause panic, just wondering if this is expected behavior?
Some HTTP/2 implementations may product empty frames, when I create a reverse proxy server，and call body.collect().await.to_bytes() to read data, server will give error in debug mode. Or just give a warning here?

Needs Body::data() variant with size limit

Right now there does not seem to be a good way to restrict the maximum size of the buffer returned by Body::data(). It's effectively unbounded.

I'm running into this in the real world. The scenario is essentially many clients sending large chunks simultaneously, leading to big memory spikes. I imagine that in some cases it's exploitable as a DDoS.

I'd like an option that lets me say "at most this many bytes" so I can rate limit incoming traffic.

Empty's Body impl `Error = Infallible` makes `Empty::new()` unable to be used w/ Body constraints

Empty has generic parameter for Data, so it would be nice to have a generic parameter for Error as well so that Empty::new() can be used w/ functions that set constraints on Body i.e B: Body<Error=..>

Return types of poll_data and poll_trailers don't agree

poll_data returns Poll<Option<Result<T, E>>> and poll_trailers returns Poll<Result<Option<T>, E>>>.

`EitherBody` helper

It will help greatly with middleware implementations, when one middleware may override responses.

parallel to actix_http::body::EitherBody

http-body-util: Incorrect required futures-util version

Trying to build http-body-util 0.1.0-rc.2 with Rust 1.69.0 gets me the following error:

error[E0432]: unresolved import `futures_util::Future`
 --> /home/wesleyac/.cargo/registry/src/github.com-1ecc6299db9ec823/http-body-util-0.1.0-rc.2/src/combinators/collect.rs:6:5
  |
6 | use futures_util::Future;
  |     ^^^^^^^^^^^^^^^^^^^^ no `Future` in the root
  |
help: a similar name exists in the module
  |
6 | use futures_util::future;
  |                   ~~~~~~
help: consider importing one of these items instead
  |
6 | use core::future::Future;
  |     ~~~~~~~~~~~~~~~~~~~~
6 | use futures_util::future::Future;
  |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
6 | use futures_util::future::MaybeDone::Future;
  |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
6 | use futures_util::future::TryMaybeDone::Future;
  |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    and 1 other candidate

For more information about this error, try `rustc --explain E0432`.
error: could not compile `http-body-util` due to previous error

Digging into this, it seems to be because it was trying to use futures-util version 0.3.13, which a different dependency was using. Updating to version 0.3.14 or newer fixes this problem.

So, the fix is to specify >=0.3.14 in Cargo.toml, although it might be better to pin a specific version, since clearly futures-util either doesn't care about or isn't careful about following semver.

Should the method be called poll_buf or poll_data?

I lean slightly toward Body::poll_data, as it feels more descriptive.

Publish to crates.io

We should publish the crate. @seanmonstar does the API look good to you? I can work on the misc publish prep soon.

`AnyBody`?

I'm trying to help the upgrade of kube-rs. We're a bit confused with the new Body trait and in our case kube-rs implements a tower Service so users can build their own stacks.
The issue is that it requires to know the body type ahead of time (it can be different types based on the exact request used).
https://github.com/kube-rs/kube/blob/c2983f6d196daecde39069f7ab322375ff9d1566/kube-client/src/client/mod.rs#L68

I was thinking maybe there should be an AnyBody enum, that takes the simple types that are implemented in this crate and sums it so then the service can use this as well.
Maybe I have wrong idea of that design but would be help to hear your thoughts.

Frame should have by-self conversion methods returning Self on error

It's currently kind of annoying to try to unpack a Frame into a series of options:

if frame.is_data() {
    let data = frame.into_data().unwrap();
    // ...
}

if frame.is_trailers() {
    let trailers = frame.into_trailers().unwrap();
    // ...
}

// etc

It would be nice if the into_* methods (or new ones) returned Result<T, Self> rather than Option<T> so you could just match on the result instead of having to do the check-and-unwrap dance:

frame = match frame.into_data() {
    Ok(data) => // ...
    Err(frame) => frame,
}

frame = match frame.into_trailers() {
    Ok(trailers) => // ....
    Err(frame) => frame,
}

Is it able to support implement Body for h2::RecvStream in crate?

I think it is convenient if implementing Body for h2::RecvStreamBody as default, because we get http::Request<h2::RecvStream> when we use h2 to handle HTTP2 requests.

impl Body for h2::RecvStream {
    type Data = Bytes;
    type Error = h2::Error;

    fn poll_data(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<Option<Result<Self::Data, Self::Error>>> {
        return self.poll_data(cx);
    }

    fn poll_trailers(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<Result<Option<HeaderMap>, Self::Error>> {
        return self.poll_trailers(cx);
    }
}

adding benchmarks

I'm looking into adding benchmarks to this library, inspired by https://github.com/hyperium/hyper/blob/master/benches/body.rs
The goal is to make it easier to check whether a change has a positive impact or not.

Before starting the implementation, I'd like to gather some initial feedback from the maintainers:

do you think it's a good idea?
if yes, do you have a preference which library should be used? (criterion, divan, other?)

Add string implementations for `http_body::Body`?

Hi folks! 👋

I'm currently working on a change to the lambda-http crate that would accept any Request<http_body::Body>. However, the current behaviour allows supporting Request<String>, Request<&str>, etc. Furthermore, it could be interesting for other web frameworks to accept Request<String> & co to give more flexibility to the end users.

However, due to the conflicting trait implementation rule, I cannot have implementations for both a Request<http_body::Body> and Request<String>, as http_body may add implementations for String in future versions.

To solve this issue, I think it'd be nice to add implementations of Body for String, &'static str, and Cow<'static, str> directly into the http_body crate. I'm happy to create a Pull Request if you find this relevant.

If not relevant and there might be a more elegant way to solve this problem, let me know!

I found a bug

Body Combinators (like .map_data or .map_err)

I've been missing e. g. being able to easily change the error type of a body w/o writing huge amounts of boilerplate. A couple of combinators could come in handy, perhaps feature-gated under util. Happy to file a PR.

In the first pass I'd add:

.map_data
.map_err
.boxed

Those should be self-explanatory. We can add more in a follow-up.

Body docs mention the old trait methods

The docs for trait Body still mention poll_data and poll_trailers. That wording should be replaced, mentioning everything goes through poll_frame now.