Skip to content

Commit

Permalink
fix(client): don't assume bodies on 204 and 304 Responses
Browse files Browse the repository at this point in the history
Closes #1242
  • Loading branch information
seanmonstar committed Jul 4, 2017
1 parent 7ce3121 commit 81c0d18
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 8 deletions.
4 changes: 2 additions & 2 deletions src/http/h1/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use self::Kind::{Length, Chunked, Eof};
///
/// If a message body does not include a Transfer-Encoding, it *should*
/// include a Content-Length header.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq)]
pub struct Decoder {
kind: Kind,
}
Expand All @@ -30,7 +30,7 @@ impl Decoder {
}
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq)]
enum Kind {
/// A Reader used when a Content-Length header is passed with a positive integer.
Length(u64),
Expand Down
49 changes: 43 additions & 6 deletions src/http/h1/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,21 @@ impl Http1Transaction for ClientTransaction {
// According to https://tools.ietf.org/html/rfc7230#section-3.3.3
// 1. HEAD responses, and Status 1xx, 204, and 304 cannot have a body.
// 2. Status 2xx to a CONNECT cannot have a body.
//
// First two steps taken care of before this method.
//
// 3. Transfer-Encoding: chunked has a chunked body.
// 4. If multiple differing Content-Length headers or invalid, close connection.
// 5. Content-Length header has a sized body.
// 6. Not Client.
// 7. Read till EOF.

//TODO: need a way to pass the Method that caused this Response

match inc.subject.0 {
100...199 |
204 |
304 => return Ok(Decoder::length(0)),
_ => (),
}

if let Some(&header::TransferEncoding(ref codings)) = inc.headers.get() {
if codings.last() == Some(&header::Encoding::Chunked) {
Ok(Decoder::chunked())
Expand Down Expand Up @@ -329,9 +336,11 @@ fn extend(dst: &mut Vec<u8>, data: &[u8]) {

#[cfg(test)]
mod tests {
use http::{ServerTransaction, ClientTransaction, Http1Transaction};
use bytes::BytesMut;

use http::{MessageHead, ServerTransaction, ClientTransaction, Http1Transaction};
use header::{ContentLength, TransferEncoding};

#[test]
fn test_parse_request() {
extern crate pretty_env_logger;
Expand Down Expand Up @@ -368,6 +377,7 @@ mod tests {
let mut raw = BytesMut::from(b"GET htt:p// HTTP/1.1\r\nHost: hyper.rs\r\n\r\n".to_vec());
ServerTransaction::parse(&mut raw).unwrap_err();
}

#[test]
fn test_parse_raw_status() {
let mut raw = BytesMut::from(b"HTTP/1.1 200 OK\r\n\r\n".to_vec());
Expand All @@ -379,6 +389,34 @@ mod tests {
assert_eq!(res.subject.1, "Howdy");
}

#[test]
fn test_decoder_response() {
use super::Decoder;

let mut head = MessageHead::<::http::RawStatus>::default();

head.subject.0 = 204;
assert_eq!(Decoder::length(0), ClientTransaction::decoder(&head).unwrap());
head.subject.0 = 304;
assert_eq!(Decoder::length(0), ClientTransaction::decoder(&head).unwrap());

head.subject.0 = 200;
assert_eq!(Decoder::eof(), ClientTransaction::decoder(&head).unwrap());

head.headers.set(TransferEncoding::chunked());
assert_eq!(Decoder::chunked(), ClientTransaction::decoder(&head).unwrap());

head.headers.remove::<TransferEncoding>();
head.headers.set(ContentLength(10));
assert_eq!(Decoder::length(10), ClientTransaction::decoder(&head).unwrap());

head.headers.set_raw("Content-Length", vec![b"5".to_vec(), b"5".to_vec()]);
assert_eq!(Decoder::length(5), ClientTransaction::decoder(&head).unwrap());

head.headers.set_raw("Content-Length", vec![b"10".to_vec(), b"11".to_vec()]);
ClientTransaction::decoder(&head).unwrap_err();
}

#[cfg(feature = "nightly")]
use test::Bencher;

Expand Down Expand Up @@ -424,8 +462,7 @@ mod tests {
#[cfg(feature = "nightly")]
#[bench]
fn bench_server_transaction_encode(b: &mut Bencher) {
use ::http::MessageHead;
use ::header::{Headers, ContentLength, ContentType};
use header::{Headers, ContentLength, ContentType};
use ::{StatusCode, HttpVersion};

let len = 108;
Expand Down

0 comments on commit 81c0d18

Please sign in to comment.