diff --git a/CHANGES.md b/CHANGES.md index 56813ce98..a8cfb8a26 100644 --- a/CHANGES.md +++ b/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 diff --git a/src/app.rs b/src/app.rs index e0f56a1ac..ccf2e88e0 100644 --- a/src/app.rs +++ b/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 { factory_ref: Rc>>, data: Vec>, data_factories: Vec, - config: AppConfigInner, external: Vec, _t: PhantomData, } @@ -52,7 +51,6 @@ impl App { 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))), } } } diff --git a/src/app_service.rs b/src/app_service.rs index bb42a3dd9..28e245228 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -41,7 +41,6 @@ where pub(crate) endpoint: T, pub(crate) data: Rc>>, pub(crate) data_factories: Rc>, - pub(crate) config: RefCell, pub(crate) services: Rc>>>, pub(crate) default: Option>, pub(crate) factory_ref: Rc>>, @@ -58,7 +57,7 @@ where InitError = (), >, { - type Config = (); + type Config = AppConfig; type Request = Request; type Response = ServiceResponse; type Error = T::Error; @@ -66,7 +65,7 @@ where type Service = AppInitService; type Future = AppInitResult; - 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()) diff --git a/src/config.rs b/src/config.rs index d57791e1a..6ce96767d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -124,14 +124,20 @@ impl AppService { } #[derive(Clone)] -pub struct AppConfig(pub(crate) Rc); +pub struct AppConfig(Rc); + +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(), + ) } } diff --git a/src/server.rs b/src/server.rs index d6835ffa8..72ef7255b 100644 --- a/src/server.rs +++ b/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, keep_alive: KeepAlive, client_timeout: u64, client_shutdown: u64, @@ -52,14 +57,13 @@ pub struct HttpServer where F: Fn() -> I + Send + Clone + 'static, I: IntoServiceFactory, - S: ServiceFactory, + S: ServiceFactory, S::Error: Into, S::InitError: fmt::Debug, S::Response: Into>, B: MessageBody, { pub(super) factory: F, - pub(super) host: Option, config: Arc>, backlog: i32, sockets: Vec, @@ -71,7 +75,7 @@ impl HttpServer where F: Fn() -> I + Send + Clone + 'static, I: IntoServiceFactory, - S: ServiceFactory, + S: ServiceFactory, S::Error: Into + 'static, S::InitError: fmt::Debug, S::Response: Into> + '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>(mut self, val: T) -> Self { - self.host = Some(val.as_ref().to_owned()); + pub fn server_hostname>(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,12 +497,13 @@ 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( @@ -483,12 +511,19 @@ where 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 HttpServer where F: Fn() -> I + Send + Clone + 'static, I: IntoServiceFactory, - S: ServiceFactory, + S: ServiceFactory, S::Error: Into, S::InitError: fmt::Debug, S::Response: Into>, diff --git a/src/test.rs b/src/test.rs index 1e2c0eec7..8de1ba141 100644 --- a/src/test.rs +++ b/src/test.rs @@ -12,7 +12,9 @@ use actix_http::{cookie::Cookie, ws, Extensions, HttpService, Request}; use actix_router::{Path, ResourceDef, Url}; use actix_rt::System; use actix_server::Server; -use actix_service::{IntoService, IntoServiceFactory, Service, ServiceFactory}; +use actix_service::{ + map_config, IntoService, IntoServiceFactory, Service, ServiceFactory, +}; use awc::error::PayloadError; use awc::{Client, ClientRequest, ClientResponse, Connector}; use bytes::{Bytes, BytesMut}; @@ -25,7 +27,7 @@ use serde_json; pub use actix_http::test::TestBuffer; -use crate::config::{AppConfig, AppConfigInner}; +use crate::config::AppConfig; use crate::data::Data; use crate::dev::{Body, MessageBody, Payload}; use crate::request::HttpRequestPool; @@ -79,7 +81,7 @@ pub async fn init_service( where R: IntoServiceFactory, S: ServiceFactory< - Config = (), + Config = AppConfig, Request = Request, Response = ServiceResponse, Error = E, @@ -87,7 +89,7 @@ where S::InitError: std::fmt::Debug, { let srv = app.into_factory(); - srv.new_service(()).await.unwrap() + srv.new_service(AppConfig::default()).await.unwrap() } /// Calls service and waits for response future completion. @@ -296,7 +298,7 @@ where pub struct TestRequest { req: HttpTestRequest, rmap: ResourceMap, - config: AppConfigInner, + config: AppConfig, path: Path, app_data: Extensions, } @@ -306,7 +308,7 @@ impl Default for TestRequest { TestRequest { req: HttpTestRequest::default(), rmap: ResourceMap::new(ResourceDef::new("")), - config: AppConfigInner::default(), + config: AppConfig::default(), path: Path::new(Url::new(Uri::default())), app_data: Extensions::new(), } @@ -462,7 +464,7 @@ impl TestRequest { head, payload, Rc::new(self.rmap), - AppConfig::new(self.config), + self.config.clone(), Rc::new(self.app_data), HttpRequestPool::create(), )) @@ -483,7 +485,7 @@ impl TestRequest { head, payload, Rc::new(self.rmap), - AppConfig::new(self.config), + self.config.clone(), Rc::new(self.app_data), HttpRequestPool::create(), ) @@ -499,7 +501,7 @@ impl TestRequest { head, Payload::None, Rc::new(self.rmap), - AppConfig::new(self.config), + self.config.clone(), Rc::new(self.app_data), HttpRequestPool::create(), ); @@ -538,7 +540,7 @@ pub fn start(factory: F) -> TestServer where F: Fn() -> I + Send + Clone + 'static, I: IntoServiceFactory, - S: ServiceFactory + 'static, + S: ServiceFactory + 'static, S::Error: Into + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, @@ -577,7 +579,7 @@ pub fn start_with(cfg: TestServerConfig, factory: F) -> TestServer where F: Fn() -> I + Send + Clone + 'static, I: IntoServiceFactory, - S: ServiceFactory + 'static, + S: ServiceFactory + 'static, S::Error: Into + 'static, S::InitError: fmt::Debug, S::Response: Into> + 'static, @@ -607,63 +609,87 @@ where match cfg.stream { StreamType::Tcp => match cfg.tp { HttpVer::Http1 => builder.listen("test", tcp, move || { + let cfg = + AppConfig::new(false, local_addr, format!("{}", local_addr)); HttpService::build() .client_timeout(ctimeout) - .h1(factory()) + .h1(map_config(factory().into_factory(), move |_| cfg.clone())) .tcp() }), HttpVer::Http2 => builder.listen("test", tcp, move || { + let cfg = + AppConfig::new(false, local_addr, format!("{}", local_addr)); HttpService::build() .client_timeout(ctimeout) - .h2(factory()) + .h2(map_config(factory().into_factory(), move |_| cfg.clone())) .tcp() }), HttpVer::Both => builder.listen("test", tcp, move || { + let cfg = + AppConfig::new(false, local_addr, format!("{}", local_addr)); HttpService::build() .client_timeout(ctimeout) - .finish(factory()) + .finish(map_config(factory().into_factory(), move |_| { + cfg.clone() + })) .tcp() }), }, #[cfg(feature = "openssl")] StreamType::Openssl(acceptor) => match cfg.tp { HttpVer::Http1 => builder.listen("test", tcp, move || { + let cfg = + AppConfig::new(true, local_addr, format!("{}", local_addr)); HttpService::build() .client_timeout(ctimeout) - .h1(factory()) + .h1(map_config(factory().into_factory(), move |_| cfg.clone())) .openssl(acceptor.clone()) }), HttpVer::Http2 => builder.listen("test", tcp, move || { + let cfg = + AppConfig::new(true, local_addr, format!("{}", local_addr)); HttpService::build() .client_timeout(ctimeout) - .h2(factory()) + .h2(map_config(factory().into_factory(), move |_| cfg.clone())) .openssl(acceptor.clone()) }), HttpVer::Both => builder.listen("test", tcp, move || { + let cfg = + AppConfig::new(true, local_addr, format!("{}", local_addr)); HttpService::build() .client_timeout(ctimeout) - .finish(factory()) + .finish(map_config(factory().into_factory(), move |_| { + cfg.clone() + })) .openssl(acceptor.clone()) }), }, #[cfg(feature = "rustls")] StreamType::Rustls(config) => match cfg.tp { HttpVer::Http1 => builder.listen("test", tcp, move || { + let cfg = + AppConfig::new(true, local_addr, format!("{}", local_addr)); HttpService::build() .client_timeout(ctimeout) - .h1(factory()) + .h1(map_config(factory().into_factory(), move |_| cfg.clone())) .rustls(config.clone()) }), HttpVer::Http2 => builder.listen("test", tcp, move || { + let cfg = + AppConfig::new(true, local_addr, format!("{}", local_addr)); HttpService::build() .client_timeout(ctimeout) - .h2(factory()) + .h2(map_config(factory().into_factory(), move |_| cfg.clone())) .rustls(config.clone()) }), HttpVer::Both => builder.listen("test", tcp, move || { + let cfg = + AppConfig::new(true, local_addr, format!("{}", local_addr)); HttpService::build() .client_timeout(ctimeout) - .finish(factory()) + .finish(map_config(factory().into_factory(), move |_| { + cfg.clone() + })) .rustls(config.clone()) }), }, diff --git a/tests/test_httpserver.rs b/tests/test_httpserver.rs index d19c46ee7..f345a70f0 100644 --- a/tests/test_httpserver.rs +++ b/tests/test_httpserver.rs @@ -5,8 +5,7 @@ use std::{net, thread, time::Duration}; #[cfg(feature = "openssl")] use open_ssl::ssl::SslAcceptorBuilder; -use actix_http::Response; -use actix_web::{web, App, HttpServer}; +use actix_web::{web, App, HttpRequest, HttpResponse, HttpServer}; fn unused_addr() -> net::SocketAddr { let addr: net::SocketAddr = "127.0.0.1:0".parse().unwrap(); @@ -28,7 +27,7 @@ async fn test_start() { let srv = HttpServer::new(|| { App::new().service( - web::resource("/").route(web::to(|| Response::Ok().body("test"))), + web::resource("/").route(web::to(|| HttpResponse::Ok().body("test"))), ) }) .workers(1) @@ -99,9 +98,10 @@ async fn test_start_ssl() { let builder = ssl_acceptor().unwrap(); let srv = HttpServer::new(|| { - App::new().service( - web::resource("/").route(web::to(|| Response::Ok().body("test"))), - ) + App::new().service(web::resource("/").route(web::to(|req: HttpRequest| { + assert!(req.app_config().secure()); + HttpResponse::Ok().body("test") + }))) }) .workers(1) .shutdown_timeout(1)