Merge pull request #318 from hyperium/revert-310-server-request-drop

Revert "fix(server): Drain requests on drop."

Author: reem

src/http.rs

@@ -3,10 +3,7 @@ use std::borrow::Cow::{Borrowed, Owned};
 use std::borrow::IntoCow;
 use std::cmp::min;
 use std::old_io::{self, Reader, IoResult, BufWriter};
-use std::old_io::util as io_util;
-use std::mem;
 use std::num::from_u16;
-use std::ptr;
 use std::str;
 use std::string::CowString;
 
@@ -23,7 +20,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.
@@ -50,30 +47,22 @@ 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 {
-        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 _cant_use = io_util::copy(self, &mut io_util::NullWriter);
+        match self {
+            SizedReader(r, _) => r,
+            ChunkedReader(r, _) => r,
+            EofReader(r) => r,
+            EmptyReader(r) => r,
+        }
     }
 }
 
@@ -127,6 +116,7 @@ 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, unsafe_destructor)]
+           slicing_syntax, box_syntax)]
 #![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::{self, IoResult};
+use std::old_io::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};
+use http::HttpReader::{SizedReader, ChunkedReader, EmptyReader};
 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: Body<HttpReader<&'a mut (Reader + 'a)>>
+    body: HttpReader<&'a mut (Reader + 'a)>
 }
 
 
@@ -39,19 +39,18 @@ impl<'a> Request<'a> {
         let headers = try!(Headers::from_raw(&mut stream));
         debug!("{:?}", headers);
 
-        let body = if let Some(len) = headers.get::<ContentLength>() {
-            SizedReader(stream, **len)
+        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 if headers.has::<TransferEncoding>() {
             todo!("check for Transfer-Encoding: chunked");
             ChunkedReader(stream, None)
         } else {
-            SizedReader(stream, 0)
-        };
-
-        let body = if method == Get || method == Head {
-            Body::Empty(body)
-        } else {
-            Body::NonEmpty(body)
+            EmptyReader(stream)
         };
 
         Ok(Request {
@@ -69,31 +68,13 @@ impl<'a> Request<'a> {
                                  RequestUri, HttpVersion,
                                  HttpReader<&'a mut (Reader + 'a)>,) {
         (self.remote_addr, self.method, self.headers,
-         self.uri, self.version, self.body.into_inner())
+         self.uri, self.version, self.body)
     }
 }
 
 impl<'a> Reader for Request<'a> {
-    #[inline]
     fn read(&mut self, buf: &mut [u8]) -> IoResult<usize> {
-        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
-        }
+        self.body.read(buf)
     }
 }
 
@@ -114,9 +95,8 @@ 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.\
+            I'm a bad request.\r\n\
         ");
 
         let mut req = Request::new(&mut stream, sock("127.0.0.1:80")).unwrap();
@@ -128,17 +108,16 @@ 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.\
+            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("".to_string()));
     }
 
     #[test]
-    fn test_post_body_with_no_content_length() {
+    fn test_post_empty_body() {
         let mut stream = MockStream::with_input(b"\
             POST / HTTP/1.1\r\n\
             Host: example.domain\r\n\
@@ -150,20 +129,6 @@ 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"\