From 8bf7964875205155e3018902a6e8facee6c145b6 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Tue, 14 Nov 2017 11:52:29 -0800 Subject: [PATCH] fix(server): GET requests with no body have None instead of Empty Closes #1373 --- src/proto/dispatch.rs | 12 +++++------ src/proto/request.rs | 6 +++--- src/server/server_proto.rs | 8 ++++---- tests/server.rs | 42 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/src/proto/dispatch.rs b/src/proto/dispatch.rs index e24ae79b8d..b98f7eee02 100644 --- 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)>, ::Error>; - fn recv_msg(&mut self, msg: ::Result<(Self::RecvItem, Body)>) -> ::Result<()>; + fn recv_msg(&mut self, msg: ::Result<(Self::RecvItem, Option)>) -> ::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)>) -> ::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)>) -> ::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 index 8c3ef18367..1f02ec430f 100644 --- a/src/proto/request.rs +++ b/src/proto/request.rs @@ -168,16 +168,16 @@ impl From> for Request { } /// Constructs a request using a received ResponseHead and optional body -pub fn from_wire(addr: Option, incoming: RequestHead, body: B) -> Request { +pub fn from_wire(addr: Option, incoming: RequestHead, body: Option) -> Request { let MessageHead { version, subject: RequestLine(method, uri), headers } = incoming; - Request:: { + 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 index c18c0513c7..956a36661b 100644 --- a/src/server/server_proto.rs +++ b/src/server/server_proto.rs @@ -219,8 +219,8 @@ impl From> 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 Service for HttpService #[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 index c1da03accc..21d0f2d3cf 100644 --- 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::::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; + + 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";