Merge pull request #147 from hyperium/empty-reader

feat(server): properly handle requests that shouldn't have bodies

src/http.rs

@@ -17,7 +17,7 @@ use HttpError::{HttpHeaderError, HttpIoError, HttpMethodError, HttpStatusError,
                 HttpUriError, HttpVersionError};
 use HttpResult;
 
-use self::HttpReader::{SizedReader, ChunkedReader, EofReader};
+use self::HttpReader::{SizedReader, ChunkedReader, EofReader, EmptyReader};
 use self::HttpWriter::{ThroughWriter, ChunkedWriter, SizedWriter, EmptyWriter};
 
 /// Readers to handle different Transfer-Encodings.
@@ -30,6 +30,7 @@ pub enum HttpReader<R> {
     /// A Reader used when Transfer-Encoding is `chunked`.
     ChunkedReader(R, Option<uint>),
     /// A Reader used for responses that don't indicate a length or chunked.
+    ///
     /// Note: This should only used for `Response`s. It is illegal for a
     /// `Request` to be made with both `Content-Length` and
     /// `Transfer-Encoding: chunked` missing, as explained from the spec:
@@ -43,6 +44,10 @@ pub enum HttpReader<R> {
     /// > reliably; the server MUST respond with the 400 (Bad Request)
     /// > status code and then close the connection.
     EofReader(R),
+    /// A Reader used for messages that should never have a body.
+    ///
+    /// See https://tools.ietf.org/html/rfc7230#section-3.3.3
+    EmptyReader(R),
 }
 
 impl<R: Reader> HttpReader<R> {
@@ -53,6 +58,7 @@ impl<R: Reader> HttpReader<R> {
             SizedReader(r, _) => r,
             ChunkedReader(r, _) => r,
             EofReader(r) => r,
+            EmptyReader(r) => r,
         }
     }
 }
@@ -106,7 +112,8 @@ impl<R: Reader> Reader for HttpReader<R> {
             },
             EofReader(ref mut body) => {
                 body.read(buf)
-            }
+            },
+            EmptyReader(_) => Err(io::standard_error(io::EndOfFile))
         }
     }
 }

src/mock.rs

@@ -40,6 +40,13 @@ impl MockStream {
             write: MemWriter::new(),
         }
     }
+
+    pub fn with_input(input: &[u8]) -> MockStream {
+        MockStream {
+            read: MemReader::new(input.to_vec()),
+            write: MemWriter::new(),
+        }
+    }
 }
 impl Reader for MockStream {
     fn read(&mut self, buf: &mut [u8]) -> IoResult<uint> {

src/server/request.rs

@@ -7,12 +7,12 @@ use std::io::net::ip::SocketAddr;
 
 use {HttpResult};
 use version::{HttpVersion};
-use method;
+use method::Method::{mod, Get, Head};
 use header::Headers;
-use header::common::ContentLength;
+use header::common::{ContentLength, TransferEncoding};
 use http::{read_request_line};
 use http::HttpReader;
-use http::HttpReader::{SizedReader, ChunkedReader};
+use http::HttpReader::{SizedReader, ChunkedReader, EmptyReader};
 use uri::RequestUri;
 
 pub type InternalReader<'a> = &'a mut Reader + 'a;
@@ -22,7 +22,7 @@ pub struct Request<'a> {
     /// The IP address of the remote connection.
     pub remote_addr: SocketAddr,
     /// The `Method`, such as `Get`, `Post`, etc.
-    pub method: method::Method,
+    pub method: Method,
     /// The headers of the incoming request.
     pub headers: Headers,
     /// The target request-uri for this request.
@@ -44,14 +44,18 @@ impl<'a> Request<'a> {
         debug!("Headers: [\n{}]", headers);
 
 
-        let body = if headers.has::<ContentLength>() {
+        let body = if method == Get || method == Head {
+            EmptyReader(stream)
+        } else if headers.has::<ContentLength>() {
             match headers.get::<ContentLength>() {
                 Some(&ContentLength(len)) => SizedReader(stream, len),
                 None => unreachable!()
             }
-        } else {
+        } else if headers.has::<TransferEncoding>() {
             todo!("check for Transfer-Encoding: chunked");
             ChunkedReader(stream, None)
+        } else {
+            EmptyReader(stream)
         };
 
         Ok(Request {
@@ -71,3 +75,51 @@ impl<'a> Reader for Request<'a> {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use mock::MockStream;
+    use super::Request;
+
+    macro_rules! sock(
+        ($s:expr) => (::std::str::from_str::<::std::io::net::ip::SocketAddr>($s).unwrap())
+    )
+
+    #[test]
+    fn test_get_empty_body() {
+        let mut stream = MockStream::with_input(b"\
+            GET / HTTP/1.1\r\n\
+            Host: example.domain\r\n\
+            \r\n\
+            I'm a bad request.\r\n\
+        ");
+
+        let mut req = Request::new(&mut stream, sock!("127.0.0.1:80")).unwrap();
+        assert_eq!(req.read_to_string(), Ok("".into_string()));
+    }
+
+    #[test]
+    fn test_head_empty_body() {
+        let mut stream = MockStream::with_input(b"\
+            HEAD / HTTP/1.1\r\n\
+            Host: example.domain\r\n\
+            \r\n\
+            I'm a bad request.\r\n\
+        ");
+
+        let mut req = Request::new(&mut stream, sock!("127.0.0.1:80")).unwrap();
+        assert_eq!(req.read_to_string(), Ok("".into_string()));
+    }
+
+    #[test]
+    fn test_post_empty_body() {
+        let mut stream = MockStream::with_input(b"\
+            POST / HTTP/1.1\r\n\
+            Host: example.domain\r\n\
+            \r\n\
+            I'm a bad request.\r\n\
+        ");
+
+        let mut req = Request::new(&mut stream, sock!("127.0.0.1:80")).unwrap();
+        assert_eq!(req.read_to_string(), Ok("".into_string()));
+    }
+}