fix(server): GET requests with no body have None instead of Empty

seanmonstar · seanmonstar · commit 8bf796487520 · 2017-11-14T11:52:29.000-08:00
Closes #1373
diff --git a/src/proto/dispatch.rs b/src/proto/dispatch.rs
@@ -18,7 +18,7 @@ pub trait Dispatch {
     type PollBody;
     type RecvItem;
     fn poll_msg(&mut self) -> Poll<Option<(Self::PollItem, Option<Self::PollBody>)>, ::Error>;
-    fn recv_msg(&mut self, msg: ::Result<(Self::RecvItem, Body)>) -> ::Result<()>;
+    fn recv_msg(&mut self, msg: ::Result<(Self::RecvItem, Option<Body>)>) -> ::Result<()>;
     fn should_poll(&self) -> bool;
 }
 
@@ -60,9 +60,9 @@ where
                         let body = if has_body {
                             let (tx, rx) = super::Body::pair();
                             self.body_tx = Some(tx);
-                            rx
+                            Some(rx)
                         } else {
-                            Body::empty()
+                            None
                         };
                         self.dispatch.recv_msg(Ok((head, body))).expect("recv_msg with Ok shouldn't error");
                     },
@@ -253,7 +253,7 @@ where
         }
     }
 
-    fn recv_msg(&mut self, msg: ::Result<(Self::RecvItem, Body)>) -> ::Result<()> {
+    fn recv_msg(&mut self, msg: ::Result<(Self::RecvItem, Option<Body>)>) -> ::Result<()> {
         let (msg, body) = msg?;
         let req = super::request::from_wire(None, msg, body);
         self.in_flight = Some(self.service.call(req));
@@ -300,10 +300,10 @@ where
         }
     }
 
-    fn recv_msg(&mut self, msg: ::Result<(Self::RecvItem, Body)>) -> ::Result<()> {
+    fn recv_msg(&mut self, msg: ::Result<(Self::RecvItem, Option<Body>)>) -> ::Result<()> {
         match msg {
             Ok((msg, body)) => {
-                let res = super::response::from_wire(msg, Some(body));
+                let res = super::response::from_wire(msg, body);
                 let cb = self.callback.take().expect("recv_msg without callback");
                 let _ = cb.send(Ok(res));
                 Ok(())
diff --git a/src/proto/request.rs b/src/proto/request.rs
@@ -168,16 +168,16 @@ impl<B> From<http::Request<B>> for Request<B> {
 }
 
 /// Constructs a request using a received ResponseHead and optional body
-pub fn from_wire<B>(addr: Option<SocketAddr>, incoming: RequestHead, body: B) -> Request<B> {
+pub fn from_wire(addr: Option<SocketAddr>, incoming: RequestHead, body: Option<Body>) -> Request<Body> {
     let MessageHead { version, subject: RequestLine(method, uri), headers } = incoming;
 
-    Request::<B> {
+    Request {
         method: method,
         uri: uri,
         headers: headers,
         version: version,
         remote_addr: addr,
-        body: Some(body),
+        body: body,
         is_proxy: false,
     }
 }
diff --git a/src/server/server_proto.rs b/src/server/server_proto.rs
@@ -219,8 +219,8 @@ impl From<Message<__ProtoRequest, proto::TokioBody>> for Request {
     #[inline]
     fn from(message: Message<__ProtoRequest, proto::TokioBody>) -> Request {
         let (head, body) = match message {
-            Message::WithoutBody(head) => (head.0, proto::Body::empty()),
-            Message::WithBody(head, body) => (head.0, body.into()),
+            Message::WithoutBody(head) => (head.0, None),
+            Message::WithBody(head, body) => (head.0, Some(body.into())),
         };
         request::from_wire(None, head, body)
     }
@@ -256,8 +256,8 @@ impl<T, B> Service for HttpService<T>
     #[inline]
     fn call(&self, message: Self::Request) -> Self::Future {
         let (head, body) = match message {
-            Message::WithoutBody(head) => (head.0, proto::Body::empty()),
-            Message::WithBody(head, body) => (head.0, body.into()),
+            Message::WithoutBody(head) => (head.0, None),
+            Message::WithBody(head, body) => (head.0, Some(body.into())),
         };
         let req = request::from_wire(Some(self.remote_addr), head, body);
         self.inner.call(req).map(Into::into)
diff --git a/tests/server.rs b/tests/server.rs
@@ -21,6 +21,7 @@ use std::time::Duration;
 
 use hyper::server::{Http, Request, Response, Service, NewService};
 
+
 #[test]
 fn get_should_ignore_body() {
     let server = serve();
@@ -56,6 +57,47 @@ fn get_with_body() {
     assert_eq!(server.body(), b"I'm a good request.");
 }
 
+#[test]
+fn get_implicitly_empty() {
+    // See https://github.com/hyperium/hyper/issues/1373
+    let mut core = Core::new().unwrap();
+    let listener = TcpListener::bind(&"127.0.0.1:0".parse().unwrap(), &core.handle()).unwrap();
+    let addr = listener.local_addr().unwrap();
+
+    thread::spawn(move || {
+        let mut tcp = connect(&addr);
+        tcp.write_all(b"\
+            GET / HTTP/1.1\r\n\
+            Host: example.domain\r\n\
+            \r\n\
+        ").unwrap();
+    });
+
+    let fut = listener.incoming()
+        .into_future()
+        .map_err(|_| unreachable!())
+        .and_then(|(item, _incoming)| {
+            let (socket, _) = item.unwrap();
+            Http::<hyper::Chunk>::new().serve_connection(socket, GetImplicitlyEmpty)
+        });
+
+    core.run(fut).unwrap();
+
+    struct GetImplicitlyEmpty;
+
+    impl Service for GetImplicitlyEmpty {
+        type Request = Request;
+        type Response = Response;
+        type Error = hyper::Error;
+        type Future = FutureResult<Self::Response, Self::Error>;
+
+        fn call(&self, req: Request) -> Self::Future {
+            assert!(req.body_ref().is_none());
+            future::ok(Response::new())
+        }
+    }
+}
+
 #[test]
 fn get_fixed_response() {
     let foo_bar = b"foo bar baz";

Original file line number	Diff line number	Diff line change
`@@ -168,16 +168,16 @@ impl<B> From<http::Request<B>> for Request<B> {`
`168`	`168`	`}`
`169`	`169`
`170`	`170`	`/// Constructs a request using a received ResponseHead and optional body`
`171`		`-pub fn from_wire<B>(addr: Option<SocketAddr>, incoming: RequestHead, body: B) -> Request<B> {`
	`171`	`+pub fn from_wire(addr: Option<SocketAddr>, incoming: RequestHead, body: Option<Body>) -> Request<Body> {`
`172`	`172`	`let MessageHead { version, subject: RequestLine(method, uri), headers } = incoming;`
`173`	`173`
`174`		`- Request::<B> {`
	`174`	`+ Request {`
`175`	`175`	`method: method,`
`176`	`176`	`uri: uri,`
`177`	`177`	`headers: headers,`
`178`	`178`	`version: version,`
`179`	`179`	`remote_addr: addr,`
`180`		`- body: Some(body),`
	`180`	`+ body: body,`
`181`	`181`	`is_proxy: false,`
`182`	`182`	`}`
`183`	`183`	`}`