From 54678308d0da4e917ab1cb98a62c986044e5d824 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Sat, 9 Mar 2019 14:06:24 -0800 Subject: [PATCH] propogate app config with http request; add tests for url_for --- Cargo.toml | 3 +- actix-files/src/lib.rs | 41 ++++----- actix-files/src/named.rs | 2 - actix-session/src/cookie.rs | 10 ++- actix-web-codegen/src/lib.rs | 6 +- src/app.rs | 61 +++++++------- src/app_service.rs | 59 ++++++------- src/config.rs | 106 +++++++++++++++++------- src/error.rs | 2 + src/info.rs | 61 ++++++-------- src/lib.rs | 12 +-- src/request.rs | 156 +++++++++++++++++++++++++++++++---- src/resource.rs | 20 ++++- src/rmap.rs | 15 ++-- src/scope.rs | 4 +- src/service.rs | 18 ++-- src/state.rs | 2 +- src/test.rs | 34 +++++--- 18 files changed, 397 insertions(+), 215 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dbc0a65d2..9c3408251 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,9 +70,10 @@ actix-service = { git = "https://github.com/actix/actix-net.git" } actix-utils = { git = "https://github.com/actix/actix-net.git" } actix-http = { git = "https://github.com/actix/actix-http.git" } -actix-router = { git = "https://github.com/actix/actix-net.git" } +#actix-router = { git = "https://github.com/actix/actix-net.git" } actix-server = { git = "https://github.com/actix/actix-net.git" } actix-server-config = { git = "https://github.com/actix/actix-net.git" } +actix-router = { path = "../actix-net/router" } bytes = "0.4" derive_more = "0.14" diff --git a/actix-files/src/lib.rs b/actix-files/src/lib.rs index 17efdd813..14c25be79 100644 --- a/actix-files/src/lib.rs +++ b/actix-files/src/lib.rs @@ -17,7 +17,7 @@ use v_htmlescape::escape as escape_html_entity; use actix_http::error::{Error, ErrorInternalServerError}; use actix_service::{boxed::BoxedNewService, NewService, Service}; -use actix_web::dev::{self, AppConfig, HttpServiceFactory, ResourceDef, Url}; +use actix_web::dev::{HttpServiceFactory, ResourceDef, ServiceConfig}; use actix_web::{ blocking, FromRequest, HttpRequest, HttpResponse, Responder, ServiceFromRequest, ServiceRequest, ServiceResponse, @@ -305,7 +305,7 @@ where P: 'static, C: StaticFileConfig + 'static, { - fn register(self, config: &mut AppConfig

) { + fn register(self, config: &mut ServiceConfig

) { if self.default.borrow().is_none() { *self.default.borrow_mut() = Some(config.default_service()); } @@ -314,11 +314,12 @@ where } else { ResourceDef::prefix(&self.path) }; - config.register_service(rdef, None, self) + config.register_service(rdef, None, self, None) } } -impl NewService> for Files { +impl NewService for Files { + type Request = ServiceRequest

; type Response = ServiceResponse; type Error = (); type Service = FilesService; @@ -350,7 +351,8 @@ pub struct FilesService { _cd_map: PhantomData, } -impl Service> for FilesService { +impl Service for FilesService { + type Request = ServiceRequest

; type Response = ServiceResponse; type Error = (); type Future = FutureResult; @@ -362,7 +364,7 @@ impl Service> for FilesService { fn call(&mut self, req: ServiceRequest

) -> Self::Future { let (req, _) = req.into_parts(); - let real_path = match PathBufWrp::get_pathbuf(req.match_info()) { + let real_path = match PathBufWrp::get_pathbuf(req.match_info().path()) { Ok(item) => item, Err(e) => return ok(ServiceResponse::from_err(e, req.clone())), }; @@ -409,13 +411,13 @@ impl Service> for FilesService { } } +#[derive(Debug)] struct PathBufWrp(PathBuf); impl PathBufWrp { - fn get_pathbuf(path: &dev::Path) -> Result { - let path_str = path.path(); + fn get_pathbuf(path: &str) -> Result { let mut buf = PathBuf::new(); - for segment in path_str.split('/') { + for segment in path.split('/') { if segment == ".." { buf.pop(); } else if segment.starts_with('.') { @@ -447,13 +449,14 @@ impl

FromRequest

for PathBufWrp { type Config = (); fn from_request(req: &mut ServiceFromRequest

) -> Self::Future { - PathBufWrp::get_pathbuf(req.match_info()) + PathBufWrp::get_pathbuf(req.match_info().path()) } } #[cfg(test)] mod tests { use std::fs; + use std::iter::FromIterator; use std::ops::Add; use std::time::{Duration, SystemTime}; @@ -1093,32 +1096,32 @@ mod tests { #[test] fn test_path_buf() { assert_eq!( - PathBuf::from_param("/test/.tt"), + PathBufWrp::get_pathbuf("/test/.tt").map(|t| t.0), Err(UriSegmentError::BadStart('.')) ); assert_eq!( - PathBuf::from_param("/test/*tt"), + PathBufWrp::get_pathbuf("/test/*tt").map(|t| t.0), Err(UriSegmentError::BadStart('*')) ); assert_eq!( - PathBuf::from_param("/test/tt:"), + PathBufWrp::get_pathbuf("/test/tt:").map(|t| t.0), Err(UriSegmentError::BadEnd(':')) ); assert_eq!( - PathBuf::from_param("/test/tt<"), + PathBufWrp::get_pathbuf("/test/tt<").map(|t| t.0), Err(UriSegmentError::BadEnd('<')) ); assert_eq!( - PathBuf::from_param("/test/tt>"), + PathBufWrp::get_pathbuf("/test/tt>").map(|t| t.0), Err(UriSegmentError::BadEnd('>')) ); assert_eq!( - PathBuf::from_param("/seg1/seg2/"), - Ok(PathBuf::from_iter(vec!["seg1", "seg2"])) + PathBufWrp::get_pathbuf("/seg1/seg2/").unwrap().0, + PathBuf::from_iter(vec!["seg1", "seg2"]) ); assert_eq!( - PathBuf::from_param("/seg1/../seg2/"), - Ok(PathBuf::from_iter(vec!["seg2"])) + PathBufWrp::get_pathbuf("/seg1/../seg2/").unwrap().0, + PathBuf::from_iter(vec!["seg2"]) ); } } diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 2fc1c454d..6372a183c 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -304,8 +304,6 @@ impl Responder for NamedFile { type Future = Result; fn respond_to(self, req: &HttpRequest) -> Self::Future { - println!("RESP: {:?}", req); - if self.status_code != StatusCode::OK { let mut resp = HttpResponse::build(self.status_code); resp.set(header::ContentType(self.content_type.clone())) diff --git a/actix-session/src/cookie.rs b/actix-session/src/cookie.rs index 7fd5ec643..e25031456 100644 --- a/actix-session/src/cookie.rs +++ b/actix-session/src/cookie.rs @@ -255,12 +255,13 @@ impl CookieSession { } } -impl Transform> for CookieSession +impl Transform for CookieSession where - S: Service, Response = ServiceResponse>, + S: Service, Response = ServiceResponse>, S::Future: 'static, S::Error: 'static, { + type Request = ServiceRequest

; type Response = ServiceResponse; type Error = S::Error; type InitError = (); @@ -281,12 +282,13 @@ pub struct CookieSessionMiddleware { inner: Rc, } -impl Service> for CookieSessionMiddleware +impl Service for CookieSessionMiddleware where - S: Service, Response = ServiceResponse>, + S: Service, Response = ServiceResponse>, S::Future: 'static, S::Error: 'static, { + type Request = ServiceRequest

; type Response = ServiceResponse; type Error = S::Error; type Future = Box>; diff --git a/actix-web-codegen/src/lib.rs b/actix-web-codegen/src/lib.rs index 26b422d77..13d1b97f5 100644 --- a/actix-web-codegen/src/lib.rs +++ b/actix-web-codegen/src/lib.rs @@ -31,7 +31,7 @@ pub fn get(args: TokenStream, input: TokenStream) -> TokenStream { struct #name; impl actix_web::dev::HttpServiceFactory

for #name { - fn register(self, config: &mut actix_web::dev::AppConfig

) { + fn register(self, config: &mut actix_web::dev::ServiceConfig

) { #ast actix_web::dev::HttpServiceFactory::register( actix_web::Resource::new(#path) @@ -68,7 +68,7 @@ pub fn post(args: TokenStream, input: TokenStream) -> TokenStream { struct #name; impl actix_web::dev::HttpServiceFactory

for #name { - fn register(self, config: &mut actix_web::dev::AppConfig

) { + fn register(self, config: &mut actix_web::dev::ServiceConfig

) { #ast actix_web::dev::HttpServiceFactory::register( actix_web::Resource::new(#path) @@ -105,7 +105,7 @@ pub fn put(args: TokenStream, input: TokenStream) -> TokenStream { struct #name; impl actix_web::dev::HttpServiceFactory

for #name { - fn register(self, config: &mut actix_web::dev::AppConfig

) { + fn register(self, config: &mut actix_web::dev::ServiceConfig

) { #ast actix_web::dev::HttpServiceFactory::register( actix_web::Resource::new(#path) diff --git a/src/app.rs b/src/app.rs index 42ce62d89..29dd1ab60 100644 --- a/src/app.rs +++ b/src/app.rs @@ -3,7 +3,8 @@ use std::marker::PhantomData; use std::rc::Rc; use actix_http::body::{Body, MessageBody}; -use actix_http::{Extensions, PayloadStream}; +use actix_http::PayloadStream; +use actix_router::ResourceDef; use actix_server_config::ServerConfig; use actix_service::boxed::{self, BoxedNewService}; use actix_service::{ @@ -12,6 +13,7 @@ use actix_service::{ use futures::IntoFuture; use crate::app_service::{AppChain, AppEntry, AppInit, AppRouting, AppRoutingFactory}; +use crate::config::{AppConfig, AppConfigInner}; use crate::resource::Resource; use crate::route::Route; use crate::service::{ @@ -29,9 +31,8 @@ where T: NewService>, { chain: T, - extensions: Extensions, state: Vec>, - host: String, + config: AppConfigInner, _t: PhantomData<(P,)>, } @@ -41,9 +42,8 @@ impl App { pub fn new() -> Self { App { chain: AppChain, - extensions: Extensions::new(), state: Vec::new(), - host: "localhost:8080".to_string(), + config: AppConfigInner::default(), _t: PhantomData, } } @@ -141,8 +141,8 @@ where services: Vec::new(), default: None, factory_ref: fref, - extensions: self.extensions, - host: self.host, + config: self.config, + external: Vec::new(), _t: PhantomData, } } @@ -174,8 +174,7 @@ where App { chain, state: self.state, - extensions: self.extensions, - host: self.host, + config: self.config, _t: PhantomData, } } @@ -223,10 +222,10 @@ where default: None, endpoint: AppEntry::new(fref.clone()), factory_ref: fref, - extensions: self.extensions, state: self.state, - host: self.host, + config: self.config, services: vec![Box::new(ServiceFactoryWrapper::new(service))], + external: Vec::new(), _t: PhantomData, } } @@ -239,7 +238,7 @@ where /// /// By default host name is set to a "localhost" value. pub fn hostname(mut self, val: &str) -> Self { - self.host = val.to_owned(); + self.config.host = val.to_owned(); self } } @@ -252,9 +251,9 @@ pub struct AppRouter { services: Vec>>, default: Option>>, factory_ref: Rc>>>, - extensions: Extensions, state: Vec>, - host: String, + config: AppConfigInner, + external: Vec, _t: PhantomData<(P, B)>, } @@ -348,8 +347,8 @@ where services: self.services, default: self.default, factory_ref: self.factory_ref, - extensions: self.extensions, - host: self.host, + config: self.config, + external: self.external, _t: PhantomData, } } @@ -382,33 +381,30 @@ where /// and are never considered for matching at request time. Calls to /// `HttpRequest::url_for()` will work as expected. /// - /// ```rust,ignore - /// # extern crate actix_web; - /// use actix_web::{App, HttpRequest, HttpResponse, Result}; + /// ```rust + /// use actix_web::{web, App, HttpRequest, HttpResponse, Result}; /// - /// fn index(req: &HttpRequest) -> Result { - /// let url = req.url_for("youtube", &["oHg5SJYRHA0"])?; - /// assert_eq!(url.as_str(), "https://youtube.com/watch/oHg5SJYRHA0"); + /// fn index(req: HttpRequest) -> Result { + /// let url = req.url_for("youtube", &["asdlkjqme"])?; + /// assert_eq!(url.as_str(), "https://youtube.com/watch/asdlkjqme"); /// Ok(HttpResponse::Ok().into()) /// } /// /// fn main() { /// let app = App::new() - /// .resource("/index.html", |r| r.get().f(index)) - /// .external_resource("youtube", "https://youtube.com/watch/{video_id}") - /// .finish(); + /// .service(web::resource("/index.html").route( + /// web::get().to(index))) + /// .external_resource("youtube", "https://youtube.com/watch/{video_id}"); /// } /// ``` - pub fn external_resource(self, _name: N, _url: U) -> Self + pub fn external_resource(mut self, name: N, url: U) -> Self where N: AsRef, U: AsRef, { - // self.parts - // .as_mut() - // .expect("Use after finish") - // .router - // .register_external(name.as_ref(), ResourceDef::external(url.as_ref())); + let mut rdef = ResourceDef::new(url.as_ref()); + *rdef.name_mut() = name.as_ref().to_string(); + self.external.push(rdef); self } } @@ -435,9 +431,10 @@ where state: self.state, endpoint: self.endpoint, services: RefCell::new(self.services), + external: RefCell::new(self.external), default: self.default, factory_ref: self.factory_ref, - extensions: Rc::new(RefCell::new(Rc::new(self.extensions))), + config: RefCell::new(AppConfig(Rc::new(self.config))), } } } diff --git a/src/app_service.rs b/src/app_service.rs index 094486d9b..75e4b3164 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -2,7 +2,7 @@ use std::cell::RefCell; use std::marker::PhantomData; use std::rc::Rc; -use actix_http::{Extensions, Request, Response}; +use actix_http::{Request, Response}; use actix_router::{Path, ResourceDef, ResourceInfo, Router, Url}; use actix_server_config::ServerConfig; use actix_service::boxed::{self, BoxedNewService, BoxedService}; @@ -10,7 +10,7 @@ use actix_service::{fn_service, AndThen, NewService, Service, ServiceExt}; use futures::future::{ok, Either, FutureResult}; use futures::{Async, Future, Poll}; -use crate::config::AppConfig; +use crate::config::{AppConfig, ServiceConfig}; use crate::guard::Guard; use crate::rmap::ResourceMap; use crate::service::{ServiceFactory, ServiceRequest, ServiceResponse}; @@ -36,10 +36,11 @@ where pub(crate) chain: C, pub(crate) endpoint: T, pub(crate) state: Vec>, - pub(crate) extensions: Rc>>, + pub(crate) config: RefCell, pub(crate) services: RefCell>>>, pub(crate) default: Option>>, pub(crate) factory_ref: Rc>>>, + pub(crate) external: RefCell>, } impl NewService for AppInit @@ -64,7 +65,7 @@ where type Service = AndThen, T::Service>; type Future = AppInitResult; - fn new_service(&self, _: &ServerConfig) -> Self::Future { + fn new_service(&self, cfg: &ServerConfig) -> Self::Future { // update resource default service let default = self.default.clone().unwrap_or_else(|| { Rc::new(boxed::new_service(fn_service(|req: ServiceRequest

| { @@ -72,12 +73,15 @@ where }))) }); - let mut config = AppConfig::new( - "127.0.0.1:8080".parse().unwrap(), - "localhost:8080".to_owned(), - false, - default.clone(), - ); + { + let mut c = self.config.borrow_mut(); + let loc_cfg = Rc::get_mut(&mut c.0).unwrap(); + loc_cfg.secure = cfg.secure(); + loc_cfg.addr = cfg.local_addr(); + } + + let mut config = + ServiceConfig::new(self.config.borrow().clone(), default.clone()); // register services std::mem::replace(&mut *self.services.borrow_mut(), Vec::new()) @@ -101,6 +105,11 @@ where ), }); + // external resources + for mut rdef in std::mem::replace(&mut *self.external.borrow_mut(), Vec::new()) { + rmap.add(&mut rdef, None); + } + // complete ResourceMap tree creation let rmap = Rc::new(rmap); rmap.finish(rmap.clone()); @@ -111,7 +120,7 @@ where endpoint: None, endpoint_fut: self.endpoint.new_service(&()), state: self.state.iter().map(|s| s.construct()).collect(), - extensions: self.extensions.clone(), + config: self.config.borrow().clone(), rmap, _t: PhantomData, } @@ -129,7 +138,7 @@ where endpoint_fut: T::Future, rmap: Rc, state: Vec>, - extensions: Rc>>, + config: AppConfig, _t: PhantomData<(P, B)>, } @@ -152,20 +161,14 @@ where type Error = C::InitError; fn poll(&mut self) -> Poll { - if let Some(extensions) = Rc::get_mut(&mut *self.extensions.borrow_mut()) { - let mut idx = 0; - while idx < self.state.len() { - if let Async::Ready(_) = self.state[idx].poll_result(extensions)? { - self.state.remove(idx); - } else { - idx += 1; - } + let mut idx = 0; + let mut extensions = self.config.0.extensions.borrow_mut(); + while idx < self.state.len() { + if let Async::Ready(_) = self.state[idx].poll_result(&mut extensions)? { + self.state.remove(idx); + } else { + idx += 1; } - if !self.state.is_empty() { - return Ok(Async::NotReady); - } - } else { - log::warn!("Multiple copies of app extensions exists"); } if self.chain.is_none() { @@ -185,7 +188,7 @@ where AppInitService { chain: self.chain.take().unwrap(), rmap: self.rmap.clone(), - extensions: self.extensions.borrow().clone(), + config: self.config.clone(), } .and_then(self.endpoint.take().unwrap()), )) @@ -202,7 +205,7 @@ where { chain: C, rmap: Rc, - extensions: Rc, + config: AppConfig, } impl Service for AppInitService @@ -223,7 +226,7 @@ where Path::new(Url::new(req.uri().clone())), req, self.rmap.clone(), - self.extensions.clone(), + self.config.clone(), ); self.chain.call(req) } diff --git a/src/config.rs b/src/config.rs index 47c2f7c45..f84376c76 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,6 +1,8 @@ +use std::cell::{Ref, RefCell}; use std::net::SocketAddr; use std::rc::Rc; +use actix_http::Extensions; use actix_router::ResourceDef; use actix_service::{boxed, IntoNewService, NewService}; @@ -13,10 +15,8 @@ type HttpNewService

= boxed::BoxedNewService<(), ServiceRequest

, ServiceResponse, (), ()>; /// Application configuration -pub struct AppConfig

{ - addr: SocketAddr, - secure: bool, - host: String, +pub struct ServiceConfig

{ + config: AppConfig, root: bool, default: Rc>, services: Vec<( @@ -27,18 +27,11 @@ pub struct AppConfig

{ )>, } -impl AppConfig

{ +impl ServiceConfig

{ /// Crate server settings instance - pub(crate) fn new( - addr: SocketAddr, - host: String, - secure: bool, - default: Rc>, - ) -> Self { - AppConfig { - addr, - secure, - host, + pub(crate) fn new(config: AppConfig, default: Rc>) -> Self { + ServiceConfig { + config, default, root: true, services: Vec::new(), @@ -62,31 +55,20 @@ impl AppConfig

{ } pub(crate) fn clone_config(&self) -> Self { - AppConfig { - addr: self.addr, - secure: self.secure, - host: self.host.clone(), + ServiceConfig { + config: self.config.clone(), default: self.default.clone(), services: Vec::new(), root: false, } } - /// Returns the socket address of the local half of this TCP connection - pub fn local_addr(&self) -> SocketAddr { - self.addr - } - - /// Returns true if connection is secure(https) - pub fn secure(&self) -> bool { - self.secure - } - - /// Returns host header value - pub fn host(&self) -> &str { - &self.host + /// Service configuration + pub fn config(&self) -> &AppConfig { + &self.config } + /// Default resource pub fn default_service(&self) -> Rc> { self.default.clone() } @@ -114,3 +96,63 @@ impl AppConfig

{ )); } } + +#[derive(Clone)] +pub struct AppConfig(pub(crate) Rc); + +impl AppConfig { + pub(crate) fn new(inner: AppConfigInner) -> Self { + AppConfig(Rc::new(inner)) + } + + /// Set server host name. + /// + /// Host name is used by application router aa 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 host(&self) -> &str { + &self.0.host + } + + /// Returns true if connection is secure(https) + pub fn secure(&self) -> bool { + self.0.secure + } + + /// Returns the socket address of the local half of this TCP connection + pub fn local_addr(&self) -> SocketAddr { + self.0.addr + } + + /// Resource map + pub fn rmap(&self) -> &ResourceMap { + &self.0.rmap + } + + /// Application extensions + pub fn extensions(&self) -> Ref { + self.0.extensions.borrow() + } +} + +pub(crate) struct AppConfigInner { + pub(crate) secure: bool, + pub(crate) host: String, + pub(crate) addr: SocketAddr, + pub(crate) rmap: ResourceMap, + pub(crate) extensions: RefCell, +} + +impl Default for AppConfigInner { + fn default() -> AppConfigInner { + AppConfigInner { + secure: false, + addr: "127.0.0.1:8080".parse().unwrap(), + host: "localhost:8080".to_owned(), + rmap: ResourceMap::new(ResourceDef::new("")), + extensions: RefCell::new(Extensions::new()), + } + } +} diff --git a/src/error.rs b/src/error.rs index d1c0d3ca9..54ca74dc2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,3 +1,5 @@ +//! Error and Result module + pub use actix_http::error::*; use derive_more::{Display, From}; use url::ParseError as UrlParseError; diff --git a/src/info.rs b/src/info.rs index 3b51215fe..c058bd517 100644 --- a/src/info.rs +++ b/src/info.rs @@ -1,19 +1,14 @@ use std::cell::Ref; -use actix_http::http::header::{self, HeaderName}; -use actix_http::RequestHead; +use crate::dev::{AppConfig, RequestHead}; +use crate::http::header::{self, HeaderName}; const X_FORWARDED_FOR: &[u8] = b"x-forwarded-for"; const X_FORWARDED_HOST: &[u8] = b"x-forwarded-host"; const X_FORWARDED_PROTO: &[u8] = b"x-forwarded-proto"; -pub enum ConnectionInfoError { - UnknownHost, - UnknownScheme, -} - /// `HttpRequest` connection information -#[derive(Clone, Default)] +#[derive(Debug, Clone, Default)] pub struct ConnectionInfo { scheme: String, host: String, @@ -23,19 +18,19 @@ pub struct ConnectionInfo { impl ConnectionInfo { /// Create *ConnectionInfo* instance for a request. - pub fn get(req: &RequestHead) -> Ref { + pub fn get<'a>(req: &'a RequestHead, cfg: &AppConfig) -> Ref<'a, Self> { if !req.extensions().contains::() { - req.extensions_mut().insert(ConnectionInfo::new(req)); + req.extensions_mut().insert(ConnectionInfo::new(req, cfg)); } Ref::map(req.extensions(), |e| e.get().unwrap()) } #[cfg_attr(feature = "cargo-clippy", allow(cyclomatic_complexity))] - fn new(req: &RequestHead) -> ConnectionInfo { + fn new(req: &RequestHead, cfg: &AppConfig) -> ConnectionInfo { let mut host = None; let mut scheme = None; let mut remote = None; - let mut peer = None; + let peer = None; // load forwarded header for hdr in req.headers.get_all(header::FORWARDED) { @@ -82,7 +77,7 @@ impl ConnectionInfo { } if scheme.is_none() { scheme = req.uri.scheme_part().map(|a| a.as_str()); - if scheme.is_none() && req.server_settings().secure() { + if scheme.is_none() && cfg.secure() { scheme = Some("https") } } @@ -105,7 +100,7 @@ impl ConnectionInfo { if host.is_none() { host = req.uri.authority_part().map(|a| a.as_str()); if host.is_none() { - host = Some(req.server_settings().host()); + host = Some(cfg.host()); } } } @@ -121,10 +116,10 @@ impl ConnectionInfo { remote = h.split(',').next().map(|v| v.trim()); } } - if remote.is_none() { - // get peeraddr from socketaddr - peer = req.peer_addr().map(|addr| format!("{}", addr)); - } + // if remote.is_none() { + // get peeraddr from socketaddr + // peer = req.peer_addr().map(|addr| format!("{}", addr)); + // } } ConnectionInfo { @@ -186,9 +181,8 @@ mod tests { #[test] fn test_forwarded() { - let req = TestRequest::default().request(); - let mut info = ConnectionInfo::default(); - info.update(&req); + let req = TestRequest::default().to_http_request(); + let info = req.connection_info(); assert_eq!(info.scheme(), "http"); assert_eq!(info.host(), "localhost:8080"); @@ -197,44 +191,39 @@ mod tests { header::FORWARDED, "for=192.0.2.60; proto=https; by=203.0.113.43; host=rust-lang.org", ) - .request(); + .to_http_request(); - let mut info = ConnectionInfo::default(); - info.update(&req); + let info = req.connection_info(); assert_eq!(info.scheme(), "https"); assert_eq!(info.host(), "rust-lang.org"); assert_eq!(info.remote(), Some("192.0.2.60")); let req = TestRequest::default() .header(header::HOST, "rust-lang.org") - .request(); + .to_http_request(); - let mut info = ConnectionInfo::default(); - info.update(&req); + let info = req.connection_info(); assert_eq!(info.scheme(), "http"); assert_eq!(info.host(), "rust-lang.org"); assert_eq!(info.remote(), None); let req = TestRequest::default() .header(X_FORWARDED_FOR, "192.0.2.60") - .request(); - let mut info = ConnectionInfo::default(); - info.update(&req); + .to_http_request(); + let info = req.connection_info(); assert_eq!(info.remote(), Some("192.0.2.60")); let req = TestRequest::default() .header(X_FORWARDED_HOST, "192.0.2.60") - .request(); - let mut info = ConnectionInfo::default(); - info.update(&req); + .to_http_request(); + let info = req.connection_info(); assert_eq!(info.host(), "192.0.2.60"); assert_eq!(info.remote(), None); let req = TestRequest::default() .header(X_FORWARDED_PROTO, "https") - .request(); - let mut info = ConnectionInfo::default(); - info.update(&req); + .to_http_request(); + let info = req.connection_info(); assert_eq!(info.scheme(), "https"); } } diff --git a/src/lib.rs b/src/lib.rs index 19f466b4c..6329d53ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,13 +2,13 @@ mod app; mod app_service; -mod extract; -mod handler; -// mod info; pub mod blocking; mod config; pub mod error; +mod extract; pub mod guard; +mod handler; +mod info; pub mod middleware; mod request; mod resource; @@ -54,7 +54,9 @@ pub mod dev { //! ``` pub use crate::app::AppRouter; - pub use crate::config::AppConfig; + pub use crate::config::{AppConfig, ServiceConfig}; + pub use crate::info::ConnectionInfo; + pub use crate::rmap::ResourceMap; pub use crate::service::HttpServiceFactory; pub use actix_http::body::{Body, BodyLength, MessageBody, ResponseBody}; @@ -62,7 +64,7 @@ pub mod dev { pub use actix_http::{ Extensions, Payload, PayloadStream, RequestHead, ResponseHead, }; - pub use actix_router::{Path, ResourceDef, Url}; + pub use actix_router::{Path, ResourceDef, ResourcePath, Url}; pub(crate) fn insert_slash(path: &str) -> String { let mut path = path.to_owned(); diff --git a/src/request.rs b/src/request.rs index 6655f1baa..717514838 100644 --- a/src/request.rs +++ b/src/request.rs @@ -7,8 +7,10 @@ use actix_http::http::{HeaderMap, Method, Uri, Version}; use actix_http::{Error, Extensions, HttpMessage, Message, Payload, RequestHead}; use actix_router::{Path, Url}; +use crate::config::AppConfig; use crate::error::UrlGenerationError; use crate::extract::FromRequest; +use crate::info::ConnectionInfo; use crate::rmap::ResourceMap; use crate::service::ServiceFromRequest; @@ -18,7 +20,7 @@ pub struct HttpRequest { pub(crate) head: Message, pub(crate) path: Path, rmap: Rc, - extensions: Rc, + config: AppConfig, } impl HttpRequest { @@ -27,13 +29,13 @@ impl HttpRequest { head: Message, path: Path, rmap: Rc, - extensions: Rc, + config: AppConfig, ) -> HttpRequest { HttpRequest { head, path, rmap, - extensions, + config, } } } @@ -92,17 +94,17 @@ impl HttpRequest { &self.path } - /// Application extensions + /// App config #[inline] - pub fn app_extensions(&self) -> &Extensions { - &self.extensions + pub fn config(&self) -> &AppConfig { + &self.config } /// Generate url for named resource /// /// ```rust /// # extern crate actix_web; - /// # use actix_web::{App, HttpRequest, HttpResponse, http}; + /// # use actix_web::{web, App, HttpRequest, HttpResponse}; /// # /// fn index(req: HttpRequest) -> HttpResponse { /// let url = req.url_for("foo", &["1", "2", "3"]); // <- generate url for "foo" resource @@ -111,11 +113,10 @@ impl HttpRequest { /// /// fn main() { /// let app = App::new() - /// .resource("/test/{one}/{two}/{three}", |r| { - /// r.name("foo"); // <- set resource name, then it could be used in `url_for` - /// r.method(http::Method::GET).f(|_| HttpResponse::Ok()); - /// }) - /// .finish(); + /// .service(web::resource("/test/{one}/{two}/{three}") + /// .name("foo") // <- set resource name, then it could be used in `url_for` + /// .route(web::get().to(|| HttpResponse::Ok())) + /// ); /// } /// ``` pub fn url_for( @@ -139,11 +140,11 @@ impl HttpRequest { self.url_for(name, &NO_PARAMS) } - // /// Get *ConnectionInfo* for the correct request. - // #[inline] - // pub fn connection_info(&self) -> Ref { - // ConnectionInfo::get(&*self) - // } + /// Get *ConnectionInfo* for the current request. + #[inline] + pub fn connection_info(&self) -> Ref { + ConnectionInfo::get(&*self, &*self.config()) + } } impl Deref for HttpRequest { @@ -234,3 +235,124 @@ impl fmt::Debug for HttpRequest { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::dev::{ResourceDef, ResourceMap}; + use crate::http::header; + use crate::test::TestRequest; + + #[test] + fn test_debug() { + let req = + TestRequest::with_header("content-type", "text/plain").to_http_request(); + let dbg = format!("{:?}", req); + assert!(dbg.contains("HttpRequest")); + } + + #[test] + fn test_no_request_cookies() { + let req = TestRequest::default().to_http_request(); + assert!(req.cookies().unwrap().is_empty()); + } + + #[test] + fn test_request_cookies() { + let req = TestRequest::default() + .header(header::COOKIE, "cookie1=value1") + .header(header::COOKIE, "cookie2=value2") + .to_http_request(); + { + let cookies = req.cookies().unwrap(); + assert_eq!(cookies.len(), 2); + assert_eq!(cookies[0].name(), "cookie1"); + assert_eq!(cookies[0].value(), "value1"); + assert_eq!(cookies[1].name(), "cookie2"); + assert_eq!(cookies[1].value(), "value2"); + } + + let cookie = req.cookie("cookie1"); + assert!(cookie.is_some()); + let cookie = cookie.unwrap(); + assert_eq!(cookie.name(), "cookie1"); + assert_eq!(cookie.value(), "value1"); + + let cookie = req.cookie("cookie-unknown"); + assert!(cookie.is_none()); + } + + #[test] + fn test_request_query() { + let req = TestRequest::with_uri("/?id=test").to_http_request(); + assert_eq!(req.query_string(), "id=test"); + } + + #[test] + fn test_url_for() { + let mut res = ResourceDef::new("/user/{name}.{ext}"); + *res.name_mut() = "index".to_string(); + + let mut rmap = ResourceMap::new(ResourceDef::new("")); + rmap.add(&mut res, None); + assert!(rmap.has_resource("/user/test.html")); + assert!(!rmap.has_resource("/test/unknown")); + + let req = TestRequest::with_header(header::HOST, "www.rust-lang.org") + .rmap(rmap) + .to_http_request(); + + assert_eq!( + req.url_for("unknown", &["test"]), + Err(UrlGenerationError::ResourceNotFound) + ); + assert_eq!( + req.url_for("index", &["test"]), + Err(UrlGenerationError::NotEnoughElements) + ); + let url = req.url_for("index", &["test", "html"]); + assert_eq!( + url.ok().unwrap().as_str(), + "http://www.rust-lang.org/user/test.html" + ); + } + + #[test] + fn test_url_for_static() { + let mut rdef = ResourceDef::new("/index.html"); + *rdef.name_mut() = "index".to_string(); + + let mut rmap = ResourceMap::new(ResourceDef::new("")); + rmap.add(&mut rdef, None); + + assert!(rmap.has_resource("/index.html")); + + let req = TestRequest::with_uri("/test") + .header(header::HOST, "www.rust-lang.org") + .rmap(rmap) + .to_http_request(); + let url = req.url_for_static("index"); + assert_eq!( + url.ok().unwrap().as_str(), + "http://www.rust-lang.org/index.html" + ); + } + + #[test] + fn test_url_for_external() { + let mut rdef = ResourceDef::new("https://youtube.com/watch/{video_id}"); + + *rdef.name_mut() = "youtube".to_string(); + + let mut rmap = ResourceMap::new(ResourceDef::new("")); + rmap.add(&mut rdef, None); + assert!(rmap.has_resource("https://youtube.com/watch/unknown")); + + let req = TestRequest::default().rmap(rmap).to_http_request(); + let url = req.url_for("youtube", &["oHg5SJYRHA0"]); + assert_eq!( + url.ok().unwrap().as_str(), + "https://youtube.com/watch/oHg5SJYRHA0" + ); + } +} diff --git a/src/resource.rs b/src/resource.rs index cc8316653..57f6f710a 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -9,7 +9,7 @@ use actix_service::{ use futures::future::{ok, Either, FutureResult}; use futures::{Async, Future, IntoFuture, Poll}; -use crate::dev::{insert_slash, AppConfig, HttpServiceFactory, ResourceDef}; +use crate::dev::{insert_slash, HttpServiceFactory, ResourceDef, ServiceConfig}; use crate::extract::FromRequest; use crate::guard::Guard; use crate::handler::{AsyncFactory, Factory}; @@ -41,6 +41,7 @@ type HttpNewService

= BoxedNewService<(), ServiceRequest

, ServiceResponse, pub struct Resource> { endpoint: T, rdef: String, + name: Option, routes: Vec>, guards: Vec>, default: Rc>>>>, @@ -54,6 +55,7 @@ impl

Resource

{ Resource { routes: Vec::new(), rdef: path.to_string(), + name: None, endpoint: ResourceEndpoint::new(fref.clone()), factory_ref: fref, guards: Vec::new(), @@ -72,6 +74,14 @@ where InitError = (), >, { + /// Set resource name. + /// + /// Name is used for url generation. + pub fn name(mut self, name: &str) -> Self { + self.name = Some(name.to_string()); + self + } + /// Add match guard to a resource. /// /// ```rust @@ -240,6 +250,7 @@ where Resource { endpoint, rdef: self.rdef, + name: self.name, guards: self.guards, routes: self.routes, default: self.default, @@ -277,7 +288,7 @@ where InitError = (), > + 'static, { - fn register(mut self, config: &mut AppConfig

) { + fn register(mut self, config: &mut ServiceConfig

) { if self.default.borrow().is_none() { *self.default.borrow_mut() = Some(config.default_service()); } @@ -286,11 +297,14 @@ where } else { Some(std::mem::replace(&mut self.guards, Vec::new())) }; - let rdef = if config.is_root() || !self.rdef.is_empty() { + let mut rdef = if config.is_root() || !self.rdef.is_empty() { ResourceDef::new(&insert_slash(&self.rdef)) } else { ResourceDef::new(&self.rdef) }; + if let Some(ref name) = self.name { + *rdef.name_mut() = name.clone(); + } config.register_service(rdef, guards, self, None) } } diff --git a/src/rmap.rs b/src/rmap.rs index 4922084bf..35fe8ee32 100644 --- a/src/rmap.rs +++ b/src/rmap.rs @@ -64,14 +64,13 @@ impl ResourceMap { if self.patterns_for(name, &mut path, &mut elements)?.is_some() { if path.starts_with('/') { - // let conn = req.connection_info(); - // Ok(Url::parse(&format!( - // "{}://{}{}", - // conn.scheme(), - // conn.host(), - // path - // ))?) - unimplemented!() + let conn = req.connection_info(); + Ok(Url::parse(&format!( + "{}://{}{}", + conn.scheme(), + conn.host(), + path + ))?) } else { Ok(Url::parse(&path)?) } diff --git a/src/scope.rs b/src/scope.rs index 6c511c69c..3b5061737 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -10,7 +10,7 @@ use actix_service::{ use futures::future::{ok, Either, Future, FutureResult}; use futures::{Async, Poll}; -use crate::dev::{AppConfig, HttpServiceFactory}; +use crate::dev::{HttpServiceFactory, ServiceConfig}; use crate::guard::Guard; use crate::resource::Resource; use crate::rmap::ResourceMap; @@ -237,7 +237,7 @@ where InitError = (), > + 'static, { - fn register(self, config: &mut AppConfig

) { + fn register(self, config: &mut ServiceConfig

) { // update default resource if needed if self.default.borrow().is_none() { *self.default.borrow_mut() = Some(config.default_service()); diff --git a/src/service.rs b/src/service.rs index ba8114588..f4b63a460 100644 --- a/src/service.rs +++ b/src/service.rs @@ -13,16 +13,16 @@ use actix_http::{ use actix_router::{Path, Resource, Url}; use futures::future::{ok, FutureResult, IntoFuture}; -use crate::config::AppConfig; +use crate::config::{AppConfig, ServiceConfig}; use crate::request::HttpRequest; use crate::rmap::ResourceMap; pub trait HttpServiceFactory

{ - fn register(self, config: &mut AppConfig

); + fn register(self, config: &mut ServiceConfig

); } pub(crate) trait ServiceFactory

{ - fn register(&mut self, config: &mut AppConfig

); + fn register(&mut self, config: &mut ServiceConfig

); } pub(crate) struct ServiceFactoryWrapper { @@ -43,7 +43,7 @@ impl ServiceFactory

for ServiceFactoryWrapper where T: HttpServiceFactory

, { - fn register(&mut self, config: &mut AppConfig

) { + fn register(&mut self, config: &mut ServiceConfig

) { if let Some(item) = self.factory.take() { item.register(config) } @@ -60,12 +60,12 @@ impl

ServiceRequest

{ path: Path, request: Request

, rmap: Rc, - extensions: Rc, + config: AppConfig, ) -> Self { let (head, payload) = request.into_parts(); ServiceRequest { payload, - req: HttpRequest::new(head, path, rmap, extensions), + req: HttpRequest::new(head, path, rmap, config), } } @@ -156,10 +156,10 @@ impl

ServiceRequest

{ &mut self.req.path } - /// Application extensions + /// Service configuration #[inline] - pub fn app_extensions(&self) -> &Extensions { - self.req.app_extensions() + pub fn app_config(&self) -> &AppConfig { + self.req.config() } /// Deconstruct request into parts diff --git a/src/state.rs b/src/state.rs index 265c6f017..2c623c70d 100644 --- a/src/state.rs +++ b/src/state.rs @@ -52,7 +52,7 @@ impl FromRequest

for State { #[inline] fn from_request(req: &mut ServiceFromRequest

) -> Self::Future { - if let Some(st) = req.app_extensions().get::>() { + if let Some(st) = req.config().extensions().get::>() { Ok(st.clone()) } else { Err(ErrorInternalServerError( diff --git a/src/test.rs b/src/test.rs index c88835a3f..b47daa2c6 100644 --- a/src/test.rs +++ b/src/test.rs @@ -5,7 +5,7 @@ use std::rc::Rc; use actix_http::http::header::{Header, HeaderName, IntoHeaderValue}; use actix_http::http::{HttpTryFrom, Method, Version}; use actix_http::test::TestRequest as HttpTestRequest; -use actix_http::{Extensions, PayloadStream, Request}; +use actix_http::{PayloadStream, Request}; use actix_router::{Path, ResourceDef, Url}; use actix_rt::Runtime; use actix_server_config::ServerConfig; @@ -13,6 +13,7 @@ use actix_service::{IntoNewService, NewService, Service}; use bytes::Bytes; use futures::Future; +use crate::config::{AppConfig, AppConfigInner}; use crate::request::HttpRequest; use crate::rmap::ResourceMap; use crate::service::{ServiceFromRequest, ServiceRequest, ServiceResponse}; @@ -142,16 +143,16 @@ where /// ``` pub struct TestRequest { req: HttpTestRequest, - extensions: Extensions, rmap: ResourceMap, + config: AppConfigInner, } impl Default for TestRequest { fn default() -> TestRequest { TestRequest { req: HttpTestRequest::default(), - extensions: Extensions::new(), rmap: ResourceMap::new(ResourceDef::new("")), + config: AppConfigInner::default(), } } } @@ -161,8 +162,8 @@ impl TestRequest { pub fn with_uri(path: &str) -> TestRequest { TestRequest { req: HttpTestRequest::default().uri(path).take(), - extensions: Extensions::new(), rmap: ResourceMap::new(ResourceDef::new("")), + config: AppConfigInner::default(), } } @@ -170,7 +171,7 @@ impl TestRequest { pub fn with_hdr(hdr: H) -> TestRequest { TestRequest { req: HttpTestRequest::default().set(hdr).take(), - extensions: Extensions::new(), + config: AppConfigInner::default(), rmap: ResourceMap::new(ResourceDef::new("")), } } @@ -183,7 +184,7 @@ impl TestRequest { { TestRequest { req: HttpTestRequest::default().header(key, value).take(), - extensions: Extensions::new(), + config: AppConfigInner::default(), rmap: ResourceMap::new(ResourceDef::new("")), } } @@ -192,7 +193,7 @@ impl TestRequest { pub fn get() -> TestRequest { TestRequest { req: HttpTestRequest::default().method(Method::GET).take(), - extensions: Extensions::new(), + config: AppConfigInner::default(), rmap: ResourceMap::new(ResourceDef::new("")), } } @@ -201,7 +202,7 @@ impl TestRequest { pub fn post() -> TestRequest { TestRequest { req: HttpTestRequest::default().method(Method::POST).take(), - extensions: Extensions::new(), + config: AppConfigInner::default(), rmap: ResourceMap::new(ResourceDef::new("")), } } @@ -247,8 +248,15 @@ impl TestRequest { } /// Set request config - pub fn config(mut self, data: T) -> Self { - self.extensions.insert(data); + pub fn config(self, data: T) -> Self { + self.config.extensions.borrow_mut().insert(data); + self + } + + #[cfg(test)] + /// Set request config + pub(crate) fn rmap(mut self, rmap: ResourceMap) -> Self { + self.rmap = rmap; self } @@ -260,7 +268,7 @@ impl TestRequest { Path::new(Url::new(req.uri().clone())), req, Rc::new(self.rmap), - Rc::new(self.extensions), + AppConfig::new(self.config), ) } @@ -277,7 +285,7 @@ impl TestRequest { Path::new(Url::new(req.uri().clone())), req, Rc::new(self.rmap), - Rc::new(self.extensions), + AppConfig::new(self.config), ) .into_request() } @@ -290,7 +298,7 @@ impl TestRequest { Path::new(Url::new(req.uri().clone())), req, Rc::new(self.rmap), - Rc::new(self.extensions), + AppConfig::new(self.config), ); ServiceFromRequest::new(req, None) }