diff --git a/rustfmt.toml b/rustfmt.toml
new file mode 100644
index 0000000000..32a9786fa1
--- /dev/null
+++ b/rustfmt.toml
@@ -0,0 +1 @@
+edition = "2018"
diff --git a/src/body/body.rs b/src/body/body.rs
index 1be2c1b0c2..fe9bf11dea 100644
--- a/src/body/body.rs
+++ b/src/body/body.rs
@@ -480,7 +480,7 @@ impl From<Bytes> for Body {
#[inline]
fn from(chunk: Bytes) -> Body {
if chunk.is_empty() {
- Body::empty()
+ Body::new(Kind::Once(Some(Bytes::new()))) // drop chunk ASAP
} else {
Body::new(Kind::Once(Some(chunk)))
}
diff --git a/src/proto/h1/role.rs b/src/proto/h1/role.rs
index a9f2f0074f..144a06eed0 100644
--- a/src/proto/h1/role.rs
+++ b/src/proto/h1/role.rs
@@ -385,7 +385,7 @@ impl Http1Transaction for Server {
}};
}
- 'headers: for (opt_name, value) in msg.head.headers.drain() {
+ for (opt_name, value) in msg.head.headers.drain() {
if let Some(n) = opt_name {
cur_name = Some(n);
handle_is_name_written!();
@@ -399,6 +399,14 @@ impl Http1Transaction for Server {
rewind(dst);
return Err(crate::Error::new_user_header());
}
+ // Can this response even contain Content-Length?
+ if !Server::can_have_body(msg.req_method, msg.head.subject) {
+ warn!(
+ "illegal content-length ({:?}) seen for {:?} response to {:?} request",
+ value, msg.head.subject, msg.req_method
+ );
+ continue;
+ }
match msg.body {
Some(BodyLength::Known(known_len)) => {
// The HttpBody claims to know a length, and
@@ -421,13 +429,15 @@ impl Http1Transaction for Server {
}
if !is_name_written {
- encoder = Encoder::length(known_len);
+ if !Server::imply_body_only(msg.req_method, msg.head.subject) {
+ encoder = Encoder::length(known_len);
+ }
extend(dst, b"content-length: ");
extend(dst, value.as_bytes());
wrote_len = true;
is_name_written = true;
}
- continue 'headers;
+ continue;
}
Some(BodyLength::Unknown) => {
// The HttpBody impl didn't know how long the
@@ -446,16 +456,18 @@ impl Http1Transaction for Server {
return Err(crate::Error::new_user_header());
}
debug_assert!(is_name_written);
- continue 'headers;
+ continue;
} else {
// we haven't written content-length yet!
- encoder = Encoder::length(len);
+ if !Server::imply_body_only(msg.req_method, msg.head.subject) {
+ encoder = Encoder::length(len);
+ }
extend(dst, b"content-length: ");
extend(dst, value.as_bytes());
wrote_len = true;
is_name_written = true;
prev_con_len = Some(len);
- continue 'headers;
+ continue;
}
} else {
warn!("illegal Content-Length value: {:?}", value);
@@ -464,13 +476,13 @@ impl Http1Transaction for Server {
}
}
None => {
- // We have no body to actually send,
- // but the headers claim a content-length.
- // There's only 2 ways this makes sense:
+ // We have no body to actually send, but the headers claim a
+ // content-length. There are 3 ways this makes sense:
//
// - The header says the length is `0`.
+ // - The response is NOT_MODIFIED
// - This is a response to a `HEAD` request.
- if msg.req_method == &Some(Method::HEAD) {
+ if Server::imply_body_only(msg.req_method, msg.head.subject) {
debug_assert_eq!(encoder, Encoder::length(0));
} else {
if value.as_bytes() != b"0" {
@@ -479,7 +491,7 @@ impl Http1Transaction for Server {
value
);
}
- continue 'headers;
+ continue;
}
}
}
@@ -492,9 +504,13 @@ impl Http1Transaction for Server {
return Err(crate::Error::new_user_header());
}
// check that we actually can send a chunked body...
- if msg.head.version == Version::HTTP_10
- || !Server::can_chunked(msg.req_method, msg.head.subject)
- {
+ if msg.head.version == Version::HTTP_10 {
+ continue;
+ } else if !Server::can_have_body(msg.req_method, msg.head.subject) {
+ warn!(
+ "illegal transfer-encoding ({:?}) seen for {:?} response to {:?} request",
+ value, msg.head.subject, msg.req_method
+ );
continue;
}
wrote_len = true;
@@ -503,7 +519,9 @@ impl Http1Transaction for Server {
must_write_chunked = !headers::is_chunked_(&value);
if !is_name_written {
- encoder = Encoder::chunked();
+ if !Server::imply_body_only(msg.req_method, msg.head.subject) {
+ encoder = Encoder::chunked();
+ }
is_name_written = true;
extend(dst, b"transfer-encoding: ");
extend(dst, value.as_bytes());
@@ -511,7 +529,7 @@ impl Http1Transaction for Server {
extend(dst, b", ");
extend(dst, value.as_bytes());
}
- continue 'headers;
+ continue;
}
header::CONNECTION => {
if !is_last && headers::connection_close(&value) {
@@ -525,7 +543,7 @@ impl Http1Transaction for Server {
extend(dst, b", ");
extend(dst, value.as_bytes());
}
- continue 'headers;
+ continue;
}
header::DATE => {
wrote_date = true;
@@ -549,44 +567,50 @@ impl Http1Transaction for Server {
handle_is_name_written!();
- if !wrote_len {
- encoder = match msg.body {
- Some(BodyLength::Unknown) => {
- if msg.head.version == Version::HTTP_10
- || !Server::can_chunked(msg.req_method, msg.head.subject)
- {
- Encoder::close_delimited()
- } else {
- extend(dst, b"transfer-encoding: chunked\r\n");
- Encoder::chunked()
- }
+ // We should provide either a Content-Length or Transfer-Encoding header based on what we
+ // know about the Body, unless:
+ // - the request was HEAD and the body is Body::empty(), because a service may simply not
+ // provide generate the real body for a HEAD request.
+ // - the response is a 304, like a HEAD, may imply things about a particular body, but a
+ // service probably did not provide the full body in its Response.
+ // - the response must neither contain nor imply a body.
+ if !wrote_len && Server::can_have_body(msg.req_method, msg.head.subject) {
+ match msg.body {
+ Some(BodyLength::Known(0)) => extend(dst, b"content-length: 0\r\n"),
+ Some(BodyLength::Known(len)) => {
+ extend(dst, b"content-length: ");
+ let _ = ::itoa::write(&mut dst, len);
+ extend(dst, b"\r\n");
}
- None | Some(BodyLength::Known(0)) => {
- if msg.head.subject != StatusCode::NOT_MODIFIED {
+ None => {
+ if !Server::imply_body_only(msg.req_method, msg.head.subject) {
extend(dst, b"content-length: 0\r\n");
}
- Encoder::length(0)
}
- Some(BodyLength::Known(len)) => {
- if msg.head.subject == StatusCode::NOT_MODIFIED {
- Encoder::length(0)
- } else {
- extend(dst, b"content-length: ");
- let _ = ::itoa::write(&mut dst, len);
- extend(dst, b"\r\n");
- Encoder::length(len)
+ Some(BodyLength::Unknown) => {
+ if msg.head.version != Version::HTTP_10 {
+ extend(dst, b"transfer-encoding: chunked\r\n");
}
}
};
+
+ if !Server::imply_body_only(msg.req_method, msg.head.subject) {
+ encoder = match msg.body {
+ Some(BodyLength::Unknown) if msg.head.version == Version::HTTP_10 => {
+ Encoder::close_delimited()
+ }
+ Some(BodyLength::Unknown) => Encoder::chunked(),
+ Some(BodyLength::Known(len)) => Encoder::length(len),
+ None => Encoder::length(0),
+ };
+ }
}
- if !Server::can_have_body(msg.req_method, msg.head.subject) {
- trace!(
- "server body forced to 0; method={:?}, status={:?}",
- msg.req_method,
- msg.head.subject
- );
- encoder = Encoder::length(0);
+ #[cfg(debug_assertions)]
+ if Server::imply_body_only(msg.req_method, msg.head.subject)
+ || !Server::can_have_body(msg.req_method, msg.head.subject)
+ {
+ assert_eq!(encoder, Encoder::length(0));
}
// cached date is much faster than formatting every request
@@ -631,24 +655,19 @@ impl Http1Transaction for Server {
#[cfg(feature = "server")]
impl Server {
fn can_have_body(method: &Option<Method>, status: StatusCode) -> bool {
- Server::can_chunked(method, status)
- }
-
- fn can_chunked(method: &Option<Method>, status: StatusCode) -> bool {
- if method == &Some(Method::HEAD) || method == &Some(Method::CONNECT) && status.is_success()
+ if method == &Some(Method::CONNECT) && status.is_success()
+ || status == StatusCode::NO_CONTENT
+ || status.is_informational()
{
false
} else {
- match status {
- // TODO: support for 1xx codes needs improvement everywhere
- // would be 100...199 => false
- StatusCode::SWITCHING_PROTOCOLS
- | StatusCode::NO_CONTENT
- | StatusCode::NOT_MODIFIED => false,
- _ => true,
- }
+ true
}
}
+
+ fn imply_body_only(method: &Option<Method>, status: StatusCode) -> bool {
+ method == &Some(Method::HEAD) || status == StatusCode::NOT_MODIFIED
+ }
}
#[cfg(feature = "client")]
diff --git a/tests/client.rs b/tests/client.rs
index 479500ab3e..f12e316252 100644
--- a/tests/client.rs
+++ b/tests/client.rs
@@ -359,6 +359,25 @@ test! {
expected: "GET / HTTP/1.1\r\nhost: {addr}\r\n\r\n",
reply: REPLY_OK,
+ client:
+ request: {
+ method: GET,
+ url: "http://{addr}/",
+ // body is Body::empty
+ },
+ response:
+ status: OK,
+ headers: {},
+ body: None,
+}
+
+test! {
+ name: client_get_req_body_explicitly_empty,
+
+ server:
+ expected: "GET / HTTP/1.1\r\nhost: {addr}\r\ncontent-length: 0\r\n\r\n",
+ reply: REPLY_OK,
+
client:
request: {
method: GET,
diff --git a/tests/server.rs b/tests/server.rs
index 72d2e459d8..f0f3fc9901 100644
--- a/tests/server.rs
+++ b/tests/server.rs
@@ -102,13 +102,13 @@ mod response_body_lengths {
let body_str = match case.body {
Bd::Known(b) => {
- reply.body(b);
+ reply.body_bytes(b);
b
}
Bd::Unknown(b) => {
let (mut tx, body) = hyper::Body::channel();
tx.try_send_data(b.into()).expect("try_send_data");
- reply.body_stream(body);
+ reply.body(body);
b
}
};
@@ -322,7 +322,7 @@ mod response_body_lengths {
let server = serve();
let addr_str = format!("http://{}", server.addr());
- server.reply().body("Hello, World!");
+ server.reply().body_bytes("Hello, World!");
let client = Client::builder()
.http2_only(true)
@@ -345,7 +345,7 @@ mod response_body_lengths {
server
.reply()
.header("content-length", "10")
- .body("Hello, World!");
+ .body_bytes("Hello, World!");
let client = Client::builder()
.http2_only(true)
@@ -368,7 +368,7 @@ fn get_chunked_response_with_ka() {
server
.reply()
.header("transfer-encoding", "chunked")
- .body(foo_bar);
+ .body_bytes(foo_bar);
let mut req = connect(server.addr());
req.write_all(
b"\
@@ -388,7 +388,7 @@ fn get_chunked_response_with_ka() {
server
.reply()
.header("content-length", quux.len().to_string())
- .body(quux);
+ .body_bytes(quux);
req.write_all(
b"\
GET /quux HTTP/1.1\r\n\
@@ -484,7 +484,7 @@ fn head_response_doesnt_send_body() {
let _ = pretty_env_logger::try_init();
let foo_bar = b"foo bar baz";
let server = serve();
- server.reply().body(foo_bar);
+ server.reply().body_bytes(foo_bar);
let mut req = connect(server.addr());
req.write_all(
b"\
@@ -515,7 +515,7 @@ fn response_does_not_set_chunked_if_body_not_allowed() {
let server = serve();
server
.reply()
- .status(hyper::StatusCode::NOT_MODIFIED)
+ .status(hyper::StatusCode::NO_CONTENT)
.header("transfer-encoding", "chunked");
let mut req = connect(server.addr());
req.write_all(
@@ -534,7 +534,7 @@ fn response_does_not_set_chunked_if_body_not_allowed() {
assert!(!response.contains("transfer-encoding"));
let mut lines = response.lines();
- assert_eq!(lines.next(), Some("HTTP/1.1 304 Not Modified"));
+ assert_eq!(lines.next(), Some("HTTP/1.1 204 No Content"));
// no body or 0\r\n\r\n
let mut lines = lines.skip_while(|line| !line.is_empty());
@@ -549,7 +549,7 @@ fn keep_alive() {
server
.reply()
.header("content-length", foo_bar.len().to_string())
- .body(foo_bar);
+ .body_bytes(foo_bar);
let mut req = connect(server.addr());
req.write_all(
b"\
@@ -568,7 +568,7 @@ fn keep_alive() {
server
.reply()
.header("content-length", quux.len().to_string())
- .body(quux);
+ .body_bytes(quux);
req.write_all(
b"\
GET /quux HTTP/1.1\r\n\
@@ -590,7 +590,7 @@ fn http_10_keep_alive() {
server
.reply()
.header("content-length", foo_bar.len().to_string())
- .body(foo_bar);
+ .body_bytes(foo_bar);
let mut req = connect(server.addr());
req.write_all(
b"\
@@ -618,7 +618,7 @@ fn http_10_keep_alive() {
server
.reply()
.header("content-length", quux.len().to_string())
- .body(quux);
+ .body_bytes(quux);
req.write_all(
b"\
GET /quux HTTP/1.0\r\n\
@@ -641,7 +641,7 @@ fn http_10_close_on_no_ka() {
.reply()
.version(Version::HTTP_10)
.header("content-length", foo_bar.len().to_string())
- .body(foo_bar);
+ .body_bytes(foo_bar);
let mut req = connect(server.addr());
// The client request with version 1.0 that may have the keep-alive header
@@ -676,7 +676,7 @@ fn disable_keep_alive() {
server
.reply()
.header("content-length", foo_bar.len().to_string())
- .body(foo_bar);
+ .body_bytes(foo_bar);
let mut req = connect(server.addr());
req.write_all(
b"\
@@ -703,7 +703,7 @@ fn header_connection_close() {
.reply()
.header("content-length", foo_bar.len().to_string())
.header("connection", "close")
- .body(foo_bar);
+ .body_bytes(foo_bar);
let mut req = connect(server.addr());
req.write_all(
b"\
@@ -844,11 +844,11 @@ fn pipeline_disabled() {
server
.reply()
.header("content-length", "12")
- .body("Hello World!");
+ .body_bytes("Hello World!");
server
.reply()
.header("content-length", "12")
- .body("Hello World!");
+ .body_bytes("Hello World!");
req.write_all(
b"\
@@ -891,11 +891,11 @@ fn pipeline_enabled() {
server
.reply()
.header("content-length", "12")
- .body("Hello World\n");
+ .body_bytes("Hello World\n");
server
.reply()
.header("content-length", "12")
- .body("Hello World\n");
+ .body_bytes("Hello World\n");
req.write_all(
b"\
@@ -1580,7 +1580,7 @@ fn streaming_body() {
static S: &[&[u8]] = &[&[b'x'; 1_000] as &[u8]; 1_00] as _;
let b = futures_util::stream::iter(S.iter()).map(|&s| Ok::<_, hyper::Error>(s));
let b = hyper::Body::wrap_stream(b);
- server.reply().body_stream(b);
+ server.reply().body(b);
let mut tcp = connect(server.addr());
tcp.write_all(b"GET / HTTP/1.1\r\n\r\n").unwrap();
@@ -1689,7 +1689,7 @@ fn http2_body_user_error_sends_reset_reason() {
)));
let b = hyper::Body::wrap_stream(b);
- server.reply().body_stream(b);
+ server.reply().body(b);
let rt = support::runtime();
@@ -1775,12 +1775,12 @@ async fn http2_service_poll_ready_error_sends_goaway() {
}
#[test]
-fn skips_content_length_for_304_responses() {
+fn keeps_content_length_and_drops_body_for_304_responses_no_cl() {
let server = serve();
server
.reply()
.status(hyper::StatusCode::NOT_MODIFIED)
- .body("foo");
+ .body(Body::empty());
let mut req = connect(server.addr());
req.write_all(
b"\
@@ -1795,15 +1795,21 @@ fn skips_content_length_for_304_responses() {
let mut response = String::new();
req.read_to_string(&mut response).unwrap();
assert!(!response.contains("content-length:"));
+ let mut lines = response.lines();
+ assert_eq!(lines.next(), Some("HTTP/1.1 304 Not Modified"));
+
+ let mut lines = lines.skip_while(|line| !line.is_empty());
+ assert_eq!(lines.next(), Some(""));
+ assert_eq!(lines.next(), None);
}
#[test]
-fn skips_content_length_and_body_for_304_responses() {
+fn keeps_content_length_and_drops_body_for_304_responses() {
let server = serve();
server
.reply()
.status(hyper::StatusCode::NOT_MODIFIED)
- .body("foo");
+ .body_bytes("foo");
let mut req = connect(server.addr());
req.write_all(
b"\
@@ -1817,7 +1823,7 @@ fn skips_content_length_and_body_for_304_responses() {
let mut response = String::new();
req.read_to_string(&mut response).unwrap();
- assert!(!response.contains("content-length:"));
+ assert!(response.contains("content-length: 3"));
let mut lines = response.lines();
assert_eq!(lines.next(), Some("HTTP/1.1 304 Not Modified"));
@@ -1826,6 +1832,35 @@ fn skips_content_length_and_body_for_304_responses() {
assert_eq!(lines.next(), None);
}
+#[test]
+fn skips_content_length_and_body_for_204_response() {
+ let server = serve();
+ server
+ .reply()
+ .status(hyper::StatusCode::NO_CONTENT)
+ .body_bytes("foo");
+ let mut req = connect(server.addr());
+ req.write_all(
+ b"\
+ GET / HTTP/1.1\r\n\
+ Host: example.domain\r\n\
+ Connection: close\r\n\
+ \r\n\
+ ",
+ )
+ .unwrap();
+
+ let mut response = String::new();
+ req.read_to_string(&mut response).unwrap();
+ assert!(!response.contains("content-length:"));
+ let mut lines = response.lines();
+ assert_eq!(lines.next(), Some("HTTP/1.1 204 No Content"));
+
+ let mut lines = lines.skip_while(|line| !line.is_empty());
+ assert_eq!(lines.next(), Some(""));
+ assert_eq!(lines.next(), None);
+}
+
#[tokio::test]
async fn http2_keep_alive_detects_unresponsive_client() {
let _ = pretty_env_logger::try_init();
@@ -2076,15 +2111,11 @@ impl<'a> ReplyBuilder<'a> {
self
}
- fn body<T: AsRef<[u8]>>(self, body: T) {
- self.tx
- .lock()
- .unwrap()
- .send(Reply::Body(body.as_ref().to_vec().into()))
- .unwrap();
+ fn body_bytes<T: AsRef<[u8]>>(self, body: T) {
+ self.body(body.as_ref().to_vec().into());
}
- fn body_stream(self, body: Body) {
+ fn body(self, body: Body) {
self.tx.lock().unwrap().send(Reply::Body(body)).unwrap();
}