Client sends TransferEncoding: chunked for empty Body

[brandlk]

When receiving a HTTP request without a body, a Request with an empty Body is constructed instead of a Request with None as Body. Since GET requests neither have the ContentLength nor the TransferEncoding header set (this is the correct behavious as far as I understood), the hyper implementation automatically sets TransferEncoding: chunked in case I forward this request unchanged (as for example a proxy would do). This results in some servers responding with "Bad request" (for example "example.org"), which again seems to be the correct behaviour. Some servers are apparently more tolerant and reply nevertheless.

This means by literally doing nothing (e.g. just forwarding an incoming request), hyper turns a valid request into an invalid one, which seems strange.
Of course I could inspect the body and send a request without a body in case it is empty or build a request without a body in case neither ContentLength nor TransferEnconding is set, but this feels a bit like a workaround. And additionally, a Request does not offer any method to remove a body from the request. The method body() which takes the body out of the request, just replaces the original one with an empty one and thus does not help in this case. This means I have to build a new request and copy the headers from the original one before passing it on.

Is this behaviour intended? Or is there a chance to fix it? By skimming through the implementation I figured that Body is just a wrapper around the tokio-proto Body. This struct internally distinguishes between "Empty", "Stream" and "Once" but doesn't expose this functionality. If it would, (e.g, offering a "is_empty()" method), then it would be easy to change the hyper behaviour when encoding a request. But I'm not sure if it is desired to expose this, since conceptually there is no difference between an "officially" empty body and a stream without items. So maybe this is a stupid approach.

[seanmonstar]

No, this is not expected behavior. In fact, in tests/client.rs, the very first test makes sure that no body doesn't send additional headers.

Indeed, it's as you've described: the reason is because request::from_wire will create a Request with a Some(Body::empty()), which then does imply there is a body.

[brandlk]

Thanks a lot for this fast fix!

[walfie]

Was this bug reintroduced at some point? I'm upgrading an app from hyper 0.11.1 and I'm having a similar issue on hyper 0.11.18. The Transfer-Encoding header is being added to requests that have an empty body.

Code

Cargo.toml

[package]
authors = ["Example"]
name = "hyper-chunked-get-error"
version = "0.1.0"

[dependencies]
futures = "0.1.18"
hyper = "=0.11.18"
tokio-core = "0.1.12"

src/main.rs

extern crate futures;
extern crate hyper;
extern crate tokio_core;

use hyper::{Client, Method, Request};
use tokio_core::reactor::Core;
use futures::{Future, Stream};

fn main() {
    let mut core = Core::new().unwrap();

    let client = Client::new(&core.handle());

    let uri = "http://localhost:9000".parse().unwrap();
    let mut request = Request::new(Method::Get, uri);
    let body: &[u8] = &[]; // Set empty body
    request.set_body(body);

    let response = core.run({
        client
            .request(request)
            .and_then(|resp| resp.body().concat2())
    }).unwrap();
    println!("{:?}", std::str::from_utf8(&response).unwrap());
}

Running

When I run it, e.g.,

# Listen to port 9000
while true; do nc -l 9000; done

# In a different shell
cargo run

I get as output (from nc), for any version of hyper >=0.11.2:

GET / HTTP/1.1
Host: localhost:9000
Transfer-Encoding: chunked

0

If I downgrade to hyper 0.11.1, I get:

GET / HTTP/1.1
Host: localhost:9000

(no Transfer-Encoding header, as I would expect)

or maybe I'm doing something wrong? I could open a separate issue if it's actually a different problem.

[aep]

running into this too. blocks rusoto/rusoto#1074

[seanmonstar]

@aep this is using v0.12?

How is the body constructed?

[aep]

@seanmonstar wrap_stream which immediately returns Ok(Async::Ready(None))
using Body::empty() works fine for now

[seanmonstar]

Ah, ok. With wrap_stream, there's no way to know how big the body could actually be, so it will default to transfer-encoding then.

[aep]

yep, no idea how to do this correctly.

I would prefer if it didn't chunk at all, since there are cases where you might want to take the stream and just send whatever you want yourself (websocket for example)

but for rusoto, this hack works fine: rusoto/rusoto@ed67c83#diff-77037ac1da2b41727b3e95f3c007f331R370