fix(server): Drain requests on drop.

If a client sent an illegal request (like a GET request with a message
body), or if there was a legal request with a body but the Handler
didn't read all of it, the remaining bytes would be left in the stream.
The next request to come from the same client would error, as the server
would confuse the remaining bytes, and think the request was malformed.

Fixes #197
Fixes #309

Author: seanmonstar

src/http.rs

@@ -3,7 +3,9 @@ use std::borrow::Cow::{Borrowed, Owned};
 use std::borrow::IntoCow;
 use std::cmp::min;
 use std::old_io::{self, Reader, IoResult, BufWriter};
+use std::mem;
 use std::num::from_u16;
+use std::ptr;
 use std::str;
 use std::string::CowString;
 
@@ -20,7 +22,7 @@ use HttpError::{HttpHeaderError, HttpIoError, HttpMethodError, HttpStatusError,
                 HttpUriError, HttpVersionError};
 use HttpResult;
 
-use self::HttpReader::{SizedReader, ChunkedReader, EofReader, EmptyReader};
+use self::HttpReader::{SizedReader, ChunkedReader, EofReader};
 use self::HttpWriter::{ThroughWriter, ChunkedWriter, SizedWriter, EmptyWriter};
 
 /// Readers to handle different Transfer-Encodings.
@@ -47,22 +49,31 @@ 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> {
 
     /// Unwraps this HttpReader and returns the underlying Reader.
+    #[inline]
     pub fn unwrap(self) -> R {
-        match self {
-            SizedReader(r, _) => r,
-            ChunkedReader(r, _) => r,
-            EofReader(r) => r,
-            EmptyReader(r) => r,
-        }
+        let r = unsafe {
+            ptr::read(match self {
+                SizedReader(ref r, _) => r,
+                ChunkedReader(ref r, _) => r,
+                EofReader(ref r) => r,
+            })
+        };
+        unsafe { mem::forget(self); }
+        r
+    }
+}
+
+#[unsafe_destructor]
+impl<R: Reader> Drop for HttpReader<R> {
+    #[inline]
+    fn drop(&mut self) {
+        let mut buf = [0u8; 512];
+        while self.read(&mut buf).is_ok() {}
     }
 }
 
@@ -116,7 +127,6 @@ impl<R: Reader> Reader for HttpReader<R> {
             EofReader(ref mut body) => {
                 body.read(buf)
             },
-            EmptyReader(_) => Err(old_io::standard_error(old_io::EndOfFile))
         }
     }
 }

src/lib.rs

@@ -1,5 +1,5 @@
 #![feature(core, collections, hash, io, os, path, std_misc,
-           slicing_syntax, box_syntax)]
+           slicing_syntax, box_syntax, unsafe_destructor)]
 #![deny(missing_docs)]
 #![cfg_attr(test, deny(warnings))]
 #![cfg_attr(test, feature(alloc, test))]

src/server/request.rs

@@ -2,7 +2,7 @@
 //!
 //! These are requests that a `hyper::Server` receives, and include its method,
 //! target URI, headers, and message body.
-use std::old_io::IoResult;
+use std::old_io::{self, IoResult};
 use std::old_io::net::ip::SocketAddr;
 
 use {HttpResult};
@@ -11,7 +11,7 @@ use method::Method::{self, Get, Head};
 use header::{Headers, ContentLength, TransferEncoding};
 use http::{read_request_line};
 use http::HttpReader;
-use http::HttpReader::{SizedReader, ChunkedReader, EmptyReader};
+use http::HttpReader::{SizedReader, ChunkedReader};
 use uri::RequestUri;
 
 /// A request bundles several parts of an incoming `NetworkStream`, given to a `Handler`.
@@ -26,7 +26,7 @@ pub struct Request<'a> {
     pub uri: RequestUri,
     /// The version of HTTP for this request.
     pub version: HttpVersion,
-    body: HttpReader<&'a mut (Reader + 'a)>
+    body: Body<HttpReader<&'a mut (Reader + 'a)>>
 }
 
 
@@ -39,18 +39,19 @@ impl<'a> Request<'a> {
         let headers = try!(Headers::from_raw(&mut stream));
         debug!("{:?}", headers);
 
-        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!()
-            }
+        let body = if let Some(len) = headers.get::<ContentLength>() {
+            SizedReader(stream, **len)
         } else if headers.has::<TransferEncoding>() {
             todo!("check for Transfer-Encoding: chunked");
             ChunkedReader(stream, None)
         } else {
-            EmptyReader(stream)
+            SizedReader(stream, 0)
+        };
+
+        let body = if method == Get || method == Head {
+            Body::Empty(body)
+        } else {
+            Body::NonEmpty(body)
         };
 
         Ok(Request {
@@ -68,13 +69,31 @@ impl<'a> Request<'a> {
                                  RequestUri, HttpVersion,
                                  HttpReader<&'a mut (Reader + 'a)>,) {
         (self.remote_addr, self.method, self.headers,
-         self.uri, self.version, self.body)
+         self.uri, self.version, self.body.into_inner())
     }
 }
 
 impl<'a> Reader for Request<'a> {
+    #[inline]
     fn read(&mut self, buf: &mut [u8]) -> IoResult<usize> {
-        self.body.read(buf)
+        match self.body {
+            Body::Empty(..) => Err(old_io::standard_error(old_io::EndOfFile)),
+            Body::NonEmpty(ref mut r) => r.read(buf)
+        }
+    }
+}
+
+enum Body<R> {
+    Empty(R),
+    NonEmpty(R),
+}
+
+impl<R> Body<R> {
+    fn into_inner(self) -> R {
+        match self {
+            Body::Empty(r) => r,
+            Body::NonEmpty(r) => r
+        }
     }
 }
 
@@ -95,8 +114,9 @@ mod tests {
         let mut stream = MockStream::with_input(b"\
             GET / HTTP/1.1\r\n\
             Host: example.domain\r\n\
+            Content-Length: 18\r\n\
             \r\n\
-            I'm a bad request.\r\n\
+            I'm a bad request.\
         ");
 
         let mut req = Request::new(&mut stream, sock("127.0.0.1:80")).unwrap();
@@ -108,16 +128,17 @@ mod tests {
         let mut stream = MockStream::with_input(b"\
             HEAD / HTTP/1.1\r\n\
             Host: example.domain\r\n\
+            Content-Length: 18\r\n\
             \r\n\
-            I'm a bad request.\r\n\
+            I'm a bad request.\
         ");
 
         let mut req = Request::new(&mut stream, sock("127.0.0.1:80")).unwrap();
         assert_eq!(req.read_to_string(), Ok("".to_string()));
     }
 
     #[test]
-    fn test_post_empty_body() {
+    fn test_post_body_with_no_content_length() {
         let mut stream = MockStream::with_input(b"\
             POST / HTTP/1.1\r\n\
             Host: example.domain\r\n\
@@ -129,6 +150,20 @@ mod tests {
         assert_eq!(req.read_to_string(), Ok("".to_string()));
     }
 
+    #[test]
+    fn test_unexpected_body_drains_upon_drop() {
+        let mut stream = MockStream::with_input(b"\
+            GET / HTTP/1.1\r\n\
+            Host: example.domain\r\n\
+            Content-Length: 18\r\n\
+            \r\n\
+            I'm a bad request.\
+        ");
+
+        Request::new(&mut stream, sock("127.0.0.1:80")).unwrap().read_to_string().unwrap();
+        assert!(stream.read.eof());
+    }
+
     #[test]
     fn test_parse_chunked_request() {
         let mut stream = MockStream::with_input(b"\