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.

Sign up for GitHub

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

Fix Content-Length and Transfer-Encoding #2432

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix Content-Length and Transfer-Encoding #2432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rustfmt.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
edition = "2018"
2 changes: 1 addition & 1 deletion src/body/body.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
Expand Down
137 changes: 78 additions & 59 deletions src/proto/h1/role.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!();
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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" {
Expand All @@ -479,7 +491,7 @@ impl Http1Transaction for Server {
value
);
}
continue 'headers;
continue;
}
}
}
Expand All @@ -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;
Expand All @@ -503,15 +519,17 @@ 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());
} else {
extend(dst, b", ");
extend(dst, value.as_bytes());
}
continue 'headers;
continue;
}
header::CONNECTION => {
if !is_last && headers::connection_close(&value) {
Expand All @@ -525,7 +543,7 @@ impl Http1Transaction for Server {
extend(dst, b", ");
extend(dst, value.as_bytes());
}
continue 'headers;
continue;
}
header::DATE => {
wrote_date = true;
Expand All @@ -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
Expand Down Expand Up @@ -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")]
Expand Down
19 changes: 19 additions & 0 deletions tests/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading