Merge pull request #923 from jwilm/fix-stack-overflow

fix(http): stackoverflow in Conn::ready
hyperium · Oct 7, 2016 · 588ef9d · 588ef9d
2 parents 8672ec5 + c32d0e9
commit 588ef9d
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 20 deletions.
diff --git a/src/client/mod.rs b/src/client/mod.rs
@@ -14,7 +14,7 @@ use std::time::Duration;
 use rotor::{self, Scope, EventSet, PollOpt};
 
 use header::Host;
-use http::{self, Next, RequestHead};
+use http::{self, Next, RequestHead, ReadyResult};
 use net::Transport;
 use uri::RequestUri;
 use {Url};
@@ -467,9 +467,16 @@ where C: Connect,
     fn ready(self, events: EventSet, scope: &mut Scope<Self::Context>) -> rotor::Response<Self, Self::Seed> {
         match self {
             ClientFsm::Socket(conn) => {
-                let res = conn.ready(events, scope);
-                let now = scope.now();
-                conn_response!(scope, res, now)
+                let mut conn = Some(conn);
+                loop {
+                    match conn.take().unwrap().ready(events, scope) {
+                        ReadyResult::Done(res) => {
+                            let now = scope.now();
+                            return conn_response!(scope, res, now);
+                        },
+                        ReadyResult::Continue(c) => conn = Some(c),
+                    }
+                }
             },
             ClientFsm::Connecting(mut seed) => {
                 if events.is_error() || events.is_hup() {

diff --git a/src/http/conn.rs b/src/http/conn.rs
@@ -496,6 +496,11 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> ConnInner<K, T, H> {
 
 }
 
+pub enum ReadyResult<C> {
+    Continue(C),
+    Done(Option<(C, Option<Duration>)>)
+}
+
 impl<K: Key, T: Transport, H: MessageHandler<T>> Conn<K, T, H> {
     pub fn new(key: K, transport: T, next: Next, notify: rotor::Notifier) -> Conn<K, T, H> {
         Conn(Box::new(ConnInner {
@@ -516,8 +521,13 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> Conn<K, T, H> {
         self
     }
 
-    pub fn ready<F>(mut self, events: EventSet, scope: &mut Scope<F>) -> Option<(Self, Option<Duration>)>
-    where F: MessageHandlerFactory<K, T, Output=H> {
+    pub fn ready<F>(
+        mut self,
+        events: EventSet,
+        scope: &mut Scope<F>
+    ) -> ReadyResult<Self>
+        where F: MessageHandlerFactory<K, T, Output=H>
+    {
         trace!("Conn::ready events='{:?}', blocked={:?}", events, self.0.transport.blocked());
 
         if events.is_error() {
@@ -579,24 +589,24 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> Conn<K, T, H> {
                 trace!("removing transport");
                 let _ = scope.deregister(&self.0.transport);
                 self.on_remove();
-                return None;
+                return ReadyResult::Done(None);
             },
         };
 
         if events.is_readable() && self.0.can_read_more(was_init) {
-            return self.ready(events, scope);
+            return ReadyResult::Continue(self);
         }
 
         trace!("scope.reregister({:?})", events);
         match scope.reregister(&self.0.transport, events, PollOpt::level()) {
             Ok(..) => {
                 let timeout = self.0.state.timeout();
-                Some((self, timeout))
+                ReadyResult::Done(Some((self, timeout)))
             },
             Err(e) => {
                 trace!("error reregistering: {:?}", e);
                 self.0.on_error(e.into(), &**scope);
-                None
+                ReadyResult::Done(None)
             }
         }
     }
@@ -607,14 +617,27 @@ impl<K: Key, T: Transport, H: MessageHandler<T>> Conn<K, T, H> {
             trace!("woke up with {:?}", next);
             self.0.state.update(next, &**scope);
         }
-        self.ready(EventSet::readable() | EventSet::writable(), scope)
+
+        let mut conn = Some(self);
+        loop {
+            match conn.take().unwrap().ready(EventSet::readable() | EventSet::writable(), scope) {
+                ReadyResult::Done(val) => return val,
+                ReadyResult::Continue(c) => conn = Some(c),
+            }
+        }
     }
 
     pub fn timeout<F>(mut self, scope: &mut Scope<F>) -> Option<(Self, Option<Duration>)>
     where F: MessageHandlerFactory<K, T, Output=H> {
         //TODO: check if this was a spurious timeout?
         self.0.on_error(::Error::Timeout, &**scope);
-        self.ready(EventSet::none(), scope)
+        let mut conn = Some(self);
+        loop {
+            match conn.take().unwrap().ready(EventSet::none(), scope) {
+                ReadyResult::Done(val) => return val,
+                ReadyResult::Continue(c) => conn = Some(c),
+            }
+        }
     }
 
     fn on_remove(self) {

diff --git a/src/http/mod.rs b/src/http/mod.rs
@@ -17,7 +17,7 @@ use version::HttpVersion::{Http10, Http11};
 #[cfg(feature = "serde-serialization")]
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
-pub use self::conn::{Conn, MessageHandler, MessageHandlerFactory, Seed, Key};
+pub use self::conn::{Conn, MessageHandler, MessageHandlerFactory, Seed, Key, ReadyResult};
 
 mod buffer;
 pub mod channel;

diff --git a/src/server/mod.rs b/src/server/mod.rs
@@ -14,7 +14,7 @@ use rotor::{self, Scope};
 pub use self::request::Request;
 pub use self::response::Response;
 
-use http::{self, Next};
+use http::{self, Next, ReadyResult};
 
 pub use net::{Accept, HttpListener, HttpsListener};
 use net::{SslServer, Transport};
@@ -248,13 +248,21 @@ where A: Accept,
                 }
             },
             ServerFsm::Conn(conn) => {
-                match conn.ready(events, scope) {
-                    Some((conn, None)) => rotor::Response::ok(ServerFsm::Conn(conn)),
-                    Some((conn, Some(dur))) => {
-                        rotor::Response::ok(ServerFsm::Conn(conn))
-                            .deadline(scope.now() + dur)
+                let mut conn = Some(conn);
+                loop {
+                    match conn.take().unwrap().ready(events, scope) {
+                        ReadyResult::Continue(c) => conn = Some(c),
+                        ReadyResult::Done(res) => {
+                            return match res {
+                                Some((conn, None)) => rotor::Response::ok(ServerFsm::Conn(conn)),
+                                Some((conn, Some(dur))) => {
+                                    rotor::Response::ok(ServerFsm::Conn(conn))
+                                        .deadline(scope.now() + dur)
+                                }
+                                None => rotor::Response::done()
+                            };
+                        }
                     }
-                    None => rotor::Response::done()
                 }
             }
         }