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

Content-Length or Transfer-Encoding header (RFC 7230) #86

Open
jbolila opened this issue Oct 29, 2017 · 3 comments
Open

Content-Length or Transfer-Encoding header (RFC 7230) #86

jbolila opened this issue Oct 29, 2017 · 3 comments

Comments

@jbolila
Copy link

jbolila commented Oct 29, 2017

The presence of a message body in a request is signaled by a Content-Length or Transfer-Encoding header field.

https://tools.ietf.org/html/rfc7230#section-3.3

Without this some tools don't work well, for example wrk.

let data = String::from_utf8_lossy(request.body()).into_owned();
let body = format!(r#"{{"data": {}, "url": "{}"}}"#, request.uri().path(), data);
Ok(response
    .header("content-length", body.len().to_string().as_str())   //  ~:o
    .body(body.into_bytes())?)

Is valid code, although has some issues, the first one is the header is not set (or any other header).

< HTTP/1.1 200 OK
* no chunk, no close, no size. Assume close to signal end

Compared with the minimal out of the box set by the http module of node:

< HTTP/1.1 200 OK
< Date: Sun, 29 Oct 2017 08:19:45 GMT
< Connection: keep-alive
< Content-Length: 21

Can these values be set automatically?

@steveklabnik
Copy link
Owner

I believe #90 has fixed this, by setting it automatically. @jbolila , any chance you could give master a try and see if wrk works?

@scurest
Copy link
Contributor

scurest commented Mar 24, 2018

On master, examples/server.rs's response now looks like

< HTTP/1.1 200 OK
< content-length: 11
< 

so it does include the content length and curl doesn't produce * no chunk, no close, no size. Assume close to signal end anymore.

But the Date and Connection fields are still missing. Date would be nice and easy to add. I'm not sure what would need to change to make keep-alive work though.


about wrk:

In my tests, since wg/wrk@522ec60 (~3 yrs ago) wrk will compute Req/Sec even without the Content-Length header.

The server will invariably crash after wrk finishes though. Cause is a broken pipe error here

simple-server/src/lib.rs

Lines 265 to 266 in 10103f5

self.handle_connection(stream)
.expect("Error handling connection.");

@scurest
Copy link
Contributor

scurest commented Mar 24, 2018

Oh. but there still isn't Content-Length for these errors:

simple-server/src/lib.rs

Lines 318 to 321 in 10103f5

let resp = Response::builder()
.status(StatusCode::PAYLOAD_TOO_LARGE)
.body("<h1>413</h1><p>Request too large!<p>".as_bytes())
.unwrap();

simple-server/src/lib.rs

Lines 341 to 343 in 10103f5

let response = response_builder
.body("<h1>404</h1><p>Not found!<p>".as_bytes())
.unwrap();

simple-server/src/lib.rs

Lines 378 to 380 in 10103f5

let response = response_builder
.body("<h1>500</h1><p>Internal Server Error!<p>".as_bytes())
.unwrap();

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

No branches or pull requests

3 participants