various cleanups and comments

src/application.rs

@@ -1,4 +1,4 @@
-use std::cell::{RefCell, UnsafeCell};
+use std::cell::RefCell;
 use std::collections::HashMap;
 use std::rc::Rc;
 
@@ -21,7 +21,7 @@ pub struct HttpApplication<S = ()> {
     prefix: String,
     prefix_len: usize,
     router: Router,
-    inner: Rc<UnsafeCell<Inner<S>>>,
+    inner: Rc<RefCell<Inner<S>>>,
     filters: Option<Vec<Box<Predicate<S>>>>,
     middlewares: Rc<RefCell<Vec<Box<Middleware<S>>>>>,
 }
@@ -69,17 +69,12 @@ impl<S: 'static> PipelineHandler<S> for Inner<S> {
 }
 
 impl<S: 'static> HttpApplication<S> {
-    #[inline]
-    fn as_ref(&self) -> &Inner<S> {
-        unsafe { &*self.inner.get() }
-    }
-
     #[inline]
     fn get_handler(&self, req: &mut HttpRequest<S>) -> HandlerType {
         if let Some(idx) = self.router.recognize(req) {
             HandlerType::Normal(idx)
         } else {
-            let inner = self.as_ref();
+            let inner = self.inner.borrow();
             req.match_info_mut().set_tail(0);
 
             'outer: for idx in 0..inner.handlers.len() {
@@ -131,7 +126,7 @@ impl<S: 'static> HttpApplication<S> {
     #[cfg(test)]
     pub(crate) fn run(&mut self, mut req: HttpRequest<S>) -> AsyncResult<HttpResponse> {
         let tp = self.get_handler(&mut req);
-        unsafe { &mut *self.inner.get() }.handle(req, tp)
+        self.inner.borrow_mut().handle(req, tp)
     }
 
     #[cfg(test)]
@@ -340,24 +335,32 @@ where
         T: FromRequest<S> + 'static,
     {
         {
-            let parts: &mut ApplicationParts<S> = unsafe {
-                &mut *(self.parts.as_mut().expect("Use after finish") as *mut _)
-            };
+            let parts = self.parts.as_mut().expect("Use after finish");
 
             // get resource handler
-            for &mut (ref pattern, ref mut handler) in &mut parts.resources {
-                if let Some(ref mut handler) = *handler {
-                    if pattern.pattern() == path {
-                        handler.method(method).with(f);
-                        return self;
-                    }
+            let mut found = false;
+            for &mut (ref pattern, ref handler) in &mut parts.resources {
+                if handler.is_some() && pattern.pattern() == path {
+                    found = true;
+                    break;
                 }
             }
 
-            let mut handler = ResourceHandler::default();
-            handler.method(method).with(f);
-            let pattern = Resource::new(handler.get_name(), path);
-            parts.resources.push((pattern, Some(handler)));
+            if !found {
+                let mut handler = ResourceHandler::default();
+                handler.method(method).with(f);
+                let pattern = Resource::new(handler.get_name(), path);
+                parts.resources.push((pattern, Some(handler)));
+            } else {
+                for &mut (ref pattern, ref mut handler) in &mut parts.resources {
+                    if let Some(ref mut handler) = *handler {
+                        if pattern.pattern() == path {
+                            handler.method(method).with(f);
+                            break;
+                        }
+                    }
+                }
+            }
         }
         self
     }
@@ -626,7 +629,7 @@ where
 
         let (router, resources) = Router::new(&prefix, parts.settings, resources);
 
-        let inner = Rc::new(UnsafeCell::new(Inner {
+        let inner = Rc::new(RefCell::new(Inner {
             prefix: prefix_len,
             default: parts.default,
             encoding: parts.encoding,

src/error.rs

@@ -3,7 +3,7 @@ use std::io::Error as IoError;
 use std::str::Utf8Error;
 use std::string::FromUtf8Error;
 use std::sync::Mutex;
-use std::{fmt, io, mem, result};
+use std::{fmt, io, result};
 
 use actix::MailboxError;
 use cookie;
@@ -22,7 +22,6 @@ pub use url::ParseError as UrlParseError;
 // re-exports
 pub use cookie::ParseError as CookieParseError;
 
-use body::Body;
 use handler::Responder;
 use httprequest::HttpRequest;
 use httpresponse::{HttpResponse, InnerHttpResponse};
@@ -671,16 +670,7 @@ impl<T> InternalError<T> {
     /// Create `InternalError` with predefined `HttpResponse`.
     pub fn from_response(cause: T, response: HttpResponse) -> Self {
         let mut resp = response.into_inner();
-        let body = mem::replace(&mut resp.body, Body::Empty);
-        match body {
-            Body::Empty => (),
-            Body::Binary(mut bin) => {
-                resp.body = Body::Binary(bin.take().into());
-            }
-            Body::Streaming(_) | Body::Actor(_) => {
-                error!("Streaming or Actor body is not support by error response");
-            }
-        }
+        resp.drop_unsupported_body();
 
         InternalError {
             cause,

src/header/shared/entity.rs

@@ -161,7 +161,7 @@ impl IntoHeaderValue for EntityTag {
     fn try_into(self) -> Result<HeaderValue, Self::Error> {
         let mut wrt = Writer::new();
         write!(wrt, "{}", self).unwrap();
-        unsafe { Ok(HeaderValue::from_shared_unchecked(wrt.take())) }
+        HeaderValue::from_shared(wrt.take())
     }
 }
 

src/header/shared/httpdate.rs

@@ -64,11 +64,7 @@ impl IntoHeaderValue for HttpDate {
     fn try_into(self) -> Result<HeaderValue, Self::Error> {
         let mut wrt = BytesMut::with_capacity(29).writer();
         write!(wrt, "{}", self.0.rfc822()).unwrap();
-        unsafe {
-            Ok(HeaderValue::from_shared_unchecked(
-                wrt.get_mut().take().freeze(),
-            ))
-        }
+        HeaderValue::from_shared(wrt.get_mut().take().freeze())
     }
 }
 

src/httpresponse.rs

@@ -894,7 +894,9 @@ pub(crate) struct InnerHttpResponse {
     error: Option<Error>,
 }
 
+/// This is here only because `failure::Fail: Send + Sync` which looks insane to me
 unsafe impl Sync for InnerHttpResponse {}
+/// This is here only because `failure::Fail: Send + Sync` which looks insane to me
 unsafe impl Send for InnerHttpResponse {}
 
 impl InnerHttpResponse {
@@ -914,6 +916,20 @@ impl InnerHttpResponse {
             error: None,
         }
     }
+
+    /// This is for failure, we can not have Send + Sync on Streaming and Actor response
+    pub(crate) fn drop_unsupported_body(&mut self) {
+        let body = mem::replace(&mut self.body, Body::Empty);
+        match body {
+            Body::Empty => (),
+            Body::Binary(mut bin) => {
+                self.body = Body::Binary(bin.take().into());
+            }
+            Body::Streaming(_) | Body::Actor(_) => {
+                error!("Streaming or Actor body is not support by error response");
+            }
+        }
+    }
 }
 
 /// Internal use only! unsafe

src/param.rs

@@ -1,10 +1,11 @@
-use http::StatusCode;
-use smallvec::SmallVec;
 use std;
 use std::ops::Index;
 use std::path::PathBuf;
 use std::str::FromStr;
 
+use http::StatusCode;
+use smallvec::SmallVec;
+
 use error::{InternalError, ResponseError, UriSegmentError};
 use uri::Url;
 

src/pipeline.rs

@@ -124,7 +124,7 @@ impl<S> PipelineInfo<S> {
 impl<S: 'static, H: PipelineHandler<S>> Pipeline<S, H> {
     pub fn new(
         req: HttpRequest<S>, mws: Rc<RefCell<Vec<Box<Middleware<S>>>>>,
-        handler: Rc<UnsafeCell<H>>, htype: HandlerType,
+        handler: Rc<RefCell<H>>, htype: HandlerType,
     ) -> Pipeline<S, H> {
         let mut info = PipelineInfo {
             mws,
@@ -133,7 +133,7 @@ impl<S: 'static, H: PipelineHandler<S>> Pipeline<S, H> {
             error: None,
             context: None,
             disconnected: None,
-            encoding: unsafe { &*handler.get() }.encoding(),
+            encoding: handler.borrow().encoding(),
         };
         let state = StartMiddlewares::init(&mut info, handler, htype);
 
@@ -171,13 +171,12 @@ impl<S: 'static, H: PipelineHandler<S>> HttpHandlerTask for Pipeline<S, H> {
     }
 
     fn poll_io(&mut self, io: &mut Writer) -> Poll<bool, Error> {
-        let info: &mut PipelineInfo<_> = unsafe { &mut *(&mut self.0 as *mut _) };
+        let mut state = mem::replace(&mut self.1, PipelineState::None);
 
         loop {
-            if self.1.is_response() {
-                let state = mem::replace(&mut self.1, PipelineState::None);
+            if state.is_response() {
                 if let PipelineState::Response(st) = state {
-                    match st.poll_io(io, info) {
+                    match st.poll_io(io, &mut self.0) {
                         Ok(state) => {
                             self.1 = state;
                             if let Some(error) = self.0.error.take() {
@@ -193,7 +192,7 @@ impl<S: 'static, H: PipelineHandler<S>> HttpHandlerTask for Pipeline<S, H> {
                     }
                 }
             }
-            match self.1 {
+            match state {
                 PipelineState::None => return Ok(Async::Ready(true)),
                 PipelineState::Error => {
                     return Err(
@@ -203,27 +202,32 @@ impl<S: 'static, H: PipelineHandler<S>> HttpHandlerTask for Pipeline<S, H> {
                 _ => (),
             }
 
-            match self.1.poll(info) {
-                Some(state) => self.1 = state,
-                None => return Ok(Async::NotReady),
+            match state.poll(&mut self.0) {
+                Some(st) => state = st,
+                None => {
+                    return {
+                        self.1 = state;
+                        Ok(Async::NotReady)
+                    }
+                }
             }
         }
     }
 
     fn poll_completed(&mut self) -> Poll<(), Error> {
-        let info: &mut PipelineInfo<_> = unsafe { &mut *(&mut self.0 as *mut _) };
-
+        let mut state = mem::replace(&mut self.1, PipelineState::None);
         loop {
-            match self.1 {
+            match state {
                 PipelineState::None | PipelineState::Error => {
                     return Ok(Async::Ready(()))
                 }
                 _ => (),
             }
 
-            if let Some(state) = self.1.poll(info) {
-                self.1 = state;
+            if let Some(st) = state.poll(&mut self.0) {
+                state = st;
             } else {
+                self.1 = state;
                 return Ok(Async::NotReady);
             }
         }
@@ -234,7 +238,7 @@ type Fut = Box<Future<Item = Option<HttpResponse>, Error = Error>>;
 
 /// Middlewares start executor
 struct StartMiddlewares<S, H> {
-    hnd: Rc<UnsafeCell<H>>,
+    hnd: Rc<RefCell<H>>,
     htype: HandlerType,
     fut: Option<Fut>,
     _s: PhantomData<S>,
@@ -242,14 +246,14 @@ struct StartMiddlewares<S, H> {
 
 impl<S: 'static, H: PipelineHandler<S>> StartMiddlewares<S, H> {
     fn init(
-        info: &mut PipelineInfo<S>, hnd: Rc<UnsafeCell<H>>, htype: HandlerType,
+        info: &mut PipelineInfo<S>, hnd: Rc<RefCell<H>>, htype: HandlerType,
     ) -> PipelineState<S, H> {
         // execute middlewares, we need this stage because middlewares could be
         // non-async and we can move to next state immediately
         let len = info.mws.borrow().len() as u16;
         loop {
             if info.count == len {
-                let reply = unsafe { &mut *hnd.get() }.handle(info.req().clone(), htype);
+                let reply = hnd.borrow_mut().handle(info.req().clone(), htype);
                 return WaitingResponse::init(info, reply);
             } else {
                 let state =
@@ -285,7 +289,9 @@ impl<S: 'static, H: PipelineHandler<S>> StartMiddlewares<S, H> {
                     }
                     loop {
                         if info.count == len {
-                            let reply = unsafe { &mut *self.hnd.get() }
+                            let reply = self
+                                .hnd
+                                .borrow_mut()
                                 .handle(info.req().clone(), self.htype);
                             return Some(WaitingResponse::init(info, reply));
                         } else {

src/router.rs

@@ -1,9 +1,9 @@
 use std::collections::HashMap;
 use std::hash::{Hash, Hasher};
-use std::mem;
 use std::rc::Rc;
 
 use regex::{escape, Regex};
+use smallvec::SmallVec;
 
 use error::UrlGenerationError;
 use httprequest::HttpRequest;
@@ -264,9 +264,9 @@ impl Resource {
     pub fn match_with_params<S>(
         &self, req: &mut HttpRequest<S>, plen: usize, insert: bool,
     ) -> bool {
-        let mut segments: [ParamItem; 24] = unsafe { mem::uninitialized() };
+        let mut segments: SmallVec<[ParamItem; 5]> = SmallVec::new();
 
-        let (names, segments_len) = {
+        let names = {
             let path = &req.path()[plen..];
             if insert {
                 if path.is_empty() {
@@ -282,7 +282,6 @@ impl Resource {
                 PatternType::Static(ref s) => return s == path,
                 PatternType::Dynamic(ref re, ref names, _) => {
                     if let Some(captures) = re.captures(path) {
-                        let mut idx = 0;
                         let mut passed = false;
                         for capture in captures.iter() {
                             if let Some(ref m) = capture {
@@ -290,14 +289,13 @@ impl Resource {
                                     passed = true;
                                     continue;
                                 }
-                                segments[idx] = ParamItem::UrlSegment(
+                                segments.push(ParamItem::UrlSegment(
                                     (plen + m.start()) as u16,
                                     (plen + m.end()) as u16,
-                                );
-                                idx += 1;
+                                ));
                             }
                         }
-                        (names, idx)
+                        names
                     } else {
                         return false;
                     }
@@ -309,9 +307,11 @@ impl Resource {
         let len = req.path().len();
         let params = req.match_info_mut();
         params.set_tail(len as u16);
-        for idx in 0..segments_len {
+        for (idx, segment) in segments.iter().enumerate() {
+            // reason: Router is part of App, which is unique per thread
+            // app is alive during whole life of tthread
             let name = unsafe { &*(names[idx].as_str() as *const _) };
-            params.add(name, segments[idx]);
+            params.add(name, *segment);
         }
         true
     }
@@ -320,9 +320,9 @@ impl Resource {
     pub fn match_prefix_with_params<S>(
         &self, req: &mut HttpRequest<S>, plen: usize,
     ) -> Option<usize> {
-        let mut segments: [ParamItem; 24] = unsafe { mem::uninitialized() };
+        let mut segments: SmallVec<[ParamItem; 5]> = SmallVec::new();
 
-        let (names, segments_len, tail_len) = {
+        let (names, tail_len) = {
             let path = &req.path()[plen..];
             let path = if path.is_empty() { "/" } else { path };
 
@@ -334,7 +334,6 @@ impl Resource {
                 },
                 PatternType::Dynamic(ref re, ref names, len) => {
                     if let Some(captures) = re.captures(path) {
-                        let mut idx = 0;
                         let mut pos = 0;
                         let mut passed = false;
                         for capture in captures.iter() {
@@ -344,15 +343,14 @@ impl Resource {
                                     continue;
                                 }
 
-                                segments[idx] = ParamItem::UrlSegment(
+                                segments.push(ParamItem::UrlSegment(
                                     (plen + m.start()) as u16,
                                     (plen + m.end()) as u16,
-                                );
-                                idx += 1;
+                                ));
                                 pos = m.end();
                             }
                         }
-                        (names, idx, pos + len)
+                        (names, pos + len)
                     } else {
                         return None;
                     }
@@ -378,9 +376,11 @@ impl Resource {
 
         let params = req.match_info_mut();
         params.set_tail(tail_len as u16);
-        for idx in 0..segments_len {
+        for (idx, segment) in segments.iter().enumerate() {
+            // reason: Router is part of App, which is unique per thread
+            // app is alive during whole life of tthread
             let name = unsafe { &*(names[idx].as_str() as *const _) };
-            params.add(name, segments[idx]);
+            params.add(name, *segment);
         }
         Some(tail_len)
     }

src/scope.rs

@@ -226,27 +226,35 @@ impl<S: 'static> Scope<S> {
         R: Responder + 'static,
         T: FromRequest<S> + 'static,
     {
-        // get resource handler
-        let slf: &Scope<S> = unsafe { &*(&self as *const _) };
-        for &(ref pattern, ref resource) in slf.resources.iter() {
+        // check if we have resource handler
+        let mut found = false;
+        for &(ref pattern, _) in self.resources.iter() {
             if pattern.pattern() == path {
-                resource.borrow_mut().method(method).with(f);
-                return self;
+                found = true;
+                break;
             }
         }
 
-        let mut handler = ResourceHandler::default();
-        handler.method(method).with(f);
-        let pattern = Resource::with_prefix(
-            handler.get_name(),
-            path,
-            if path.is_empty() { "" } else { "/" },
-            false,
-        );
-        Rc::get_mut(&mut self.resources)
-            .expect("Can not use after configuration")
-            .push((pattern, Rc::new(RefCell::new(handler))));
-
+        if found {
+            for &(ref pattern, ref resource) in self.resources.iter() {
+                if pattern.pattern() == path {
+                    resource.borrow_mut().method(method).with(f);
+                    break;
+                }
+            }
+        } else {
+            let mut handler = ResourceHandler::default();
+            handler.method(method).with(f);
+            let pattern = Resource::with_prefix(
+                handler.get_name(),
+                path,
+                if path.is_empty() { "" } else { "/" },
+                false,
+            );
+            Rc::get_mut(&mut self.resources)
+                .expect("Can not use after configuration")
+                .push((pattern, Rc::new(RefCell::new(handler))));
+        }
         self
     }
 

src/ws/mask.rs

@@ -7,7 +7,7 @@ use std::ptr::copy_nonoverlapping;
 /// Mask/unmask a frame.
 #[inline]
 pub fn apply_mask(buf: &mut [u8], mask: u32) {
-    apply_mask_fast32(buf, mask)
+    unsafe { apply_mask_fast32(buf, mask) }
 }
 
 /// A safe unoptimized mask application.
@@ -20,9 +20,11 @@ fn apply_mask_fallback(buf: &mut [u8], mask: &[u8; 4]) {
 }
 
 /// Faster version of `apply_mask()` which operates on 8-byte blocks.
+///
+/// unsafe because uses pointer math and bit operations for performance
 #[inline]
 #[cfg_attr(feature = "cargo-clippy", allow(cast_lossless))]
-fn apply_mask_fast32(buf: &mut [u8], mask_u32: u32) {
+unsafe fn apply_mask_fast32(buf: &mut [u8], mask_u32: u32) {
     let mut ptr = buf.as_mut_ptr();
     let mut len = buf.len();
 
@@ -32,10 +34,8 @@ fn apply_mask_fast32(buf: &mut [u8], mask_u32: u32) {
         let n = if head > 4 { head - 4 } else { head };
 
         let mask_u32 = if n > 0 {
-            unsafe {
-                xor_mem(ptr, mask_u32, n);
-                ptr = ptr.offset(head as isize);
-            }
+            xor_mem(ptr, mask_u32, n);
+            ptr = ptr.offset(head as isize);
             len -= n;
             if cfg!(target_endian = "big") {
                 mask_u32.rotate_left(8 * n as u32)
@@ -47,11 +47,9 @@ fn apply_mask_fast32(buf: &mut [u8], mask_u32: u32) {
         };
 
         if head > 4 {
-            unsafe {
-                *(ptr as *mut u32) ^= mask_u32;
-                ptr = ptr.offset(4);
-                len -= 4;
-            }
+            *(ptr as *mut u32) ^= mask_u32;
+            ptr = ptr.offset(4);
+            len -= 4;
         }
         mask_u32
     } else {
@@ -68,27 +66,21 @@ fn apply_mask_fast32(buf: &mut [u8], mask_u32: u32) {
         mask_u64 = mask_u64 << 32 | mask_u32 as u64;
 
         while len >= 8 {
-            unsafe {
-                *(ptr as *mut u64) ^= mask_u64;
-                ptr = ptr.offset(8);
-                len -= 8;
-            }
+            *(ptr as *mut u64) ^= mask_u64;
+            ptr = ptr.offset(8);
+            len -= 8;
         }
     }
 
     while len >= 4 {
-        unsafe {
-            *(ptr as *mut u32) ^= mask_u32;
-            ptr = ptr.offset(4);
-            len -= 4;
-        }
+        *(ptr as *mut u32) ^= mask_u32;
+        ptr = ptr.offset(4);
+        len -= 4;
     }
 
     // Possible last block.
     if len > 0 {
-        unsafe {
-            xor_mem(ptr, mask_u32, len);
-        }
+        xor_mem(ptr, mask_u32, len);
     }
 }
 
@@ -107,7 +99,7 @@ unsafe fn xor_mem(ptr: *mut u8, mask: u32, len: usize) {
 
 #[cfg(test)]
 mod tests {
-    use super::{apply_mask_fallback, apply_mask_fast32};
+    use super::{apply_mask, apply_mask_fallback};
     use std::ptr;
 
     #[test]
@@ -126,7 +118,7 @@ mod tests {
             apply_mask_fallback(&mut masked, &mask);
 
             let mut masked_fast = unmasked.clone();
-            apply_mask_fast32(&mut masked_fast, mask_u32);
+            apply_mask(&mut masked_fast, mask_u32);
 
             assert_eq!(masked, masked_fast);
         }
@@ -137,7 +129,7 @@ mod tests {
             apply_mask_fallback(&mut masked[1..], &mask);
 
             let mut masked_fast = unmasked.clone();
-            apply_mask_fast32(&mut masked_fast[1..], mask_u32);
+            apply_mask(&mut masked_fast[1..], mask_u32);
 
             assert_eq!(masked, masked_fast);
         }