AppConfig::secure() is always false. actix#1202

CHANGES.md

@@ -6,6 +6,11 @@
 
 * Move `BodyEncoding` to `dev` module #1220
 
+### Fixed
+
+* Fix `AppConfig::secure()` is always false. #1202
+
+
 ## [2.0.0-alpha.6] - 2019-12-15
 
 ### Fixed

src/app.rs

@@ -12,7 +12,7 @@ use actix_service::{
 use futures::future::{FutureExt, LocalBoxFuture};
 
 use crate::app_service::{AppEntry, AppInit, AppRoutingFactory};
-use crate::config::{AppConfig, AppConfigInner, ServiceConfig};
+use crate::config::ServiceConfig;
 use crate::data::{Data, DataFactory};
 use crate::dev::ResourceDef;
 use crate::error::Error;
@@ -36,7 +36,6 @@ pub struct App<T, B> {
     factory_ref: Rc<RefCell<Option<AppRoutingFactory>>>,
     data: Vec<Box<dyn DataFactory>>,
     data_factories: Vec<FnDataFactory>,
-    config: AppConfigInner,
     external: Vec<ResourceDef>,
     _t: PhantomData<B>,
 }
@@ -52,7 +51,6 @@ impl App<AppEntry, Body> {
             services: Vec::new(),
             default: None,
             factory_ref: fref,
-            config: AppConfigInner::default(),
             external: Vec::new(),
             _t: PhantomData,
         }
@@ -225,18 +223,6 @@ where
         self
     }
 
-    /// Set server host name.
-    ///
-    /// Host name is used by application router as a hostname for url generation.
-    /// Check [ConnectionInfo](./dev/struct.ConnectionInfo.html#method.host)
-    /// documentation for more information.
-    ///
-    /// By default host name is set to a "localhost" value.
-    pub fn hostname(mut self, val: &str) -> Self {
-        self.config.host = val.to_owned();
-        self
-    }
-
     /// Default service to be used if no matching resource could be found.
     ///
     /// It is possible to use services like `Resource`, `Route`.
@@ -383,7 +369,6 @@ where
             services: self.services,
             default: self.default,
             factory_ref: self.factory_ref,
-            config: self.config,
             external: self.external,
             _t: PhantomData,
         }
@@ -445,7 +430,6 @@ where
             services: self.services,
             default: self.default,
             factory_ref: self.factory_ref,
-            config: self.config,
             external: self.external,
             _t: PhantomData,
         }
@@ -472,7 +456,6 @@ where
             external: RefCell::new(self.external),
             default: self.default,
             factory_ref: self.factory_ref,
-            config: RefCell::new(AppConfig(Rc::new(self.config))),
         }
     }
 }

src/app_service.rs

@@ -41,7 +41,6 @@ where
     pub(crate) endpoint: T,
     pub(crate) data: Rc<Vec<Box<dyn DataFactory>>>,
     pub(crate) data_factories: Rc<Vec<FnDataFactory>>,
-    pub(crate) config: RefCell<AppConfig>,
     pub(crate) services: Rc<RefCell<Vec<Box<dyn AppServiceFactory>>>>,
     pub(crate) default: Option<Rc<HttpNewService>>,
     pub(crate) factory_ref: Rc<RefCell<Option<AppRoutingFactory>>>,
@@ -58,15 +57,15 @@ where
         InitError = (),
     >,
 {
-    type Config = ();
+    type Config = AppConfig;
     type Request = Request;
     type Response = ServiceResponse<B>;
     type Error = T::Error;
     type InitError = T::InitError;
     type Service = AppInitService<T::Service, B>;
     type Future = AppInitResult<T, B>;
 
-    fn new_service(&self, _: ()) -> Self::Future {
+    fn new_service(&self, config: AppConfig) -> Self::Future {
         // update resource default service
         let default = self.default.clone().unwrap_or_else(|| {
             Rc::new(boxed::factory(fn_service(|req: ServiceRequest| {
@@ -75,11 +74,7 @@ where
         });
 
         // App config
-        let mut config = AppService::new(
-            self.config.borrow().clone(),
-            default.clone(),
-            self.data.clone(),
-        );
+        let mut config = AppService::new(config, default.clone(), self.data.clone());
 
         // register services
         std::mem::replace(&mut *self.services.borrow_mut(), Vec::new())

src/config.rs

@@ -124,14 +124,20 @@ impl AppService {
 }
 
 #[derive(Clone)]
-pub struct AppConfig(pub(crate) Rc<AppConfigInner>);
+pub struct AppConfig(Rc<AppConfigInner>);
+
+struct AppConfigInner {
+    secure: bool,
+    host: String,
+    addr: SocketAddr,
+}
 
 impl AppConfig {
-    pub(crate) fn new(inner: AppConfigInner) -> Self {
-        AppConfig(Rc::new(inner))
+    pub(crate) fn new(secure: bool, addr: SocketAddr, host: String) -> Self {
+        AppConfig(Rc::new(AppConfigInner { secure, addr, host }))
     }
 
-    /// Set server host name.
+    /// Server host name.
     ///
     /// Host name is used by application router as a hostname for url generation.
     /// Check [ConnectionInfo](./struct.ConnectionInfo.html#method.host)
@@ -153,19 +159,13 @@ impl AppConfig {
     }
 }
 
-pub(crate) struct AppConfigInner {
-    pub(crate) secure: bool,
-    pub(crate) host: String,
-    pub(crate) addr: SocketAddr,
-}
-
-impl Default for AppConfigInner {
-    fn default() -> AppConfigInner {
-        AppConfigInner {
-            secure: false,
-            addr: "127.0.0.1:8080".parse().unwrap(),
-            host: "localhost:8080".to_owned(),
-        }
+impl Default for AppConfig {
+    fn default() -> Self {
+        AppConfig::new(
+            false,
+            "127.0.0.1:8080".parse().unwrap(),
+            "localhost:8080".to_owned(),
+        )
     }
 }
 

src/server.rs

@@ -6,7 +6,9 @@ use actix_http::{
     body::MessageBody, Error, HttpService, KeepAlive, Protocol, Request, Response,
 };
 use actix_server::{Server, ServerBuilder};
-use actix_service::{pipeline_factory, IntoServiceFactory, Service, ServiceFactory};
+use actix_service::{
+    map_config, pipeline_factory, IntoServiceFactory, Service, ServiceFactory,
+};
 use futures::future::ok;
 
 use net2::TcpBuilder;
@@ -16,12 +18,15 @@ use actix_tls::openssl::{AlpnError, SslAcceptor, SslAcceptorBuilder};
 #[cfg(feature = "rustls")]
 use actix_tls::rustls::ServerConfig as RustlsServerConfig;
 
+use crate::config::AppConfig;
+
 struct Socket {
     scheme: &'static str,
     addr: net::SocketAddr,
 }
 
 struct Config {
+    host: Option<String>,
     keep_alive: KeepAlive,
     client_timeout: u64,
     client_shutdown: u64,
@@ -52,14 +57,13 @@ pub struct HttpServer<F, I, S, B>
 where
     F: Fn() -> I + Send + Clone + 'static,
     I: IntoServiceFactory<S>,
-    S: ServiceFactory<Config = (), Request = Request>,
+    S: ServiceFactory<Config = AppConfig, Request = Request>,
     S::Error: Into<Error>,
     S::InitError: fmt::Debug,
     S::Response: Into<Response<B>>,
     B: MessageBody,
 {
     pub(super) factory: F,
-    pub(super) host: Option<String>,
     config: Arc<Mutex<Config>>,
     backlog: i32,
     sockets: Vec<Socket>,
@@ -71,7 +75,7 @@ impl<F, I, S, B> HttpServer<F, I, S, B>
 where
     F: Fn() -> I + Send + Clone + 'static,
     I: IntoServiceFactory<S>,
-    S: ServiceFactory<Config = (), Request = Request>,
+    S: ServiceFactory<Config = AppConfig, Request = Request>,
     S::Error: Into<Error> + 'static,
     S::InitError: fmt::Debug,
     S::Response: Into<Response<B>> + 'static,
@@ -82,8 +86,8 @@ where
     pub fn new(factory: F) -> Self {
         HttpServer {
             factory,
-            host: None,
             config: Arc::new(Mutex::new(Config {
+                host: None,
                 keep_alive: KeepAlive::Timeout(5),
                 client_timeout: 5000,
                 client_shutdown: 5000,
@@ -184,8 +188,8 @@ where
     /// documentation for more information.
     ///
     /// By default host name is set to a "localhost" value.
-    pub fn server_hostname<T: AsRef<str>>(mut self, val: T) -> Self {
-        self.host = Some(val.as_ref().to_owned());
+    pub fn server_hostname<T: AsRef<str>>(self, val: T) -> Self {
+        self.config.lock().unwrap().host = Some(val.as_ref().to_owned());
         self
     }
 
@@ -246,11 +250,17 @@ where
             lst,
             move || {
                 let c = cfg.lock().unwrap();
+                let cfg = AppConfig::new(
+                    false,
+                    addr,
+                    c.host.clone().unwrap_or_else(|| format!("{}", addr)),
+                );
+
                 HttpService::build()
                     .keep_alive(c.keep_alive)
                     .client_timeout(c.client_timeout)
                     .local_addr(addr)
-                    .finish(factory())
+                    .finish(map_config(factory().into_factory(), move |_| cfg.clone()))
                     .tcp()
             },
         )?;
@@ -288,11 +298,16 @@ where
             lst,
             move || {
                 let c = cfg.lock().unwrap();
+                let cfg = AppConfig::new(
+                    true,
+                    addr,
+                    c.host.clone().unwrap_or_else(|| format!("{}", addr)),
+                );
                 HttpService::build()
                     .keep_alive(c.keep_alive)
                     .client_timeout(c.client_timeout)
                     .client_disconnect(c.client_shutdown)
-                    .finish(factory())
+                    .finish(map_config(factory().into_factory(), move |_| cfg.clone()))
                     .openssl(acceptor.clone())
             },
         )?;
@@ -330,11 +345,16 @@ where
             lst,
             move || {
                 let c = cfg.lock().unwrap();
+                let cfg = AppConfig::new(
+                    true,
+                    addr,
+                    c.host.clone().unwrap_or_else(|| format!("{}", addr)),
+                );
                 HttpService::build()
                     .keep_alive(c.keep_alive)
                     .client_timeout(c.client_timeout)
                     .client_disconnect(c.client_shutdown)
-                    .finish(factory())
+                    .finish(map_config(factory().into_factory(), move |_| cfg.clone()))
                     .rustls(config.clone())
             },
         )?;
@@ -435,24 +455,31 @@ where
 
         let cfg = self.config.clone();
         let factory = self.factory.clone();
-        // todo duplicated:
+        let socket_addr = net::SocketAddr::new(
+            net::IpAddr::V4(net::Ipv4Addr::new(127, 0, 0, 1)),
+            8080,
+        );
         self.sockets.push(Socket {
             scheme: "http",
-            addr: net::SocketAddr::new(
-                net::IpAddr::V4(net::Ipv4Addr::new(127, 0, 0, 1)),
-                8080,
-            ),
+            addr: socket_addr,
         });
 
         let addr = format!("actix-web-service-{:?}", lst.local_addr()?);
 
         self.builder = self.builder.listen_uds(addr, lst, move || {
             let c = cfg.lock().unwrap();
+            let config = AppConfig::new(
+                false,
+                socket_addr,
+                c.host.clone().unwrap_or_else(|| format!("{}", socket_addr)),
+            );
             pipeline_factory(|io: UnixStream| ok((io, Protocol::Http1, None))).and_then(
                 HttpService::build()
                     .keep_alive(c.keep_alive)
                     .client_timeout(c.client_timeout)
-                    .finish(factory()),
+                    .finish(map_config(factory().into_factory(), move |_| {
+                        config.clone()
+                    })),
             )
         })?;
         Ok(self)
@@ -470,25 +497,33 @@ where
 
         let cfg = self.config.clone();
         let factory = self.factory.clone();
+        let socket_addr = net::SocketAddr::new(
+            net::IpAddr::V4(net::Ipv4Addr::new(127, 0, 0, 1)),
+            8080,
+        );
         self.sockets.push(Socket {
             scheme: "http",
-            addr: net::SocketAddr::new(
-                net::IpAddr::V4(net::Ipv4Addr::new(127, 0, 0, 1)),
-                8080,
-            ),
+            addr: socket_addr,
         });
 
         self.builder = self.builder.bind_uds(
             format!("actix-web-service-{:?}", addr.as_ref()),
             addr,
             move || {
                 let c = cfg.lock().unwrap();
+                let config = AppConfig::new(
+                    false,
+                    socket_addr,
+                    c.host.clone().unwrap_or_else(|| format!("{}", socket_addr)),
+                );
                 pipeline_factory(|io: UnixStream| ok((io, Protocol::Http1, None)))
                     .and_then(
                         HttpService::build()
                             .keep_alive(c.keep_alive)
                             .client_timeout(c.client_timeout)
-                            .finish(factory()),
+                            .finish(map_config(factory().into_factory(), move |_| {
+                                config.clone()
+                            })),
                     )
             },
         )?;
@@ -500,7 +535,7 @@ impl<F, I, S, B> HttpServer<F, I, S, B>
 where
     F: Fn() -> I + Send + Clone + 'static,
     I: IntoServiceFactory<S>,
-    S: ServiceFactory<Config = (), Request = Request>,
+    S: ServiceFactory<Config = AppConfig, Request = Request>,
     S::Error: Into<Error>,
     S::InitError: fmt::Debug,
     S::Response: Into<Response<B>>,