From a817ddb57b358748b47797b2d11b22236edf471d Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Mon, 7 May 2018 13:50:43 -0700 Subject: [PATCH 01/47] add variable segments support for scope prefix --- src/application.rs | 98 +++++++++++++++++-------- src/error.rs | 4 +- src/router.rs | 114 +++++++++++++++++++++++++++--- src/scope.rs | 149 ++++++++++++++++++++++++++++++++++----- src/ws/context.rs | 2 +- tests/test_middleware.rs | 2 +- 6 files changed, 306 insertions(+), 63 deletions(-) diff --git a/src/application.rs b/src/application.rs index 76ae1ba73..20dfa7901 100644 --- a/src/application.rs +++ b/src/application.rs @@ -29,7 +29,12 @@ pub(crate) struct Inner { default: ResourceHandler, encoding: ContentEncoding, resources: Vec>, - handlers: Vec<(String, Box>)>, + handlers: Vec>, +} + +enum PrefixHandlerType { + Handler(String, Box>), + Scope(Resource, Box>), } impl PipelineHandler for Inner { @@ -44,7 +49,10 @@ impl PipelineHandler for Inner { HandlerType::Normal(idx) => { self.resources[idx].handle(req, Some(&mut self.default)) } - HandlerType::Handler(idx) => self.handlers[idx].1.handle(req), + HandlerType::Handler(idx) => match self.handlers[idx] { + PrefixHandlerType::Handler(_, ref mut hnd) => hnd.handle(req), + PrefixHandlerType::Scope(_, ref mut hnd) => hnd.handle(req), + }, HandlerType::Default => self.default.handle(req, None), } } @@ -62,27 +70,49 @@ impl HttpApplication { HandlerType::Normal(idx) } else { let inner = self.as_ref(); + let path: &'static str = + unsafe { &*(&req.path()[inner.prefix..] as *const _) }; + let path_len = path.len(); for idx in 0..inner.handlers.len() { - let &(ref prefix, _) = &inner.handlers[idx]; - let m = { - let path = &req.path()[inner.prefix..]; - path.starts_with(prefix) - && (path.len() == prefix.len() - || path.split_at(prefix.len()).1.starts_with('/')) - }; + match &inner.handlers[idx] { + PrefixHandlerType::Handler(ref prefix, _) => { + let m = { + path.starts_with(prefix) + && (path_len == prefix.len() + || path.split_at(prefix.len()).1.starts_with('/')) + }; - if m { - let prefix_len = inner.prefix + prefix.len(); - let path: &'static str = - unsafe { &*(&req.path()[prefix_len..] as *const _) }; + if m { + let prefix_len = inner.prefix + prefix.len(); + let path: &'static str = + unsafe { &*(&req.path()[prefix_len..] as *const _) }; - req.set_prefix_len(prefix_len as u16); - if path.is_empty() { - req.match_info_mut().add("tail", "/"); - } else { - req.match_info_mut().add("tail", path); + req.set_prefix_len(prefix_len as u16); + if path.is_empty() { + req.match_info_mut().add("tail", "/"); + } else { + req.match_info_mut().add("tail", path); + } + return HandlerType::Handler(idx); + } + } + PrefixHandlerType::Scope(ref pattern, _) => { + if let Some(prefix_len) = + pattern.match_prefix_with_params(path, req.match_info_mut()) + { + let prefix_len = inner.prefix + prefix_len - 1; + let path: &'static str = + unsafe { &*(&req.path()[prefix_len..] as *const _) }; + + req.set_prefix_len(prefix_len as u16); + if path.is_empty() { + req.match_info_mut().set("tail", "/"); + } else { + req.match_info_mut().set("tail", path); + } + return HandlerType::Handler(idx); + } } - return HandlerType::Handler(idx); } } HandlerType::Default @@ -131,7 +161,7 @@ struct ApplicationParts { settings: ServerSettings, default: ResourceHandler, resources: Vec<(Resource, Option>)>, - handlers: Vec<(String, Box>)>, + handlers: Vec>, external: HashMap, encoding: ContentEncoding, middlewares: Vec>>, @@ -305,7 +335,7 @@ where /// Configure scope for common root path. /// /// Scopes collect multiple paths under a common path prefix. - /// Scope path can not contain variable path segments as resources. + /// Scope path can contain variable path segments as resources. /// /// ```rust /// # extern crate actix_web; @@ -313,7 +343,7 @@ where /// /// fn main() { /// let app = App::new() - /// .scope("/app", |scope| { + /// .scope("/{project_id}", |scope| { /// scope.resource("/path1", |r| r.f(|_| HttpResponse::Ok())) /// .resource("/path2", |r| r.f(|_| HttpResponse::Ok())) /// .resource("/path3", |r| r.f(|_| HttpResponse::MethodNotAllowed())) @@ -322,9 +352,9 @@ where /// ``` /// /// In the above example, three routes get added: - /// * /app/path1 - /// * /app/path2 - /// * /app/path3 + /// * /{project_id}/path1 + /// * /{project_id}/path2 + /// * /{project_id}/path3 /// pub fn scope(mut self, path: &str, f: F) -> App where @@ -337,9 +367,15 @@ where if !path.is_empty() && !path.starts_with('/') { path.insert(0, '/') } + if !path.ends_with('/') { + path.push('/'); + } let parts = self.parts.as_mut().expect("Use after finish"); - parts.handlers.push((path, scope)); + parts.handlers.push(PrefixHandlerType::Scope( + Resource::prefix("", &path), + scope, + )); } self } @@ -496,11 +532,15 @@ where if !path.is_empty() && !path.starts_with('/') { path.insert(0, '/') } + if path.len() > 1 && path.ends_with('/') { + path.pop(); + } let parts = self.parts.as_mut().expect("Use after finish"); - parts - .handlers - .push((path, Box::new(WrapHandler::new(handler)))); + parts.handlers.push(PrefixHandlerType::Handler( + path, + Box::new(WrapHandler::new(handler)), + )); } self } diff --git a/src/error.rs b/src/error.rs index 9482e067b..8c46774e5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -850,7 +850,7 @@ mod tests { } macro_rules! from { - ($from: expr => $error: pat) => { + ($from:expr => $error:pat) => { match ParseError::from($from) { e @ $error => { assert!(format!("{}", e).len() >= 5); @@ -861,7 +861,7 @@ mod tests { } macro_rules! from_and_cause { - ($from: expr => $error: pat) => { + ($from:expr => $error:pat) => { match ParseError::from($from) { e @ $error => { let desc = format!("{}", e.cause().unwrap()); diff --git a/src/router.rs b/src/router.rs index 2c7d5c32e..1e7126b63 100644 --- a/src/router.rs +++ b/src/router.rs @@ -142,7 +142,8 @@ enum PatternElement { #[derive(Clone, Debug)] enum PatternType { Static(String), - Dynamic(Regex, Vec), + Prefix(String), + Dynamic(Regex, Vec, usize), } #[derive(Debug, Copy, Clone, PartialEq)] @@ -173,14 +174,23 @@ impl Resource { /// /// Panics if path pattern is wrong. pub fn new(name: &str, path: &str) -> Self { - Resource::with_prefix(name, path, "/") + Resource::with_prefix(name, path, "/", false) + } + + /// Parse path pattern and create new `Resource` instance. + /// + /// Use `prefix` type instead of `static`. + /// + /// Panics if path regex pattern is wrong. + pub fn prefix(name: &str, path: &str) -> Self { + Resource::with_prefix(name, path, "/", true) } /// Construct external resource /// /// Panics if path pattern is wrong. pub fn external(name: &str, path: &str) -> Self { - let mut resource = Resource::with_prefix(name, path, "/"); + let mut resource = Resource::with_prefix(name, path, "/", false); resource.rtp = ResourceType::External; resource } @@ -197,8 +207,9 @@ impl Resource { } /// Parse path pattern and create new `Resource` instance with custom prefix - pub fn with_prefix(name: &str, path: &str, prefix: &str) -> Self { - let (pattern, elements, is_dynamic) = Resource::parse(path, prefix); + pub fn with_prefix(name: &str, path: &str, prefix: &str, for_prefix: bool) -> Self { + let (pattern, elements, is_dynamic, len) = + Resource::parse(path, prefix, for_prefix); let tp = if is_dynamic { let re = match Regex::new(&pattern) { @@ -208,7 +219,9 @@ impl Resource { let names = re.capture_names() .filter_map(|name| name.map(|name| name.to_owned())) .collect(); - PatternType::Dynamic(re, names) + PatternType::Dynamic(re, names, len) + } else if for_prefix { + PatternType::Prefix(pattern.clone()) } else { PatternType::Static(pattern.clone()) }; @@ -240,7 +253,8 @@ impl Resource { pub fn is_match(&self, path: &str) -> bool { match self.tp { PatternType::Static(ref s) => s == path, - PatternType::Dynamic(ref re, _) => re.is_match(path), + PatternType::Dynamic(ref re, _, _) => re.is_match(path), + PatternType::Prefix(ref s) => path.starts_with(s), } } @@ -249,7 +263,7 @@ impl Resource { ) -> bool { match self.tp { PatternType::Static(ref s) => s == path, - PatternType::Dynamic(ref re, ref names) => { + PatternType::Dynamic(ref re, ref names, _) => { if let Some(captures) = re.captures(path) { let mut idx = 0; for capture in captures.iter() { @@ -265,6 +279,42 @@ impl Resource { false } } + PatternType::Prefix(ref s) => path.starts_with(s), + } + } + + pub fn match_prefix_with_params<'a>( + &'a self, path: &'a str, params: &'a mut Params<'a>, + ) -> Option { + match self.tp { + PatternType::Static(ref s) => if s == path { + Some(s.len()) + } else { + None + }, + PatternType::Dynamic(ref re, ref names, len) => { + if let Some(captures) = re.captures(path) { + let mut idx = 0; + let mut pos = 0; + for capture in captures.iter() { + if let Some(ref m) = capture { + if idx != 0 { + params.add(names[idx - 1].as_str(), m.as_str()); + } + idx += 1; + pos = m.end(); + } + } + Some(pos + len) + } else { + None + } + } + PatternType::Prefix(ref s) => if path.starts_with(s) { + Some(s.len()) + } else { + None + }, } } @@ -297,7 +347,9 @@ impl Resource { Ok(path) } - fn parse(pattern: &str, prefix: &str) -> (String, Vec, bool) { + fn parse( + pattern: &str, prefix: &str, for_prefix: bool, + ) -> (String, Vec, bool, usize) { const DEFAULT_PATTERN: &str = "[^/]+"; let mut re1 = String::from("^") + prefix; @@ -309,6 +361,7 @@ impl Resource { let mut param_pattern = String::from(DEFAULT_PATTERN); let mut is_dynamic = false; let mut elems = Vec::new(); + let mut len = 0; for (index, ch) in pattern.chars().enumerate() { // All routes must have a leading slash so its optional to have one @@ -325,6 +378,7 @@ impl Resource { param_name.clear(); param_pattern = String::from(DEFAULT_PATTERN); + len = 0; in_param_pattern = false; in_param = false; } else if ch == ':' { @@ -348,16 +402,19 @@ impl Resource { re1.push_str(escape(&ch.to_string()).as_str()); re2.push(ch); el.push(ch); + len += 1; } } let re = if is_dynamic { - re1.push('$'); + if !for_prefix { + re1.push('$'); + } re1 } else { re2 }; - (re, elems, is_dynamic) + (re, elems, is_dynamic, len) } } @@ -570,6 +627,41 @@ mod tests { assert_eq!(req.match_info().get("id").unwrap(), "adahg32"); } + #[test] + fn test_resource_prefix() { + let re = Resource::prefix("test", "/name"); + assert!(re.is_match("/name")); + assert!(re.is_match("/name/")); + assert!(re.is_match("/name/test/test")); + assert!(re.is_match("/name1")); + assert!(re.is_match("/name~")); + + let re = Resource::prefix("test", "/name/"); + assert!(re.is_match("/name/")); + assert!(re.is_match("/name/gs")); + assert!(!re.is_match("/name")); + } + + #[test] + fn test_reousrce_prefix_dynamic() { + let re = Resource::prefix("test", "/{name}/"); + assert!(re.is_match("/name/")); + assert!(re.is_match("/name/gs")); + assert!(!re.is_match("/name")); + + let mut req = TestRequest::with_uri("/test2/").finish(); + assert!(re.match_with_params("/test2/", req.match_info_mut())); + assert_eq!(&req.match_info()["name"], "test2"); + + let mut req = + TestRequest::with_uri("/test2/subpath1/subpath2/index.html").finish(); + assert!(re.match_with_params( + "/test2/subpath1/subpath2/index.html", + req.match_info_mut() + )); + assert_eq!(&req.match_info()["name"], "test2"); + } + #[test] fn test_request_resource() { let routes = vec![ diff --git a/src/scope.rs b/src/scope.rs index fedc7ceda..b9ee0e8e9 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -21,7 +21,12 @@ type ScopeResources = Rc>>)>> /// /// Scope is a set of resources with common root path. /// Scopes collect multiple paths under a common path prefix. -/// Scope path can not contain variable path segments as resources. +/// Scope path can contain variable path segments as resources. +/// Scope prefix is always complete path segment, i.e `/app` would +/// be converted to a `/app/` and it would not match `/app` path. +/// +/// You can use variable path segments with `Path` extractor, also variable +/// segments are available in `HttpRequest::match_info()`. /// /// ```rust /// # extern crate actix_web; @@ -29,7 +34,7 @@ type ScopeResources = Rc>>)>> /// /// fn main() { /// let app = App::new() -/// .scope("/app", |scope| { +/// .scope("/{project_id}/", |scope| { /// scope.resource("/path1", |r| r.f(|_| HttpResponse::Ok())) /// .resource("/path2", |r| r.f(|_| HttpResponse::Ok())) /// .resource("/path3", |r| r.f(|_| HttpResponse::MethodNotAllowed())) @@ -38,12 +43,12 @@ type ScopeResources = Rc>>)>> /// ``` /// /// In the above example three routes get registered: -/// * /app/path1 - reponds to all http method -/// * /app/path2 - `GET` requests -/// * /app/path3 - `HEAD` requests +/// * /{project_id}/path1 - reponds to all http method +/// * /{project_id}/path2 - `GET` requests +/// * /{project_id}/path3 - `HEAD` requests /// pub struct Scope { - nested: Vec<(String, Route)>, + nested: Vec<(Resource, Route)>, middlewares: Rc>>>, default: Rc>>, resources: ScopeResources, @@ -102,12 +107,16 @@ impl Scope { if !path.is_empty() && !path.starts_with('/') { path.insert(0, '/') } + if !path.ends_with('/') { + path.push('/'); + } let handler = UnsafeCell::new(Box::new(Wrapper { scope, state: Rc::new(state), })); - self.nested.push((path, handler)); + self.nested + .push((Resource::prefix("", &path), handler)); self } @@ -149,9 +158,14 @@ impl Scope { if !path.is_empty() && !path.starts_with('/') { path.insert(0, '/') } + if !path.ends_with('/') { + path.push('/'); + } - self.nested - .push((path, UnsafeCell::new(Box::new(scope)))); + self.nested.push(( + Resource::prefix("", &path), + UnsafeCell::new(Box::new(scope)), + )); self } @@ -298,17 +312,14 @@ impl RouteHandler for Scope { } // nested scopes - for &(ref prefix, ref handler) in &self.nested { - let len = req.prefix_len() as usize; - let m = { - let path = &req.path()[len..]; - path.starts_with(prefix) - && (path.len() == prefix.len() - || path.split_at(prefix.len()).1.starts_with('/')) - }; + let len = req.prefix_len() as usize; + let path: &'static str = unsafe { &*(&req.path()[len..] as *const _) }; - if m { - let prefix_len = len + prefix.len(); + for &(ref prefix, ref handler) in &self.nested { + if let Some(prefix_len) = + prefix.match_prefix_with_params(path, req.match_info_mut()) + { + let prefix_len = len + prefix_len - 1; let path: &'static str = unsafe { &*(&req.path()[prefix_len..] as *const _) }; @@ -698,7 +709,10 @@ impl Response { #[cfg(test)] mod tests { + use bytes::Bytes; + use application::App; + use body::Body; use http::StatusCode; use httpresponse::HttpResponse; use test::TestRequest; @@ -716,6 +730,36 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::OK); } + #[test] + fn test_scope_variable_segment() { + let mut app = App::new() + .scope("/ab-{project}", |scope| { + scope.resource("/path1", |r| { + r.f(|r| { + HttpResponse::Ok() + .body(format!("project: {}", &r.match_info()["project"])) + }) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/ab-project1/path1").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + + match resp.as_msg().body() { + Body::Binary(ref b) => { + let bytes: Bytes = b.clone().into(); + assert_eq!(bytes, Bytes::from_static(b"project: project1")); + } + _ => panic!(), + } + + let req = TestRequest::with_uri("/aa-project1/path1").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + } + #[test] fn test_scope_with_state() { struct State; @@ -748,6 +792,73 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::CREATED); } + #[test] + fn test_nested_scope_with_variable_segment() { + let mut app = App::new() + .scope("/app", |scope| { + scope.nested("/{project_id}", |scope| { + scope.resource("/path1", |r| { + r.f(|r| { + HttpResponse::Created().body(format!( + "project: {}", + &r.match_info()["project_id"] + )) + }) + }) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/project_1/path1").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::CREATED); + + match resp.as_msg().body() { + Body::Binary(ref b) => { + let bytes: Bytes = b.clone().into(); + assert_eq!(bytes, Bytes::from_static(b"project: project_1")); + } + _ => panic!(), + } + } + + #[test] + fn test_nested2_scope_with_variable_segment() { + let mut app = App::new() + .scope("/app", |scope| { + scope.nested("/{project}", |scope| { + scope.nested("/{id}", |scope| { + scope.resource("/path1", |r| { + r.f(|r| { + HttpResponse::Created().body(format!( + "project: {} - {}", + &r.match_info()["project"], + &r.match_info()["id"], + )) + }) + }) + }) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/test/1/path1").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::CREATED); + + match resp.as_msg().body() { + Body::Binary(ref b) => { + let bytes: Bytes = b.clone().into(); + assert_eq!(bytes, Bytes::from_static(b"project: test - 1")); + } + _ => panic!(), + } + + let req = TestRequest::with_uri("/app/test/1/path2").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + } + #[test] fn test_default_resource() { let mut app = App::new() diff --git a/src/ws/context.rs b/src/ws/context.rs index 723b215ad..6114a0301 100644 --- a/src/ws/context.rs +++ b/src/ws/context.rs @@ -13,9 +13,9 @@ use context::{ActorHttpContext, Drain, Frame as ContextFrame}; use error::{Error, ErrorInternalServerError}; use httprequest::HttpRequest; -use ws::WsWriter; use ws::frame::Frame; use ws::proto::{CloseReason, OpCode}; +use ws::WsWriter; /// Execution context for `WebSockets` actors pub struct WebsocketContext diff --git a/tests/test_middleware.rs b/tests/test_middleware.rs index 9a6bf0040..99151afd7 100644 --- a/tests/test_middleware.rs +++ b/tests/test_middleware.rs @@ -551,7 +551,7 @@ fn test_async_middleware_multiple() { assert_eq!(num1.load(Ordering::Relaxed), 2); assert_eq!(num2.load(Ordering::Relaxed), 2); - thread::sleep(Duration::from_millis(30)); + thread::sleep(Duration::from_millis(50)); assert_eq!(num3.load(Ordering::Relaxed), 2); } From c755d71a8b25c4243232ff8599349ee9d603918a Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Mon, 7 May 2018 14:40:04 -0700 Subject: [PATCH 02/47] add filters support to scopes --- src/application.rs | 21 ++++-- src/scope.rs | 169 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 169 insertions(+), 21 deletions(-) diff --git a/src/application.rs b/src/application.rs index 20dfa7901..4b5747a4d 100644 --- a/src/application.rs +++ b/src/application.rs @@ -9,6 +9,7 @@ use httprequest::HttpRequest; use httpresponse::HttpResponse; use middleware::Middleware; use pipeline::{HandlerType, Pipeline, PipelineHandler}; +use pred::Predicate; use resource::ResourceHandler; use router::{Resource, Router}; use scope::Scope; @@ -34,7 +35,7 @@ pub(crate) struct Inner { enum PrefixHandlerType { Handler(String, Box>), - Scope(Resource, Box>), + Scope(Resource, Box>, Vec>>), } impl PipelineHandler for Inner { @@ -51,7 +52,7 @@ impl PipelineHandler for Inner { } HandlerType::Handler(idx) => match self.handlers[idx] { PrefixHandlerType::Handler(_, ref mut hnd) => hnd.handle(req), - PrefixHandlerType::Scope(_, ref mut hnd) => hnd.handle(req), + PrefixHandlerType::Scope(_, ref mut hnd, _) => hnd.handle(req), }, HandlerType::Default => self.default.handle(req, None), } @@ -73,8 +74,8 @@ impl HttpApplication { let path: &'static str = unsafe { &*(&req.path()[inner.prefix..] as *const _) }; let path_len = path.len(); - for idx in 0..inner.handlers.len() { - match &inner.handlers[idx] { + 'outer: for idx in 0..inner.handlers.len() { + match inner.handlers[idx] { PrefixHandlerType::Handler(ref prefix, _) => { let m = { path.starts_with(prefix) @@ -96,10 +97,16 @@ impl HttpApplication { return HandlerType::Handler(idx); } } - PrefixHandlerType::Scope(ref pattern, _) => { + PrefixHandlerType::Scope(ref pattern, _, ref filters) => { if let Some(prefix_len) = pattern.match_prefix_with_params(path, req.match_info_mut()) { + for filter in filters { + if !filter.check(req) { + continue 'outer; + } + } + let prefix_len = inner.prefix + prefix_len - 1; let path: &'static str = unsafe { &*(&req.path()[prefix_len..] as *const _) }; @@ -361,7 +368,7 @@ where F: FnOnce(Scope) -> Scope, { { - let scope = Box::new(f(Scope::new())); + let mut scope = Box::new(f(Scope::new())); let mut path = path.trim().trim_right_matches('/').to_owned(); if !path.is_empty() && !path.starts_with('/') { @@ -372,9 +379,11 @@ where } let parts = self.parts.as_mut().expect("Use after finish"); + let filters = scope.take_filters(); parts.handlers.push(PrefixHandlerType::Scope( Resource::prefix("", &path), scope, + filters, )); } self diff --git a/src/scope.rs b/src/scope.rs index b9ee0e8e9..74009841f 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,5 +1,6 @@ use std::cell::UnsafeCell; use std::marker::PhantomData; +use std::mem; use std::rc::Rc; use futures::{Async, Future, Poll}; @@ -11,11 +12,13 @@ use httprequest::HttpRequest; use httpresponse::HttpResponse; use middleware::{Finished as MiddlewareFinished, Middleware, Response as MiddlewareResponse, Started as MiddlewareStarted}; +use pred::Predicate; use resource::ResourceHandler; use router::Resource; type Route = UnsafeCell>>; type ScopeResources = Rc>>)>>; +type NestedInfo = (Resource, Route, Vec>>); /// Resources scope /// @@ -25,8 +28,8 @@ type ScopeResources = Rc>>)>> /// Scope prefix is always complete path segment, i.e `/app` would /// be converted to a `/app/` and it would not match `/app` path. /// -/// You can use variable path segments with `Path` extractor, also variable -/// segments are available in `HttpRequest::match_info()`. +/// You can get variable path segments from HttpRequest::match_info()`. +/// `Path` extractor also is able to extract scope level variable segments. /// /// ```rust /// # extern crate actix_web; @@ -48,7 +51,8 @@ type ScopeResources = Rc>>)>> /// * /{project_id}/path3 - `HEAD` requests /// pub struct Scope { - nested: Vec<(Resource, Route)>, + filters: Vec>>, + nested: Vec>, middlewares: Rc>>>, default: Rc>>, resources: ScopeResources, @@ -63,6 +67,7 @@ impl Default for Scope { impl Scope { pub fn new() -> Scope { Scope { + filters: Vec::new(), nested: Vec::new(), resources: Rc::new(Vec::new()), middlewares: Rc::new(Vec::new()), @@ -70,6 +75,36 @@ impl Scope { } } + #[inline] + pub(crate) fn take_filters(&mut self) -> Vec>> { + mem::replace(&mut self.filters, Vec::new()) + } + + /// Add match predicate to scoupe. + /// + /// ```rust + /// # extern crate actix_web; + /// use actix_web::{http, pred, App, HttpRequest, HttpResponse, Path}; + /// + /// fn index(data: Path<(String, String)>) -> &'static str { + /// "Welcome!" + /// } + /// + /// fn main() { + /// let app = App::new() + /// .scope("/app", |scope| { + /// scope.filter(pred::Header("content-type", "text/plain")) + /// .route("/test1", http::Method::GET, index) + /// .route("/test2", http::Method::POST, + /// |_: HttpRequest| HttpResponse::MethodNotAllowed()) + /// }); + /// } + /// ``` + pub fn filter + 'static>(mut self, p: T) -> Self { + self.filters.push(Box::new(p)); + self + } + /// Create nested scope with new state. /// /// ```rust @@ -96,12 +131,13 @@ impl Scope { F: FnOnce(Scope) -> Scope, { let scope = Scope { + filters: Vec::new(), nested: Vec::new(), resources: Rc::new(Vec::new()), middlewares: Rc::new(Vec::new()), default: Rc::new(UnsafeCell::new(ResourceHandler::default_not_found())), }; - let scope = f(scope); + let mut scope = f(scope); let mut path = path.trim().trim_right_matches('/').to_owned(); if !path.is_empty() && !path.starts_with('/') { @@ -111,12 +147,14 @@ impl Scope { path.push('/'); } - let handler = UnsafeCell::new(Box::new(Wrapper { - scope, - state: Rc::new(state), - })); + let state = Rc::new(state); + let filters: Vec>> = vec![Box::new(FiltersWrapper { + state: Rc::clone(&state), + filters: scope.take_filters(), + })]; + let handler = UnsafeCell::new(Box::new(Wrapper { scope, state })); self.nested - .push((Resource::prefix("", &path), handler)); + .push((Resource::prefix("", &path), handler, filters)); self } @@ -147,12 +185,13 @@ impl Scope { F: FnOnce(Scope) -> Scope, { let scope = Scope { + filters: Vec::new(), nested: Vec::new(), resources: Rc::new(Vec::new()), middlewares: Rc::new(Vec::new()), default: Rc::new(UnsafeCell::new(ResourceHandler::default_not_found())), }; - let scope = f(scope); + let mut scope = f(scope); let mut path = path.trim().trim_right_matches('/').to_owned(); if !path.is_empty() && !path.starts_with('/') { @@ -162,9 +201,11 @@ impl Scope { path.push('/'); } + let filters = scope.take_filters(); self.nested.push(( Resource::prefix("", &path), UnsafeCell::new(Box::new(scope)), + filters, )); self @@ -315,10 +356,15 @@ impl RouteHandler for Scope { let len = req.prefix_len() as usize; let path: &'static str = unsafe { &*(&req.path()[len..] as *const _) }; - for &(ref prefix, ref handler) in &self.nested { + 'outer: for &(ref prefix, ref handler, ref filters) in &self.nested { if let Some(prefix_len) = prefix.match_prefix_with_params(path, req.match_info_mut()) { + for filter in filters { + if !filter.check(&mut req) { + continue 'outer; + } + } let prefix_len = len + prefix_len - 1; let path: &'static str = unsafe { &*(&req.path()[prefix_len..] as *const _) }; @@ -363,6 +409,23 @@ impl RouteHandler for Wrapper { } } +struct FiltersWrapper { + state: Rc, + filters: Vec>>, +} + +impl Predicate for FiltersWrapper { + fn check(&self, req: &mut HttpRequest) -> bool { + let mut req = req.change_state(Rc::clone(&self.state)); + for filter in &self.filters { + if !filter.check(&mut req) { + return false; + } + } + true + } +} + /// Compose resource level middlewares with route handler. struct Compose { info: ComposeInfo, @@ -713,8 +776,9 @@ mod tests { use application::App; use body::Body; - use http::StatusCode; + use http::{Method, StatusCode}; use httpresponse::HttpResponse; + use pred; use test::TestRequest; #[test] @@ -730,6 +794,29 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::OK); } + #[test] + fn test_scope_filter() { + let mut app = App::new() + .scope("/app", |scope| { + scope + .filter(pred::Get()) + .resource("/path1", |r| r.f(|_| HttpResponse::Ok())) + }) + .finish(); + + let req = TestRequest::with_uri("/app/path1") + .method(Method::POST) + .finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + + let req = TestRequest::with_uri("/app/path1") + .method(Method::GET) + .finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + } + #[test] fn test_scope_variable_segment() { let mut app = App::new() @@ -748,7 +835,7 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::OK); match resp.as_msg().body() { - Body::Binary(ref b) => { + &Body::Binary(ref b) => { let bytes: Bytes = b.clone().into(); assert_eq!(bytes, Bytes::from_static(b"project: project1")); } @@ -777,6 +864,33 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::CREATED); } + #[test] + fn test_scope_with_state_filter() { + struct State; + + let mut app = App::new() + .scope("/app", |scope| { + scope.with_state("/t1", State, |scope| { + scope + .filter(pred::Get()) + .resource("/path1", |r| r.f(|_| HttpResponse::Ok())) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/t1/path1") + .method(Method::POST) + .finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + + let req = TestRequest::with_uri("/app/t1/path1") + .method(Method::GET) + .finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + } + #[test] fn test_nested_scope() { let mut app = App::new() @@ -792,6 +906,31 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::CREATED); } + #[test] + fn test_nested_scope_filter() { + let mut app = App::new() + .scope("/app", |scope| { + scope.nested("/t1", |scope| { + scope + .filter(pred::Get()) + .resource("/path1", |r| r.f(|_| HttpResponse::Ok())) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/t1/path1") + .method(Method::POST) + .finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + + let req = TestRequest::with_uri("/app/t1/path1") + .method(Method::GET) + .finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + } + #[test] fn test_nested_scope_with_variable_segment() { let mut app = App::new() @@ -814,7 +953,7 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::CREATED); match resp.as_msg().body() { - Body::Binary(ref b) => { + &Body::Binary(ref b) => { let bytes: Bytes = b.clone().into(); assert_eq!(bytes, Bytes::from_static(b"project: project_1")); } @@ -847,7 +986,7 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::CREATED); match resp.as_msg().body() { - Body::Binary(ref b) => { + &Body::Binary(ref b) => { let bytes: Bytes = b.clone().into(); assert_eq!(bytes, Bytes::from_static(b"project: test - 1")); } From 72908d974c642785f83a8b0d28d800ab26308cee Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Mon, 7 May 2018 15:19:03 -0700 Subject: [PATCH 03/47] test for Scope::route(); prep release --- CHANGES.md | 2 +- Cargo.toml | 2 +- src/scope.rs | 36 +++++++++++++++++++++++++++--------- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 3955251d7..c0ba7dd26 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,6 @@ # Changes -## 0.6.0 (...) +## 0.6.0 (2018-05-08) * Add route scopes #202 diff --git a/Cargo.toml b/Cargo.toml index a0bdf7088..fe63ef8a6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web" -version = "0.6.0-dev" +version = "0.6.0" authors = ["Nikolay Kim "] description = "Actix web is a simple, pragmatic and extremely fast web framework for Rust." readme = "README.md" diff --git a/src/scope.rs b/src/scope.rs index 74009841f..e5d160bbe 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -28,7 +28,7 @@ type NestedInfo = (Resource, Route, Vec>>); /// Scope prefix is always complete path segment, i.e `/app` would /// be converted to a `/app/` and it would not match `/app` path. /// -/// You can get variable path segments from HttpRequest::match_info()`. +/// You can get variable path segments from `HttpRequest::match_info()`. /// `Path` extractor also is able to extract scope level variable segments. /// /// ```rust @@ -50,6 +50,7 @@ type NestedInfo = (Resource, Route, Vec>>); /// * /{project_id}/path2 - `GET` requests /// * /{project_id}/path3 - `HEAD` requests /// +#[derive(Default)] pub struct Scope { filters: Vec>>, nested: Vec>, @@ -58,12 +59,7 @@ pub struct Scope { resources: ScopeResources, } -impl Default for Scope { - fn default() -> Scope { - Scope::new() - } -} - +#[cfg_attr(feature = "cargo-clippy", allow(new_without_default_derive))] impl Scope { pub fn new() -> Scope { Scope { @@ -319,7 +315,7 @@ impl Scope { /// middlewares get invoked on scope level. /// /// *Note* `Middleware::finish()` fires right after response get - /// prepared. It does not wait until body get sent to peer. + /// prepared. It does not wait until body get sent to the peer. pub fn middleware>(mut self, mw: M) -> Scope { Rc::get_mut(&mut self.middlewares) .expect("Can not use after configuration") @@ -333,7 +329,7 @@ impl RouteHandler for Scope { let path = unsafe { &*(&req.match_info()["tail"] as *const _) }; let path = if path == "" { "/" } else { path }; - // recognize paths + // recognize resources for &(ref pattern, ref resource) in self.resources.iter() { if pattern.match_with_params(path, req.match_info_mut()) { let default = unsafe { &mut *self.default.as_ref().get() }; @@ -777,6 +773,7 @@ mod tests { use application::App; use body::Body; use http::{Method, StatusCode}; + use httprequest::HttpRequest; use httpresponse::HttpResponse; use pred; use test::TestRequest; @@ -794,6 +791,27 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::OK); } + #[test] + fn test_scope_route() { + let mut app = App::new() + .scope("app", |scope| { + scope.route("/path1", Method::GET, |r: HttpRequest<_>| { + HttpResponse::Ok() + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/path1").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + + let req = TestRequest::with_uri("/app/path1") + .method(Method::POST) + .finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + } + #[test] fn test_scope_filter() { let mut app = App::new() From 3c6c1268c98225b153d6e7ae87b54477d4fc1fc5 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Mon, 7 May 2018 15:54:29 -0700 Subject: [PATCH 04/47] travis cover report --- .travis.yml | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index 415d792dc..94cdb08bc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,8 +31,17 @@ before_script: script: - | + if [[ "$TRAVIS_RUST_VERSION" != "beta" ]]; then cargo clean cargo test --features="alpn,tls" -- --nocapture + fi + - | + if [[ "$TRAVIS_RUST_VERSION" == "beta" ]]; then + bash <(curl https://raw.githubusercontent.com/xd009642/tarpaulin/master/travis-install.sh) + USE_SKEPTIC=1 cargo tarpaulin --out Xml + bash <(curl -s https://codecov.io/bash) + echo "Uploaded code coverage" + fi # Upload docs after_success: @@ -44,11 +53,3 @@ after_success: ./ghp-import/ghp_import.py -n -p -f -m "Documentation upload" -r https://"$GH_TOKEN"@github.com/"$TRAVIS_REPO_SLUG.git" target/doc && echo "Uploaded documentation" fi - - - | - if [[ "$TRAVIS_OS_NAME" == "linux" && "$TRAVIS_RUST_VERSION" == "beta" ]]; then - bash <(curl https://raw.githubusercontent.com/xd009642/tarpaulin/master/travis-install.sh) - USE_SKEPTIC=1 cargo tarpaulin --out Xml - bash <(curl -s https://codecov.io/bash) - echo "Uploaded code coverage" - fi From 8cda36286673c887e6ae4b4eb17d18cef71d4a42 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Mon, 7 May 2018 16:09:41 -0700 Subject: [PATCH 05/47] simplify pipeline --- src/pipeline.rs | 25 ++++++++----------------- src/route.rs | 21 ++++++--------------- src/scope.rs | 37 +++++++++++++++++++------------------ 3 files changed, 33 insertions(+), 50 deletions(-) diff --git a/src/pipeline.rs b/src/pipeline.rs index 1b621882f..25fe11e1d 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -251,23 +251,14 @@ impl> StartMiddlewares { Ok(Started::Response(resp)) => { return RunMiddlewares::init(info, resp) } - Ok(Started::Future(mut fut)) => match fut.poll() { - Ok(Async::NotReady) => { - return PipelineState::Starting(StartMiddlewares { - hnd, - htype, - fut: Some(fut), - _s: PhantomData, - }) - } - Ok(Async::Ready(resp)) => { - if let Some(resp) = resp { - return RunMiddlewares::init(info, resp); - } - info.count += 1; - } - Err(err) => return ProcessResponse::init(err.into()), - }, + Ok(Started::Future(fut)) => { + return PipelineState::Starting(StartMiddlewares { + hnd, + htype, + fut: Some(fut), + _s: PhantomData, + }) + } Err(err) => return ProcessResponse::init(err.into()), } } diff --git a/src/route.rs b/src/route.rs index d17f13f11..215a7f226 100644 --- a/src/route.rs +++ b/src/route.rs @@ -346,21 +346,12 @@ impl StartMiddlewares { Ok(MiddlewareStarted::Response(resp)) => { return RunMiddlewares::init(info, resp) } - Ok(MiddlewareStarted::Future(mut fut)) => match fut.poll() { - Ok(Async::NotReady) => { - return ComposeState::Starting(StartMiddlewares { - fut: Some(fut), - _s: PhantomData, - }) - } - Ok(Async::Ready(resp)) => { - if let Some(resp) = resp { - return RunMiddlewares::init(info, resp); - } - info.count += 1; - } - Err(err) => return FinishingMiddlewares::init(info, err.into()), - }, + Ok(MiddlewareStarted::Future(fut)) => { + return ComposeState::Starting(StartMiddlewares { + fut: Some(fut), + _s: PhantomData, + }) + } Err(err) => return FinishingMiddlewares::init(info, err.into()), } } diff --git a/src/scope.rs b/src/scope.rs index e5d160bbe..c399ac76a 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -521,21 +521,12 @@ impl StartMiddlewares { Ok(MiddlewareStarted::Response(resp)) => { return RunMiddlewares::init(info, resp) } - Ok(MiddlewareStarted::Future(mut fut)) => match fut.poll() { - Ok(Async::NotReady) => { - return ComposeState::Starting(StartMiddlewares { - fut: Some(fut), - _s: PhantomData, - }) - } - Ok(Async::Ready(resp)) => { - if let Some(resp) = resp { - return RunMiddlewares::init(info, resp); - } - info.count += 1; - } - Err(err) => return Response::init(err.into()), - }, + Ok(MiddlewareStarted::Future(fut)) => { + return ComposeState::Starting(StartMiddlewares { + fut: Some(fut), + _s: PhantomData, + }) + } Err(err) => return Response::init(err.into()), } } @@ -795,9 +786,13 @@ mod tests { fn test_scope_route() { let mut app = App::new() .scope("app", |scope| { - scope.route("/path1", Method::GET, |r: HttpRequest<_>| { - HttpResponse::Ok() - }) + scope + .route("/path1", Method::GET, |r: HttpRequest<_>| { + HttpResponse::Ok() + }) + .route("/path1", Method::DELETE, |r: HttpRequest<_>| { + HttpResponse::Ok() + }) }) .finish(); @@ -805,6 +800,12 @@ mod tests { let resp = app.run(req); assert_eq!(resp.as_msg().status(), StatusCode::OK); + let req = TestRequest::with_uri("/app/path1") + .method(Method::DELETE) + .finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + let req = TestRequest::with_uri("/app/path1") .method(Method::POST) .finish(); From ecda97aadd0c5bee5e66a89d32e62860f4e56096 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 8 May 2018 05:54:06 -0700 Subject: [PATCH 06/47] update doc string --- src/client/connector.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/client/connector.rs b/src/client/connector.rs index d9fa46d15..dea00ea21 100644 --- a/src/client/connector.rs +++ b/src/client/connector.rs @@ -347,6 +347,7 @@ impl ClientConnector { /// Keep-alive period is the period between connection usage. If /// the delay between repeated usages of the same connection /// exceeds this period, the connection is closed. + /// Default keep-alive period is 15 seconds. pub fn conn_keep_alive(mut self, dur: Duration) -> Self { self.conn_keep_alive = dur; self @@ -356,6 +357,7 @@ impl ClientConnector { /// /// Connection lifetime is max lifetime of any opened connection /// until it is closed regardless of keep-alive period. + /// Default lifetime period is 75 seconds. pub fn conn_lifetime(mut self, dur: Duration) -> Self { self.conn_lifetime = dur; self From b3cc43bb9b598e145c3c9e4be6ebc79acec28eff Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 8 May 2018 13:41:04 -0700 Subject: [PATCH 07/47] Fix connector's default keep-alive and lifetime settings #212 --- CHANGES.md | 4 ++++ MIGRATION.md | 14 ++++++++++++++ src/client/connector.rs | 8 ++++---- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c0ba7dd26..a96fd3318 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,9 @@ # Changes +## 0.6.1 (2018-05-xx) + +* Fix connector's default `keep-alive` and `lifetime` settings #212 + ## 0.6.0 (2018-05-08) * Add route scopes #202 diff --git a/MIGRATION.md b/MIGRATION.md index e2da4004d..984f3a3da 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -11,6 +11,17 @@ * `HttpRequest::extensions()` returns read only reference to the request's Extension `HttpRequest::extensions_mut()` returns mutable reference. +* Instead of + + `use actix_web::middleware::{ + CookieSessionBackend, CookieSessionError, RequestSession, + Session, SessionBackend, SessionImpl, SessionStorage};` + + use `actix_web::middleware::session` + + `use actix_web::middleware::session{CookieSessionBackend, CookieSessionError, + RequestSession, Session, SessionBackend, SessionImpl, SessionStorage};` + * `FromRequest::from_request()` accepts mutable reference to a request * `FromRequest::Result` has to implement `Into>` @@ -33,6 +44,9 @@ let q = Query::>::extract(req); ``` +* Websocket operations are implemented as `WsWriter` trait. + you need to use `use actix_web::ws::WsWriter` + ## Migration from 0.4 to 0.5 diff --git a/src/client/connector.rs b/src/client/connector.rs index dea00ea21..e082c9ed5 100644 --- a/src/client/connector.rs +++ b/src/client/connector.rs @@ -223,8 +223,8 @@ impl Default for ClientConnector { pool: Rc::new(Pool::new(Rc::clone(&_modified))), pool_modified: _modified, connector: builder.build().unwrap(), - conn_lifetime: Duration::from_secs(15), - conn_keep_alive: Duration::from_secs(75), + conn_lifetime: Duration::from_secs(75), + conn_keep_alive: Duration::from_secs(15), limit: 100, limit_per_host: 0, acquired: 0, @@ -243,8 +243,8 @@ impl Default for ClientConnector { subscriber: None, pool: Rc::new(Pool::new(Rc::clone(&_modified))), pool_modified: _modified, - conn_lifetime: Duration::from_secs(15), - conn_keep_alive: Duration::from_secs(75), + conn_lifetime: Duration::from_secs(75), + conn_keep_alive: Duration::from_secs(15), limit: 100, limit_per_host: 0, acquired: 0, From 6f75b0e95e11c6d34288796433795de3cfdbd617 Mon Sep 17 00:00:00 2001 From: Niklas Fiekas Date: Tue, 8 May 2018 22:45:28 +0200 Subject: [PATCH 08/47] let Path::from_request() fail with ErrorNotFound --- CHANGES.md | 2 ++ src/extractor.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a96fd3318..a42cbe6a0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,8 @@ * Fix connector's default `keep-alive` and `lifetime` settings #212 +* Send `ErrorNotFound` instead of `ErrorBadRequest` when path extractor fails #214 + ## 0.6.0 (2018-05-08) * Add route scopes #202 diff --git a/src/extractor.rs b/src/extractor.rs index 1f06f782d..a08e96674 100644 --- a/src/extractor.rs +++ b/src/extractor.rs @@ -11,7 +11,7 @@ use serde::de::{self, DeserializeOwned}; use serde_urlencoded; use de::PathDeserializer; -use error::{Error, ErrorBadRequest}; +use error::{Error, ErrorNotFound, ErrorBadRequest}; use handler::{AsyncResult, FromRequest}; use httpmessage::{HttpMessage, MessageBody, UrlEncoded}; use httprequest::HttpRequest; @@ -108,7 +108,7 @@ where fn from_request(req: &HttpRequest, _: &Self::Config) -> Self::Result { let req = req.clone(); de::Deserialize::deserialize(PathDeserializer::new(&req)) - .map_err(|e| e.into()) + .map_err(ErrorNotFound) .map(|inner| Path { inner }) } } From 47d80382b2830d075cf6298c5496c5eac2dc978f Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 8 May 2018 15:44:50 -0700 Subject: [PATCH 09/47] Fix http/2 payload streaming #215 --- CHANGES.md | 4 +++- Cargo.toml | 2 +- src/pipeline.rs | 4 ++-- src/scope.rs | 4 ++-- src/server/h2.rs | 25 ++++++++++++++----------- tests/test_server.rs | 2 +- 6 files changed, 23 insertions(+), 18 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a42cbe6a0..7be1c98ee 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,8 @@ # Changes -## 0.6.1 (2018-05-xx) +## 0.6.1 (2018-05-08) + +* Fix http/2 payload streaming #215 * Fix connector's default `keep-alive` and `lifetime` settings #212 diff --git a/Cargo.toml b/Cargo.toml index fe63ef8a6..d88fc4aaf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web" -version = "0.6.0" +version = "0.6.1" authors = ["Nikolay Kim "] description = "Actix web is a simple, pragmatic and extremely fast web framework for Rust." readme = "README.md" diff --git a/src/pipeline.rs b/src/pipeline.rs index 25fe11e1d..e00c06178 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -492,8 +492,8 @@ impl ProcessResponse { if let Some(err) = self.resp.error() { if self.resp.status().is_server_error() { error!( - "Error occured during request handling: {}", - err + "Error occured during request handling, status: {} {}", + self.resp.status(), err ); } else { warn!( diff --git a/src/scope.rs b/src/scope.rs index c399ac76a..c5aefb699 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -787,10 +787,10 @@ mod tests { let mut app = App::new() .scope("app", |scope| { scope - .route("/path1", Method::GET, |r: HttpRequest<_>| { + .route("/path1", Method::GET, |_: HttpRequest<_>| { HttpResponse::Ok() }) - .route("/path1", Method::DELETE, |r: HttpRequest<_>| { + .route("/path1", Method::DELETE, |_: HttpRequest<_>| { HttpResponse::Ok() }) }) diff --git a/src/server/h2.rs b/src/server/h2.rs index e2013357a..fc7824b22 100644 --- a/src/server/h2.rs +++ b/src/server/h2.rs @@ -343,24 +343,27 @@ impl Entry { } fn poll_payload(&mut self) { - if !self.flags.contains(EntryFlags::REOF) { - if self.payload.need_read() == PayloadStatus::Read { - if let Err(err) = self.recv.release_capacity().release_capacity(32_768) { - self.payload.set_error(PayloadError::Http2(err)) - } - } else if let Err(err) = self.recv.release_capacity().release_capacity(0) { - self.payload.set_error(PayloadError::Http2(err)) - } - + while !self.flags.contains(EntryFlags::REOF) + && self.payload.need_read() == PayloadStatus::Read + { match self.recv.poll() { Ok(Async::Ready(Some(chunk))) => { + let l = chunk.len(); self.payload.feed_data(chunk); + if let Err(err) = self.recv.release_capacity().release_capacity(l) { + self.payload.set_error(PayloadError::Http2(err)); + break; + } } Ok(Async::Ready(None)) => { self.flags.insert(EntryFlags::REOF); + self.payload.feed_eof(); + } + Ok(Async::NotReady) => break, + Err(err) => { + self.payload.set_error(PayloadError::Http2(err)); + break; } - Ok(Async::NotReady) => (), - Err(err) => self.payload.set_error(PayloadError::Http2(err)), } } } diff --git a/tests/test_server.rs b/tests/test_server.rs index e61cedd3b..863f800ac 100644 --- a/tests/test_server.rs +++ b/tests/test_server.rs @@ -809,7 +809,7 @@ fn test_h2() { }) }); let _res = core.run(tcp); - // assert_eq!(res.unwrap(), Bytes::from_static(STR.as_ref())); + // assert_eq!(_res.unwrap(), Bytes::from_static(STR.as_ref())); } #[test] From 54c33a7aff8063f60d6648aa50863081287cba96 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 8 May 2018 16:30:34 -0700 Subject: [PATCH 10/47] Allow to exclude certain endpoints from logging #211 --- CHANGES.md | 2 ++ src/middleware/logger.rs | 35 +++++++++++++++++++++++------------ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 7be1c98ee..cc8151dd9 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,6 +8,8 @@ * Send `ErrorNotFound` instead of `ErrorBadRequest` when path extractor fails #214 +* Allow to exclude certain endpoints from logging #211 + ## 0.6.0 (2018-05-08) * Add route scopes #202 diff --git a/src/middleware/logger.rs b/src/middleware/logger.rs index adfc3d2b0..086c232cc 100644 --- a/src/middleware/logger.rs +++ b/src/middleware/logger.rs @@ -1,7 +1,7 @@ //! Request logging middleware +use std::collections::HashSet; use std::env; -use std::fmt; -use std::fmt::{Display, Formatter}; +use std::fmt::{self, Display, Formatter}; use libc; use regex::Regex; @@ -74,6 +74,7 @@ use middleware::{Finished, Middleware, Started}; /// pub struct Logger { format: Format, + exclude: HashSet, } impl Logger { @@ -81,8 +82,15 @@ impl Logger { pub fn new(format: &str) -> Logger { Logger { format: Format::new(format), + exclude: HashSet::new(), } } + + /// Ignore and do not log access info for specified path. + pub fn exclude>(mut self, path: T) -> Self { + self.exclude.insert(path.into()); + self + } } impl Default for Logger { @@ -94,6 +102,7 @@ impl Default for Logger { fn default() -> Logger { Logger { format: Format::default(), + exclude: HashSet::new(), } } } @@ -102,21 +111,23 @@ struct StartTime(time::Tm); impl Logger { fn log(&self, req: &mut HttpRequest, resp: &HttpResponse) { - let entry_time = req.extensions().get::().unwrap().0; - - let render = |fmt: &mut Formatter| { - for unit in &self.format.0 { - unit.render(fmt, req, resp, entry_time)?; - } - Ok(()) - }; - info!("{}", FormatDisplay(&render)); + if let Some(entry_time) = req.extensions().get::() { + let render = |fmt: &mut Formatter| { + for unit in &self.format.0 { + unit.render(fmt, req, resp, entry_time.0)?; + } + Ok(()) + }; + info!("{}", FormatDisplay(&render)); + } } } impl Middleware for Logger { fn start(&self, req: &mut HttpRequest) -> Result { - req.extensions_mut().insert(StartTime(time::now())); + if !self.exclude.contains(req.path()) { + req.extensions_mut().insert(StartTime(time::now())); + } Ok(Started::Done) } From 7c395fcc83c6dfab8913a9e95007cccc1e9f2eba Mon Sep 17 00:00:00 2001 From: Luke Cowell Date: Tue, 8 May 2018 17:40:18 -0700 Subject: [PATCH 11/47] replace typo `scoupe` with `scope` --- src/scope.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scope.rs b/src/scope.rs index c5aefb699..aecfd6bfa 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -76,7 +76,7 @@ impl Scope { mem::replace(&mut self.filters, Vec::new()) } - /// Add match predicate to scoupe. + /// Add match predicate to scope. /// /// ```rust /// # extern crate actix_web; From c26c5fd9a4b47db5de0a22652021c3e897d3fe41 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 8 May 2018 18:34:36 -0700 Subject: [PATCH 12/47] prep release --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index cc8151dd9..a2d3ae25a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,7 @@ * Allow to exclude certain endpoints from logging #211 + ## 0.6.0 (2018-05-08) * Add route scopes #202 From 7c4941f8681caac7296c1d6312023fae62aa5653 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 8 May 2018 18:48:09 -0700 Subject: [PATCH 13/47] update migration doc --- MIGRATION.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MIGRATION.md b/MIGRATION.md index 984f3a3da..f6829a50a 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1,5 +1,7 @@ ## Migration from 0.5 to 0.6 +* `Path` extractor return `ErrorNotFound` on failure instead of `ErrorBadRequest` + * `ws::Message::Close` now includes optional close reason. `ws::CloseCode::Status` and `ws::CloseCode::Empty` have been removed. From be12d5e6fc8e3474e65f25a50f88d4668ad754b1 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 9 May 2018 05:48:06 -0700 Subject: [PATCH 14/47] make WsWriter trait optional --- CHANGES.md | 5 +++ src/lib.rs | 2 ++ src/ws/client.rs | 34 ++++++++++++++++-- src/ws/context.rs | 90 +++++++++++++++++++++++++++++++---------------- src/ws/mod.rs | 10 +++--- 5 files changed, 104 insertions(+), 37 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a2d3ae25a..51267c763 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,10 @@ # Changes +## 0.6.2 (2018-05-09) + +* WsWriter trait is optional. + + ## 0.6.1 (2018-05-08) * Fix http/2 payload streaming #215 diff --git a/src/lib.rs b/src/lib.rs index 92a319bcd..62d8fd833 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -175,6 +175,8 @@ pub use httprequest::HttpRequest; pub use httpresponse::HttpResponse; pub use json::Json; pub use scope::Scope; + +#[doc(hidden)] pub use ws::WsWriter; #[cfg(feature = "openssl")] diff --git a/src/ws/client.rs b/src/ws/client.rs index aa392a90a..780de03cc 100644 --- a/src/ws/client.rs +++ b/src/ws/client.rs @@ -518,9 +518,7 @@ impl ClientWriter { fn as_mut(&mut self) -> &mut Inner { unsafe { &mut *self.inner.get() } } -} -impl WsWriter for ClientWriter { /// Send text frame #[inline] fn text>(&mut self, text: T) { @@ -561,3 +559,35 @@ impl WsWriter for ClientWriter { self.write(Frame::close(reason, true)); } } + +impl WsWriter for ClientWriter { + /// Send text frame + #[inline] + fn send_text>(&mut self, text: T) { + self.text(text) + } + + /// Send binary frame + #[inline] + fn send_binary>(&mut self, data: B) { + self.binary(data) + } + + /// Send ping frame + #[inline] + fn send_ping(&mut self, message: &str) { + self.ping(message) + } + + /// Send pong frame + #[inline] + fn send_pong(&mut self, message: &str) { + self.pong(message) + } + + /// Send close frame + #[inline] + fn send_close(&mut self, reason: Option) { + self.close(reason); + } +} diff --git a/src/ws/context.rs b/src/ws/context.rs index 6114a0301..36f73e99e 100644 --- a/src/ws/context.rs +++ b/src/ws/context.rs @@ -149,36 +149,6 @@ where Drain::new(rx) } - /// Check if connection still open - #[inline] - pub fn connected(&self) -> bool { - !self.disconnected - } - - #[inline] - fn add_frame(&mut self, frame: ContextFrame) { - if self.stream.is_none() { - self.stream = Some(SmallVec::new()); - } - if let Some(s) = self.stream.as_mut() { - s.push(frame) - } - self.inner.modify(); - } - - /// Handle of the running future - /// - /// SpawnHandle is the handle returned by `AsyncContext::spawn()` method. - pub fn handle(&self) -> SpawnHandle { - self.inner.curr_handle() - } -} - -impl WsWriter for WebsocketContext -where - A: Actor, - S: 'static, -{ /// Send text frame #[inline] fn text>(&mut self, text: T) { @@ -218,6 +188,66 @@ where fn close(&mut self, reason: Option) { self.write(Frame::close(reason, false)); } + + /// Check if connection still open + #[inline] + pub fn connected(&self) -> bool { + !self.disconnected + } + + #[inline] + fn add_frame(&mut self, frame: ContextFrame) { + if self.stream.is_none() { + self.stream = Some(SmallVec::new()); + } + if let Some(s) = self.stream.as_mut() { + s.push(frame) + } + self.inner.modify(); + } + + /// Handle of the running future + /// + /// SpawnHandle is the handle returned by `AsyncContext::spawn()` method. + pub fn handle(&self) -> SpawnHandle { + self.inner.curr_handle() + } +} + +impl WsWriter for WebsocketContext +where + A: Actor, + S: 'static, +{ + /// Send text frame + #[inline] + fn send_text>(&mut self, text: T) { + self.text(text) + } + + /// Send binary frame + #[inline] + fn send_binary>(&mut self, data: B) { + self.binary(data) + } + + /// Send ping frame + #[inline] + fn send_ping(&mut self, message: &str) { + self.ping(message) + } + + /// Send pong frame + #[inline] + fn send_pong(&mut self, message: &str) { + self.pong(message) + } + + /// Send close frame + #[inline] + fn send_close(&mut self, reason: Option) { + self.close(reason) + } } impl ActorHttpContext for WebsocketContext diff --git a/src/ws/mod.rs b/src/ws/mod.rs index a39b95b1d..9fb40dd97 100644 --- a/src/ws/mod.rs +++ b/src/ws/mod.rs @@ -343,15 +343,15 @@ where /// Common writing methods for a websocket. pub trait WsWriter { /// Send a text - fn text>(&mut self, text: T); + fn send_text>(&mut self, text: T); /// Send a binary - fn binary>(&mut self, data: B); + fn send_binary>(&mut self, data: B); /// Send a ping message - fn ping(&mut self, message: &str); + fn send_ping(&mut self, message: &str); /// Send a pong message - fn pong(&mut self, message: &str); + fn send_pong(&mut self, message: &str); /// Close the connection - fn close(&mut self, reason: Option); + fn send_close(&mut self, reason: Option); } #[cfg(test)] From b748bf3b0d99ee1ea72ac46b7870a589014109e1 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 9 May 2018 06:05:16 -0700 Subject: [PATCH 15/47] make api public --- src/ws/client.rs | 10 +++++----- src/ws/context.rs | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ws/client.rs b/src/ws/client.rs index 780de03cc..92087efa5 100644 --- a/src/ws/client.rs +++ b/src/ws/client.rs @@ -521,19 +521,19 @@ impl ClientWriter { /// Send text frame #[inline] - fn text>(&mut self, text: T) { + pub fn text>(&mut self, text: T) { self.write(Frame::message(text.into(), OpCode::Text, true, true)); } /// Send binary frame #[inline] - fn binary>(&mut self, data: B) { + pub fn binary>(&mut self, data: B) { self.write(Frame::message(data, OpCode::Binary, true, true)); } /// Send ping frame #[inline] - fn ping(&mut self, message: &str) { + pub fn ping(&mut self, message: &str) { self.write(Frame::message( Vec::from(message), OpCode::Ping, @@ -544,7 +544,7 @@ impl ClientWriter { /// Send pong frame #[inline] - fn pong(&mut self, message: &str) { + pub fn pong(&mut self, message: &str) { self.write(Frame::message( Vec::from(message), OpCode::Pong, @@ -555,7 +555,7 @@ impl ClientWriter { /// Send close frame #[inline] - fn close(&mut self, reason: Option) { + pub fn close(&mut self, reason: Option) { self.write(Frame::close(reason, true)); } } diff --git a/src/ws/context.rs b/src/ws/context.rs index 36f73e99e..79c3aa356 100644 --- a/src/ws/context.rs +++ b/src/ws/context.rs @@ -151,19 +151,19 @@ where /// Send text frame #[inline] - fn text>(&mut self, text: T) { + pub fn text>(&mut self, text: T) { self.write(Frame::message(text.into(), OpCode::Text, true, false)); } /// Send binary frame #[inline] - fn binary>(&mut self, data: B) { + pub fn binary>(&mut self, data: B) { self.write(Frame::message(data, OpCode::Binary, true, false)); } /// Send ping frame #[inline] - fn ping(&mut self, message: &str) { + pub fn ping(&mut self, message: &str) { self.write(Frame::message( Vec::from(message), OpCode::Ping, @@ -174,7 +174,7 @@ where /// Send pong frame #[inline] - fn pong(&mut self, message: &str) { + pub fn pong(&mut self, message: &str) { self.write(Frame::message( Vec::from(message), OpCode::Pong, @@ -185,7 +185,7 @@ where /// Send close frame #[inline] - fn close(&mut self, reason: Option) { + pub fn close(&mut self, reason: Option) { self.write(Frame::close(reason, false)); } From b043c346326417782e7c1c176f025f3397a04248 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 9 May 2018 06:05:44 -0700 Subject: [PATCH 16/47] bump version --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d88fc4aaf..8c474cca1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web" -version = "0.6.1" +version = "0.6.2" authors = ["Nikolay Kim "] description = "Actix web is a simple, pragmatic and extremely fast web framework for Rust." readme = "README.md" From e58b38fd13eddf086f96919e99fd2c63cc8e058f Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 9 May 2018 06:12:16 -0700 Subject: [PATCH 17/47] deprecate WsWrite from top level mod --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index 62d8fd833..8436a278e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -177,6 +177,7 @@ pub use json::Json; pub use scope::Scope; #[doc(hidden)] +#[deprecated(since = "0.6.2", note = "please use `use actix_web::ws::WsWriter`")] pub use ws::WsWriter; #[cfg(feature = "openssl")] From 18575ee1ee26bfc54bea891d452b0a51f4b42b73 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 9 May 2018 16:27:31 -0700 Subject: [PATCH 18/47] Add Router::with_async() method for async handler registration --- CHANGES.md | 5 ++ build.rs | 11 +++- src/resource.rs | 21 +++++++ src/route.rs | 72 ++++++++++++++++++++- src/with.rs | 139 +++++++++++++++++++++++++++++++++++++++++ tests/test_handlers.rs | 78 +++++++++++++++++++++++ 6 files changed, 324 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 51267c763..cf4df3e36 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,10 @@ # Changes +## 0.6.3 (2018-05-xx) + +* Add `Router::with_async()` method for async handler registration. + + ## 0.6.2 (2018-05-09) * WsWriter trait is optional. diff --git a/build.rs b/build.rs index 3b3001f90..7cb25c731 100644 --- a/build.rs +++ b/build.rs @@ -1,8 +1,17 @@ extern crate version_check; fn main() { + let mut has_impl_trait = true; + + match version_check::is_min_version("1.26.0") { + Some((true, _)) => println!("cargo:rustc-cfg=actix_impl_trait"), + _ => (), + }; match version_check::is_nightly() { - Some(true) => println!("cargo:rustc-cfg=actix_nightly"), + Some(true) => { + println!("cargo:rustc-cfg=actix_nightly"); + println!("cargo:rustc-cfg=actix_impl_trait"); + } Some(false) => (), None => (), }; diff --git a/src/resource.rs b/src/resource.rs index fb08afd94..e52760f4e 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -1,9 +1,11 @@ use std::marker::PhantomData; use std::rc::Rc; +use futures::Future; use http::{Method, StatusCode}; use smallvec::SmallVec; +use error::Error; use handler::{AsyncResult, FromRequest, Handler, Responder}; use httprequest::HttpRequest; use httpresponse::HttpResponse; @@ -183,6 +185,25 @@ impl ResourceHandler { self.routes.last_mut().unwrap().with(handler); } + /// Register a new route and add async handler. + /// + /// This is shortcut for: + /// + /// ```rust,ignore + /// Application::resource("/", |r| r.route().with_async(index) + /// ``` + pub fn with_async(&mut self, handler: F) + where + F: Fn(T) -> R + 'static, + R: Future + 'static, + I: Responder + 'static, + E: Into + 'static, + T: FromRequest + 'static, + { + self.routes.push(Route::default()); + self.routes.last_mut().unwrap().with_async(handler); + } + /// Register a resource middleware /// /// This is similar to `App's` middlewares, but diff --git a/src/route.rs b/src/route.rs index 215a7f226..4ff3279eb 100644 --- a/src/route.rs +++ b/src/route.rs @@ -13,7 +13,7 @@ use httpresponse::HttpResponse; use middleware::{Finished as MiddlewareFinished, Middleware, Response as MiddlewareResponse, Started as MiddlewareStarted}; use pred::Predicate; -use with::{ExtractorConfig, With, With2, With3}; +use with::{ExtractorConfig, With, With2, With3, WithAsync}; /// Resource route definition /// @@ -129,6 +129,34 @@ impl Route { /// |r| r.method(http::Method::GET).with(index)); // <- use `with` extractor /// } /// ``` + /// + /// It is possible to use tuples for specifing multiple extractors for one + /// handler function. + /// + /// ```rust + /// # extern crate bytes; + /// # extern crate actix_web; + /// # extern crate futures; + /// #[macro_use] extern crate serde_derive; + /// # use std::collections::HashMap; + /// use actix_web::{http, App, Query, Path, Result, Json}; + /// + /// #[derive(Deserialize)] + /// struct Info { + /// username: String, + /// } + /// + /// /// extract path info using serde + /// fn index(info: (Path, Query>, Json)) -> Result { + /// Ok(format!("Welcome {}!", info.0.username)) + /// } + /// + /// fn main() { + /// let app = App::new().resource( + /// "/{username}/index.html", // <- define path parameters + /// |r| r.method(http::Method::GET).with(index)); // <- use `with` extractor + /// } + /// ``` pub fn with(&mut self, handler: F) -> ExtractorConfig where F: Fn(T) -> R + 'static, @@ -140,6 +168,47 @@ impl Route { cfg } + /// Set async handler function, use request extractor for parameters. + /// + /// ```rust + /// # extern crate bytes; + /// # extern crate actix_web; + /// # extern crate futures; + /// #[macro_use] extern crate serde_derive; + /// use actix_web::{App, Path, Error, http}; + /// use futures::Future; + /// + /// #[derive(Deserialize)] + /// struct Info { + /// username: String, + /// } + /// + /// /// extract path info using serde + /// fn index(info: Path) -> Box> { + /// unimplemented!() + /// } + /// + /// fn main() { + /// let app = App::new().resource( + /// "/{username}/index.html", // <- define path parameters + /// |r| r.method(http::Method::GET) + /// .with_async(index)); // <- use `with` extractor + /// } + /// ``` + pub fn with_async(&mut self, handler: F) -> ExtractorConfig + where + F: Fn(T) -> R + 'static, + R: Future + 'static, + I: Responder + 'static, + E: Into + 'static, + T: FromRequest + 'static, + { + let cfg = ExtractorConfig::default(); + self.h(WithAsync::new(handler, Clone::clone(&cfg))); + cfg + } + + #[doc(hidden)] /// Set handler function, use request extractor for both parameters. /// /// ```rust @@ -189,6 +258,7 @@ impl Route { (cfg1, cfg2) } + #[doc(hidden)] /// Set handler function, use request extractor for all parameters. pub fn with3( &mut self, handler: F, diff --git a/src/with.rs b/src/with.rs index fa3d7d802..dca600bbe 100644 --- a/src/with.rs +++ b/src/with.rs @@ -167,6 +167,145 @@ where } } +pub struct WithAsync +where + F: Fn(T) -> R, + R: Future, + I: Responder, + E: Into, + T: FromRequest, + S: 'static, +{ + hnd: Rc>, + cfg: ExtractorConfig, + _s: PhantomData, +} + +impl WithAsync +where + F: Fn(T) -> R, + R: Future, + I: Responder, + E: Into, + T: FromRequest, + S: 'static, +{ + pub fn new(f: F, cfg: ExtractorConfig) -> Self { + WithAsync { + cfg, + hnd: Rc::new(UnsafeCell::new(f)), + _s: PhantomData, + } + } +} + +impl Handler for WithAsync +where + F: Fn(T) -> R + 'static, + R: Future + 'static, + I: Responder + 'static, + E: Into + 'static, + T: FromRequest + 'static, + S: 'static, +{ + type Result = AsyncResult; + + fn handle(&mut self, req: HttpRequest) -> Self::Result { + let mut fut = WithAsyncHandlerFut { + req, + started: false, + hnd: Rc::clone(&self.hnd), + cfg: self.cfg.clone(), + fut1: None, + fut2: None, + fut3: None, + }; + + match fut.poll() { + Ok(Async::Ready(resp)) => AsyncResult::ok(resp), + Ok(Async::NotReady) => AsyncResult::async(Box::new(fut)), + Err(e) => AsyncResult::err(e), + } + } +} + +struct WithAsyncHandlerFut +where + F: Fn(T) -> R, + R: Future + 'static, + I: Responder + 'static, + E: Into + 'static, + T: FromRequest + 'static, + S: 'static, +{ + started: bool, + hnd: Rc>, + cfg: ExtractorConfig, + req: HttpRequest, + fut1: Option>>, + fut2: Option, + fut3: Option>>, +} + +impl Future for WithAsyncHandlerFut +where + F: Fn(T) -> R, + R: Future + 'static, + I: Responder + 'static, + E: Into + 'static, + T: FromRequest + 'static, + S: 'static, +{ + type Item = HttpResponse; + type Error = Error; + + fn poll(&mut self) -> Poll { + if let Some(ref mut fut) = self.fut3 { + return fut.poll(); + } + + if self.fut2.is_some() { + return match self.fut2.as_mut().unwrap().poll() { + Ok(Async::NotReady) => Ok(Async::NotReady), + Ok(Async::Ready(r)) => match r.respond_to(&self.req) { + Ok(r) => match r.into().into() { + AsyncResultItem::Err(err) => Err(err), + AsyncResultItem::Ok(resp) => Ok(Async::Ready(resp)), + AsyncResultItem::Future(fut) => { + self.fut3 = Some(fut); + self.poll() + } + }, + Err(e) => Err(e.into()), + }, + Err(e) => Err(e.into()), + }; + } + + let item = if !self.started { + self.started = true; + let reply = T::from_request(&self.req, self.cfg.as_ref()).into(); + match reply.into() { + AsyncResultItem::Err(err) => return Err(err), + AsyncResultItem::Ok(msg) => msg, + AsyncResultItem::Future(fut) => { + self.fut1 = Some(fut); + return self.poll(); + } + } + } else { + match self.fut1.as_mut().unwrap().poll()? { + Async::Ready(item) => item, + Async::NotReady => return Ok(Async::NotReady), + } + }; + + let hnd: &mut F = unsafe { &mut *self.hnd.get() }; + self.fut2 = Some((*hnd)(item)); + self.poll() + } +} + pub struct With2 where F: Fn(T1, T2) -> R, diff --git a/tests/test_handlers.rs b/tests/test_handlers.rs index 8aea34d0a..42a9f3ace 100644 --- a/tests/test_handlers.rs +++ b/tests/test_handlers.rs @@ -9,6 +9,7 @@ extern crate tokio_core; extern crate serde_derive; extern crate serde_json; +use std::io; use std::time::Duration; use actix::*; @@ -377,6 +378,83 @@ fn test_path_and_query_extractor2_async4() { assert_eq!(response.status(), StatusCode::BAD_REQUEST); } +#[cfg(actix_impl_trait)] +fn test_impl_trait( + data: (Json, Path, Query), +) -> impl Future { + Timeout::new(Duration::from_millis(10), &Arbiter::handle()) + .unwrap() + .and_then(move |_| { + Ok(format!( + "Welcome {} - {}!", + data.1.username, + (data.0).0 + )) + }) +} + +#[cfg(actix_impl_trait)] +fn test_impl_trait_err( + _data: (Json, Path, Query), +) -> impl Future { + Timeout::new(Duration::from_millis(10), &Arbiter::handle()) + .unwrap() + .and_then(move |_| Err(io::Error::new(io::ErrorKind::Other, "other"))) +} + +#[cfg(actix_impl_trait)] +#[test] +fn test_path_and_query_extractor2_async4_impl_trait() { + let mut srv = test::TestServer::new(|app| { + app.resource("/{username}/index.html", |r| { + r.route().with_async(test_impl_trait) + }); + }); + + // client request + let request = srv.post() + .uri(srv.url("/test1/index.html?username=test2")) + .header("content-type", "application/json") + .body("{\"test\": 1}") + .unwrap(); + let response = srv.execute(request.send()).unwrap(); + assert!(response.status().is_success()); + + // read response + let bytes = srv.execute(response.body()).unwrap(); + assert_eq!( + bytes, + Bytes::from_static(b"Welcome test1 - {\"test\":1}!") + ); + + // client request + let request = srv.get() + .uri(srv.url("/test1/index.html")) + .finish() + .unwrap(); + let response = srv.execute(request.send()).unwrap(); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); +} + +#[cfg(actix_impl_trait)] +#[test] +fn test_path_and_query_extractor2_async4_impl_trait_err() { + let mut srv = test::TestServer::new(|app| { + app.resource("/{username}/index.html", |r| { + r.route().with_async(test_impl_trait_err) + }); + }); + + // client request + let request = srv.post() + .uri(srv.url("/test1/index.html?username=test2")) + .header("content-type", "application/json") + .body("{\"test\": 1}") + .unwrap(); + let response = srv.execute(request.send()).unwrap(); + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); +} + #[test] fn test_non_ascii_route() { let mut srv = test::TestServer::new(|app| { From 8b473745cb4b27ae4773393cc0b0024bf389f196 Mon Sep 17 00:00:00 2001 From: dowwie Date: Thu, 10 May 2018 11:26:38 -0400 Subject: [PATCH 19/47] added error response functions for 501,502,503,504 --- src/error.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/error.rs b/src/error.rs index 8c46774e5..2ad4f6b91 100644 --- a/src/error.rs +++ b/src/error.rs @@ -757,6 +757,48 @@ where InternalError::new(err, StatusCode::INTERNAL_SERVER_ERROR).into() } +/// Helper function that creates wrapper of any error and +/// generate *NOT IMPLEMENTED* response. +#[allow(non_snake_case)] +pub fn ErrorNotImplemented(err: T) -> Error +where + T: Send + Sync + fmt::Debug + fmt::Display + 'static, +{ + InternalError::new(err, StatusCode::NOT_IMPLEMENTED).into() +} + +/// Helper function that creates wrapper of any error and +/// generate *BAD GATEWAY* response. +#[allow(non_snake_case)] +pub fn ErrorBadGateway(err: T) -> Error +where + T: Send + Sync + fmt::Debug + fmt::Display + 'static, +{ + InternalError::new(err, StatusCode::BAD_GATEWAY).into() +} + + +/// Helper function that creates wrapper of any error and +/// generate *SERVICE UNAVAILABLE* response. +#[allow(non_snake_case)] +pub fn ErrorServiceUnavailable(err: T) -> Error +where + T: Send + Sync + fmt::Debug + fmt::Display + 'static, +{ + InternalError::new(err, StatusCode::SERVICE_UNAVAILABLE).into() +} + +/// Helper function that creates wrapper of any error and +/// generate *GATEWAY TIMEOUT* response. +#[allow(non_snake_case)] +pub fn ErrorGatewayTimeout(err: T) -> Error +where + T: Send + Sync + fmt::Debug + fmt::Display + 'static, +{ + InternalError::new(err, StatusCode::GATEWAY_TIMEOUT).into() +} + + #[cfg(test)] mod tests { use super::*; From 2f244ea028280119022ac2be0b0d5cf63da86b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Gr=C3=B6ber?= Date: Thu, 10 May 2018 18:12:59 +0200 Subject: [PATCH 20/47] fix order of name and id in readme example --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 05757da48..096f98024 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ extern crate actix_web; use actix_web::{http, server, App, Path}; fn index(info: Path<(u32, String)>) -> String { - format!("Hello {}! id:{}", info.0, info.1) + format!("Hello {}! id:{}", info.1, info.0) } fn main() { From 76f021a6e3fcf6b44abc98d8615396234aadf44d Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 10 May 2018 09:13:26 -0700 Subject: [PATCH 21/47] add tests for ErrorXXX helpers --- CHANGES.md | 2 ++ build.rs | 2 -- src/error.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index cf4df3e36..db938c937 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,8 @@ * Add `Router::with_async()` method for async handler registration. +* Added error response functions for 501,502,503,504 + ## 0.6.2 (2018-05-09) diff --git a/build.rs b/build.rs index 7cb25c731..c8457944c 100644 --- a/build.rs +++ b/build.rs @@ -1,8 +1,6 @@ extern crate version_check; fn main() { - let mut has_impl_trait = true; - match version_check::is_min_version("1.26.0") { Some((true, _)) => println!("cargo:rustc-cfg=actix_impl_trait"), _ => (), diff --git a/src/error.rs b/src/error.rs index 2ad4f6b91..fe9796727 100644 --- a/src/error.rs +++ b/src/error.rs @@ -777,7 +777,6 @@ where InternalError::new(err, StatusCode::BAD_GATEWAY).into() } - /// Helper function that creates wrapper of any error and /// generate *SERVICE UNAVAILABLE* response. #[allow(non_snake_case)] @@ -798,7 +797,6 @@ where InternalError::new(err, StatusCode::GATEWAY_TIMEOUT).into() } - #[cfg(test)] mod tests { use super::*; @@ -954,4 +952,52 @@ mod tests { let resp: HttpResponse = err.error_response(); assert_eq!(resp.status(), StatusCode::OK); } + + #[test] + fn test_error_helpers() { + let r: HttpResponse = ErrorBadRequest("err").into(); + assert_eq!(r.status(), StatusCode::BAD_REQUEST); + + let r: HttpResponse = ErrorUnauthorized("err").into(); + assert_eq!(r.status(), StatusCode::UNAUTHORIZED); + + let r: HttpResponse = ErrorForbidden("err").into(); + assert_eq!(r.status(), StatusCode::FORBIDDEN); + + let r: HttpResponse = ErrorNotFound("err").into(); + assert_eq!(r.status(), StatusCode::NOT_FOUND); + + let r: HttpResponse = ErrorMethodNotAllowed("err").into(); + assert_eq!(r.status(), StatusCode::METHOD_NOT_ALLOWED); + + let r: HttpResponse = ErrorRequestTimeout("err").into(); + assert_eq!(r.status(), StatusCode::REQUEST_TIMEOUT); + + let r: HttpResponse = ErrorConflict("err").into(); + assert_eq!(r.status(), StatusCode::CONFLICT); + + let r: HttpResponse = ErrorGone("err").into(); + assert_eq!(r.status(), StatusCode::GONE); + + let r: HttpResponse = ErrorPreconditionFailed("err").into(); + assert_eq!(r.status(), StatusCode::PRECONDITION_FAILED); + + let r: HttpResponse = ErrorExpectationFailed("err").into(); + assert_eq!(r.status(), StatusCode::EXPECTATION_FAILED); + + let r: HttpResponse = ErrorInternalServerError("err").into(); + assert_eq!(r.status(), StatusCode::INTERNAL_SERVER_ERROR); + + let r: HttpResponse = ErrorNotImplemented("err").into(); + assert_eq!(r.status(), StatusCode::NOT_IMPLEMENTED); + + let r: HttpResponse = ErrorBadGateway("err").into(); + assert_eq!(r.status(), StatusCode::BAD_GATEWAY); + + let r: HttpResponse = ErrorServiceUnavailable("err").into(); + assert_eq!(r.status(), StatusCode::SERVICE_UNAVAILABLE); + + let r: HttpResponse = ErrorGatewayTimeout("err").into(); + assert_eq!(r.status(), StatusCode::GATEWAY_TIMEOUT); + } } From 92f993e05438c9c989fd32f91d12011bf21add52 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 10 May 2018 09:37:38 -0700 Subject: [PATCH 22/47] Fix client request timeout handling --- CHANGES.md | 2 ++ src/client/mod.rs | 1 + src/client/pipeline.rs | 12 ++++++------ 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index db938c937..fd7f0e2db 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,8 @@ * Added error response functions for 501,502,503,504 +* Fix client request timeout handling + ## 0.6.2 (2018-05-09) diff --git a/src/client/mod.rs b/src/client/mod.rs index 436fcf206..2116ae360 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -49,6 +49,7 @@ use httpresponse::HttpResponse; impl ResponseError for SendRequestError { fn error_response(&self) -> HttpResponse { match *self { + SendRequestError::Timeout => HttpResponse::GatewayTimeout(), SendRequestError::Connector(_) => HttpResponse::BadGateway(), _ => HttpResponse::InternalServerError(), }.into() diff --git a/src/client/pipeline.rs b/src/client/pipeline.rs index 60eb4f8c2..6a36bdd23 100644 --- a/src/client/pipeline.rs +++ b/src/client/pipeline.rs @@ -194,6 +194,7 @@ impl Future for SendRequest { self.state = State::Send(pl); } State::Send(mut pl) => { + pl.poll_timeout()?; pl.poll_write().map_err(|e| { io::Error::new(io::ErrorKind::Other, format!("{}", e).as_str()) })?; @@ -315,7 +316,7 @@ impl Pipeline { { Async::NotReady => need_run = true, Async::Ready(_) => { - let _ = self.poll_timeout().map_err(|e| { + self.poll_timeout().map_err(|e| { io::Error::new(io::ErrorKind::Other, format!("{}", e)) })?; } @@ -371,16 +372,15 @@ impl Pipeline { } } - fn poll_timeout(&mut self) -> Poll<(), SendRequestError> { + fn poll_timeout(&mut self) -> Result<(), SendRequestError> { if self.timeout.is_some() { match self.timeout.as_mut().unwrap().poll() { - Ok(Async::Ready(())) => Err(SendRequestError::Timeout), - Ok(Async::NotReady) => Ok(Async::NotReady), + Ok(Async::Ready(())) => return Err(SendRequestError::Timeout), + Ok(Async::NotReady) => (), Err(_) => unreachable!(), } - } else { - Ok(Async::NotReady) } + Ok(()) } #[inline] From d8fa43034f5d988b196f7f3c244ffdb6f40766ae Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 10 May 2018 11:00:22 -0700 Subject: [PATCH 23/47] export ExtractorConfig type --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index 8436a278e..e050130fe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -213,6 +213,7 @@ pub mod dev { pub use resource::ResourceHandler; pub use route::Route; pub use router::{Resource, ResourceType, Router}; + pub use with::ExtractorConfig; } pub mod http { From b6039b0bff0d1f4ec79a81a19fbdd95a7abbefac Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 10 May 2018 11:04:03 -0700 Subject: [PATCH 24/47] add doc string --- src/with.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/with.rs b/src/with.rs index dca600bbe..099bcea4f 100644 --- a/src/with.rs +++ b/src/with.rs @@ -9,6 +9,36 @@ use handler::{AsyncResult, AsyncResultItem, FromRequest, Handler, Responder}; use httprequest::HttpRequest; use httpresponse::HttpResponse; +/// Extractor configuration +/// +/// `Route::with()` and `Route::with_async()` returns instance +/// of the `ExtractorConfig` type. It could be used for extractor configuration. +/// +/// In this example `Form` configured. +/// +/// ```rust +/// # extern crate actix_web; +/// #[macro_use] extern crate serde_derive; +/// use actix_web::{App, Form, Result, http}; +/// +/// #[derive(Deserialize)] +/// struct FormData { +/// username: String, +/// } +/// +/// fn index(form: Form) -> Result { +/// Ok(format!("Welcome {}!", form.username)) +/// } +/// +/// fn main() { +/// let app = App::new().resource( +/// "/index.html", |r| { +/// r.method(http::Method::GET) +/// .with(index) +/// .limit(4096);} // <- change form extractor configuration +/// ); +/// } +/// ``` pub struct ExtractorConfig> { cfg: Rc>, } From 4b1a471b35881937a62f3f511051e02e7252cd50 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 10 May 2018 13:03:43 -0700 Subject: [PATCH 25/47] add more examples for extractor config --- src/with.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/with.rs b/src/with.rs index 099bcea4f..fa4f4dc41 100644 --- a/src/with.rs +++ b/src/with.rs @@ -39,6 +39,32 @@ use httpresponse::HttpResponse; /// ); /// } /// ``` +/// +/// Same could be donce with multiple extractors +/// +/// ```rust +/// # extern crate actix_web; +/// #[macro_use] extern crate serde_derive; +/// use actix_web::{App, Form, Path, Result, http}; +/// +/// #[derive(Deserialize)] +/// struct FormData { +/// username: String, +/// } +/// +/// fn index(data: (Path<(String,)>, Form)) -> Result { +/// Ok(format!("Welcome {}!", data.1.username)) +/// } +/// +/// fn main() { +/// let app = App::new().resource( +/// "/index.html", |r| { +/// r.method(http::Method::GET) +/// .with(index) +/// .1.limit(4096);} // <- change form extractor configuration +/// ); +/// } +/// ``` pub struct ExtractorConfig> { cfg: Rc>, } From 961969854353911be6207c05315d3b1bc86d9f0b Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 10 May 2018 13:04:56 -0700 Subject: [PATCH 26/47] doc string --- src/route.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/route.rs b/src/route.rs index 4ff3279eb..1322d1087 100644 --- a/src/route.rs +++ b/src/route.rs @@ -169,6 +169,8 @@ impl Route { } /// Set async handler function, use request extractor for parameters. + /// Also this method needs to be used if your handler function returns + /// `impl Future<>` /// /// ```rust /// # extern crate bytes; From a38afa0cec54ffe3d25ee0505e65918e6cc4b87e Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 10 May 2018 13:05:56 -0700 Subject: [PATCH 27/47] --no-count for tarpaulin --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 94cdb08bc..39f74d87d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -38,7 +38,7 @@ script: - | if [[ "$TRAVIS_RUST_VERSION" == "beta" ]]; then bash <(curl https://raw.githubusercontent.com/xd009642/tarpaulin/master/travis-install.sh) - USE_SKEPTIC=1 cargo tarpaulin --out Xml + USE_SKEPTIC=1 cargo tarpaulin --out Xml --no-count bash <(curl -s https://codecov.io/bash) echo "Uploaded code coverage" fi From 095ad328ee7e3d24b23cef3493ce73fa1340dfd8 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 10 May 2018 15:45:06 -0700 Subject: [PATCH 28/47] prepare release --- CHANGES.md | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index fd7f0e2db..df577a991 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,6 @@ # Changes -## 0.6.3 (2018-05-xx) +## 0.6.3 (2018-05-10) * Add `Router::with_async()` method for async handler registration. diff --git a/Cargo.toml b/Cargo.toml index 8c474cca1..24a500518 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web" -version = "0.6.2" +version = "0.6.3" authors = ["Nikolay Kim "] description = "Actix web is a simple, pragmatic and extremely fast web framework for Rust." readme = "README.md" From 487a713ca0b29e07b13fef45984927e9f279eb88 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Fri, 11 May 2018 15:01:15 -0700 Subject: [PATCH 29/47] update doc string --- src/handler.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/handler.rs b/src/handler.rs index 7b88248bf..a10a6f9c9 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -492,14 +492,15 @@ where /// } /// /// /// extract path info using serde -/// fn index(state: State, info: Path) -> String { -/// format!("{} {}!", state.msg, info.username) +/// fn index(data: (State, Path)) -> String { +/// let (state, path) = data; +/// format!("{} {}!", state.msg, path.username) /// } /// /// fn main() { /// let app = App::with_state(MyApp{msg: "Welcome"}).resource( /// "/{username}/index.html", // <- define path parameters -/// |r| r.method(http::Method::GET).with2(index)); // <- use `with` extractor +/// |r| r.method(http::Method::GET).with(index)); // <- use `with` extractor /// } /// ``` pub struct State(HttpRequest); From f735da504b2842428563201f1b60534d6bc96570 Mon Sep 17 00:00:00 2001 From: skorgu Date: Fri, 11 May 2018 20:36:54 -0400 Subject: [PATCH 30/47] Include mention of http client in README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 096f98024..00d9953ae 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ Actix web is a simple, pragmatic and extremely fast web framework for Rust. [CORS](https://actix.rs/actix-web/actix_web/middleware/cors/index.html), [CSRF](https://actix.rs/actix-web/actix_web/middleware/csrf/index.html)) * Built on top of [Actix actor framework](https://github.com/actix/actix) +* Includes an asynchronous [HTTP client](https://github.com/actix/actix-web/blob/master/src/client/mod.rs) ## Documentation & community resources From 9306631d6e3345e4a79a66f79886b67157fe751a Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Fri, 11 May 2018 21:19:48 -0700 Subject: [PATCH 31/47] Fix segfault in ServerSettings::get_response_builder() --- CHANGES.md | 5 +++++ Cargo.toml | 2 +- src/server/settings.rs | 13 ++++++++++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index df577a991..f0f7cd7ef 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,10 @@ # Changes +## 0.6.4 (2018-05-11) + +* Fix segfault in ServerSettings::get_response_builder() + + ## 0.6.3 (2018-05-10) * Add `Router::with_async()` method for async handler registration. diff --git a/Cargo.toml b/Cargo.toml index 24a500518..6fd073553 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web" -version = "0.6.3" +version = "0.6.4" authors = ["Nikolay Kim "] description = "Actix web is a simple, pragmatic and extremely fast web framework for Rust." readme = "README.md" diff --git a/src/server/settings.rs b/src/server/settings.rs index cd17681ba..f75033c1b 100644 --- a/src/server/settings.rs +++ b/src/server/settings.rs @@ -16,7 +16,6 @@ use body::Body; use httpresponse::{HttpResponse, HttpResponseBuilder, HttpResponsePool}; /// Various server settings -#[derive(Clone)] pub struct ServerSettings { addr: Option, secure: bool, @@ -28,6 +27,18 @@ pub struct ServerSettings { unsafe impl Sync for ServerSettings {} unsafe impl Send for ServerSettings {} +impl Clone for ServerSettings { + fn clone(&self) -> Self { + ServerSettings { + addr: self.addr, + secure: self.secure, + host: self.host.clone(), + cpu_pool: self.cpu_pool.clone(), + responses: HttpResponsePool::pool(), + } + } +} + struct InnerCpuPool { cpu_pool: UnsafeCell>, } From d65a03f6ac3c7175e14b0d9be07038563bd954c4 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Sun, 13 May 2018 08:43:09 -0700 Subject: [PATCH 32/47] use latest nightly for appveyor --- .appveyor.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index e06f90ca6..7933588a3 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -31,13 +31,13 @@ environment: CHANNEL: beta # Nightly channel - TARGET: i686-pc-windows-gnu - CHANNEL: nightly-2017-12-21 + CHANNEL: nightly - TARGET: i686-pc-windows-msvc - CHANNEL: nightly-2017-12-21 + CHANNEL: nightly - TARGET: x86_64-pc-windows-gnu - CHANNEL: nightly-2017-12-21 + CHANNEL: nightly - TARGET: x86_64-pc-windows-msvc - CHANNEL: nightly-2017-12-21 + CHANNEL: nightly # Install Rust and Cargo # (Based on from https://github.com/rust-lang/libc/blob/master/appveyor.yml) From 5ea2d68438fe1812afd80a70c6d5cc7b427de19e Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 15 May 2018 07:55:36 -0700 Subject: [PATCH 33/47] h1 decoder blocks on error #222 --- src/server/h1.rs | 46 ++++++++++++++-------------------------------- 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/src/server/h1.rs b/src/server/h1.rs index 8d862b39b..b6de5cc53 100644 --- a/src/server/h1.rs +++ b/src/server/h1.rs @@ -67,7 +67,9 @@ where H: HttpHandler + 'static, { pub fn new( - settings: Rc>, stream: T, addr: Option, + settings: Rc>, + stream: T, + addr: Option, buf: BytesMut, ) -> Self { let bytes = settings.get_shared_bytes(); @@ -149,7 +151,8 @@ where pub fn poll_io(&mut self) { // read io from socket if !self.flags.intersects(Flags::ERROR) - && self.tasks.len() < MAX_PIPELINED_MESSAGES && self.can_read() + && self.tasks.len() < MAX_PIPELINED_MESSAGES + && self.can_read() { if self.read() { // notify all tasks @@ -205,8 +208,7 @@ where self.stream.reset(); if ready { - item.flags - .insert(EntryFlags::EOF | EntryFlags::FINISHED); + item.flags.insert(EntryFlags::EOF | EntryFlags::FINISHED); } else { item.flags.insert(EntryFlags::FINISHED); } @@ -347,6 +349,7 @@ where } else { error!("Internal server error: unexpected payload chunk"); self.flags.insert(Flags::ERROR); + break; } } Ok(Some(Message::Eof)) => { @@ -355,6 +358,7 @@ where } else { error!("Internal server error: unexpected eof"); self.flags.insert(Flags::ERROR); + break; } } Ok(None) => break, @@ -367,6 +371,7 @@ where }; payload.set_error(e); } + break; } } } @@ -614,10 +619,7 @@ mod tests { assert_eq!(req.version(), Version::HTTP_11); assert_eq!(*req.method(), Method::GET); assert_eq!(req.path(), "/test"); - assert_eq!( - req.headers().get("test").unwrap().as_bytes(), - b"value" - ); + assert_eq!(req.headers().get("test").unwrap().as_bytes(), b"value"); } Ok(_) | Err(_) => unreachable!("Error during parsing http request"), } @@ -918,13 +920,7 @@ mod tests { .as_ref(), b"line" ); - assert!( - reader - .decode(&mut buf, &settings) - .unwrap() - .unwrap() - .eof() - ); + assert!(reader.decode(&mut buf, &settings).unwrap().unwrap().eof()); } #[test] @@ -1005,13 +1001,7 @@ mod tests { assert!(reader.decode(&mut buf, &settings).unwrap().is_none()); buf.extend(b"\r\n"); - assert!( - reader - .decode(&mut buf, &settings) - .unwrap() - .unwrap() - .eof() - ); + assert!(reader.decode(&mut buf, &settings).unwrap().unwrap().eof()); } #[test] @@ -1029,17 +1019,9 @@ mod tests { assert!(req.chunked().unwrap()); buf.extend(b"4;test\r\ndata\r\n4\r\nline\r\n0\r\n\r\n"); // test: test\r\n\r\n") - let chunk = reader - .decode(&mut buf, &settings) - .unwrap() - .unwrap() - .chunk(); + let chunk = reader.decode(&mut buf, &settings).unwrap().unwrap().chunk(); assert_eq!(chunk, Bytes::from_static(b"data")); - let chunk = reader - .decode(&mut buf, &settings) - .unwrap() - .unwrap() - .chunk(); + let chunk = reader.decode(&mut buf, &settings).unwrap().unwrap().chunk(); assert_eq!(chunk, Bytes::from_static(b"line")); let msg = reader.decode(&mut buf, &settings).unwrap().unwrap(); assert!(msg.eof()); From 953a0d4e4ab1db724faea5b9196e391414e34920 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 15 May 2018 09:29:59 -0700 Subject: [PATCH 34/47] add test case for #222 --- CHANGES.md | 5 +++ src/server/h1.rs | 101 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index f0f7cd7ef..a8c4e1e50 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,10 @@ # Changes +## 0.6.5 (2018-05-15) + +* Fix error handling during request decoding #222 + + ## 0.6.4 (2018-05-11) * Fix segfault in ServerSettings::get_response_builder() diff --git a/src/server/h1.rs b/src/server/h1.rs index b6de5cc53..933ce0a80 100644 --- a/src/server/h1.rs +++ b/src/server/h1.rs @@ -403,8 +403,12 @@ where #[cfg(test)] mod tests { - use bytes::{Bytes, BytesMut}; + use std::net::Shutdown; + use std::{cmp, time}; + + use bytes::{Buf, Bytes, BytesMut}; use http::{Method, Version}; + use tokio_io::{AsyncRead, AsyncWrite}; use super::*; use application::HttpApplication; @@ -468,6 +472,101 @@ mod tests { }}; } + struct Buffer { + buf: Bytes, + err: Option, + } + + impl Buffer { + fn new(data: &'static str) -> Buffer { + Buffer { + buf: Bytes::from(data), + err: None, + } + } + fn feed_data(&mut self, data: &'static str) { + let mut b = BytesMut::from(self.buf.as_ref()); + b.extend(data.as_bytes()); + self.buf = b.take().freeze(); + } + } + + impl AsyncRead for Buffer {} + impl io::Read for Buffer { + fn read(&mut self, dst: &mut [u8]) -> Result { + if self.buf.is_empty() { + if self.err.is_some() { + Err(self.err.take().unwrap()) + } else { + Err(io::Error::new(io::ErrorKind::WouldBlock, "")) + } + } else { + let size = cmp::min(self.buf.len(), dst.len()); + let b = self.buf.split_to(size); + dst[..size].copy_from_slice(&b); + Ok(size) + } + } + } + + impl IoStream for Buffer { + fn shutdown(&mut self, _: Shutdown) -> io::Result<()> { + Ok(()) + } + fn set_nodelay(&mut self, _: bool) -> io::Result<()> { + Ok(()) + } + fn set_linger(&mut self, _: Option) -> io::Result<()> { + Ok(()) + } + } + impl io::Write for Buffer { + fn write(&mut self, buf: &[u8]) -> io::Result { + Ok(buf.len()) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + impl AsyncWrite for Buffer { + fn shutdown(&mut self) -> Poll<(), io::Error> { + Ok(Async::Ready(())) + } + fn write_buf(&mut self, _: &mut B) -> Poll { + Ok(Async::NotReady) + } + } + + #[test] + fn test_req_parse() { + let buf = Buffer::new("GET /test HTTP/1.1\r\n\r\n"); + let readbuf = BytesMut::new(); + let settings = Rc::new(WorkerSettings::::new( + Vec::new(), + KeepAlive::Os, + )); + + let mut h1 = Http1::new(Rc::clone(&settings), buf, None, readbuf); + h1.poll_io(); + h1.parse(); + assert_eq!(h1.tasks.len(), 1); + } + + #[test] + fn test_req_parse_err() { + let buf = Buffer::new("GET /test HTTP/1\r\n\r\n"); + let readbuf = BytesMut::new(); + let settings = Rc::new(WorkerSettings::::new( + Vec::new(), + KeepAlive::Os, + )); + + let mut h1 = Http1::new(Rc::clone(&settings), buf, None, readbuf); + h1.poll_io(); + h1.parse(); + assert!(h1.flags.contains(Flags::ERROR)); + } + #[test] fn test_parse() { let mut buf = BytesMut::from("GET /test HTTP/1.1\r\n\r\n"); From ef89430f9b63190973d1aac94ed53a7dc0042802 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 15 May 2018 09:53:58 -0700 Subject: [PATCH 35/47] undeprecate query() and store query in extensions --- MIGRATION.md | 2 +- src/httprequest.rs | 43 +++++++++++++++++++++---------------------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index f6829a50a..f93e83c20 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -32,7 +32,7 @@ https://actix.rs/actix-web/actix_web/trait.Responder.html#tymethod.respond_to) is generic over `S` -* `HttpRequest::query()` is deprecated. Use `Query` extractor. +* Use `Query` extractor instead of HttpRequest::query()`. ```rust fn index(q: Query>) -> Result<..> { diff --git a/src/httprequest.rs b/src/httprequest.rs index 229c06874..fbca52caf 100644 --- a/src/httprequest.rs +++ b/src/httprequest.rs @@ -27,7 +27,6 @@ use uri::Url as InnerUrl; bitflags! { pub(crate) struct MessageFlags: u8 { - const QUERY = 0b0000_0001; const KEEPALIVE = 0b0000_0010; } } @@ -41,7 +40,6 @@ pub struct HttpInnerMessage { pub extensions: Extensions, pub params: Params<'static>, pub cookies: Option>>, - pub query: Params<'static>, pub addr: Option, pub payload: Option, pub info: Option>, @@ -49,6 +47,8 @@ pub struct HttpInnerMessage { resource: RouterResource, } +struct Query(Params<'static>); + #[derive(Debug, Copy, Clone, PartialEq)] enum RouterResource { Notset, @@ -64,7 +64,6 @@ impl Default for HttpInnerMessage { headers: HeaderMap::with_capacity(16), flags: MessageFlags::empty(), params: Params::new(), - query: Params::new(), addr: None, cookies: None, payload: None, @@ -109,7 +108,10 @@ impl HttpRequest<()> { /// Construct a new Request. #[inline] pub fn new( - method: Method, uri: Uri, version: Version, headers: HeaderMap, + method: Method, + uri: Uri, + version: Version, + headers: HeaderMap, payload: Option, ) -> HttpRequest { let url = InnerUrl::new(uri); @@ -121,7 +123,6 @@ impl HttpRequest<()> { headers, payload, params: Params::new(), - query: Params::new(), extensions: Extensions::new(), cookies: None, addr: None, @@ -306,7 +307,9 @@ impl HttpRequest { /// } /// ``` pub fn url_for( - &self, name: &str, elements: U, + &self, + name: &str, + elements: U, ) -> Result where U: IntoIterator, @@ -369,20 +372,20 @@ impl HttpRequest { } #[doc(hidden)] - #[deprecated(since = "0.6.0", note = "please use `Query` extractor")] /// Get a reference to the Params object. /// Params is a container for url query parameters. - pub fn query(&self) -> &Params { - if !self.as_ref().flags.contains(MessageFlags::QUERY) { - self.as_mut().flags.insert(MessageFlags::QUERY); - let params: &mut Params = - unsafe { mem::transmute(&mut self.as_mut().query) }; - params.clear(); + pub fn query<'a>(&'a self) -> &'a Params { + if let None = self.extensions().get::() { + let mut params: Params<'a> = Params::new(); for (key, val) in form_urlencoded::parse(self.query_string().as_ref()) { params.add(key, val); } + let params: Params<'static> = unsafe { mem::transmute(params) }; + self.as_mut().extensions.insert(Query(params)); } - unsafe { mem::transmute(&self.as_ref().query) } + let params: &Params<'a> = + unsafe { mem::transmute(&self.extensions().get::().unwrap().0) }; + params } /// The query string in the URL. @@ -664,10 +667,8 @@ mod tests { let mut resource = ResourceHandler::<()>::default(); resource.name("index"); - let routes = vec![( - Resource::new("index", "/user/{name}.{ext}"), - Some(resource), - )]; + let routes = + vec![(Resource::new("index", "/user/{name}.{ext}"), Some(resource))]; let (router, _) = Router::new("/", ServerSettings::default(), routes); assert!(router.has_route("/user/test.html")); assert!(!router.has_route("/test/unknown")); @@ -696,10 +697,8 @@ mod tests { let mut resource = ResourceHandler::<()>::default(); resource.name("index"); - let routes = vec![( - Resource::new("index", "/user/{name}.{ext}"), - Some(resource), - )]; + let routes = + vec![(Resource::new("index", "/user/{name}.{ext}"), Some(resource))]; let (router, _) = Router::new("/prefix/", ServerSettings::default(), routes); assert!(router.has_route("/user/test.html")); assert!(!router.has_route("/prefix/user/test.html")); From b9d870645f1d25e1b981d0fa7c9afafbab601c74 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 15 May 2018 10:09:48 -0700 Subject: [PATCH 36/47] store cookies in extensions --- src/httprequest.rs | 17 ++++++++++------- src/test.rs | 15 +++++---------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/httprequest.rs b/src/httprequest.rs index fbca52caf..0c3ee31de 100644 --- a/src/httprequest.rs +++ b/src/httprequest.rs @@ -39,7 +39,6 @@ pub struct HttpInnerMessage { pub headers: HeaderMap, pub extensions: Extensions, pub params: Params<'static>, - pub cookies: Option>>, pub addr: Option, pub payload: Option, pub info: Option>, @@ -48,6 +47,7 @@ pub struct HttpInnerMessage { } struct Query(Params<'static>); +struct Cookies(Vec>); #[derive(Debug, Copy, Clone, PartialEq)] enum RouterResource { @@ -65,7 +65,6 @@ impl Default for HttpInnerMessage { flags: MessageFlags::empty(), params: Params::new(), addr: None, - cookies: None, payload: None, extensions: Extensions::new(), info: None, @@ -90,7 +89,6 @@ impl HttpInnerMessage { self.addr = None; self.info = None; self.flags = MessageFlags::empty(); - self.cookies = None; self.payload = None; self.prefix = 0; self.resource = RouterResource::Notset; @@ -124,7 +122,6 @@ impl HttpRequest<()> { payload, params: Params::new(), extensions: Extensions::new(), - cookies: None, addr: None, info: None, prefix: 0, @@ -402,7 +399,7 @@ impl HttpRequest { /// Load request cookies. pub fn cookies(&self) -> Result<&Vec>, CookieParseError> { - if self.as_ref().cookies.is_none() { + if let None = self.extensions().get::() { let msg = self.as_mut(); let mut cookies = Vec::new(); for hdr in msg.headers.get_all(header::COOKIE) { @@ -413,9 +410,9 @@ impl HttpRequest { } } } - msg.cookies = Some(cookies); + msg.extensions.insert(Cookies(cookies)); } - Ok(&self.as_ref().cookies.as_ref().unwrap()) + Ok(&self.extensions().get::().unwrap().0) } /// Return request cookie. @@ -430,6 +427,12 @@ impl HttpRequest { None } + pub(crate) fn set_cookies(&mut self, cookies: Option>>) { + if let Some(cookies) = cookies { + self.extensions_mut().insert(Cookies(cookies)); + } + } + /// Get a reference to the Params object. /// /// Params is a container for url parameters. diff --git a/src/test.rs b/src/test.rs index 4e7398396..135135f7e 100644 --- a/src/test.rs +++ b/src/test.rs @@ -355,12 +355,7 @@ impl TestApp { /// Register handler for "/" pub fn handler>(&mut self, handler: H) { - self.app = Some( - self.app - .take() - .unwrap() - .resource("/", |r| r.h(handler)), - ); + self.app = Some(self.app.take().unwrap().resource("/", |r| r.h(handler))); } /// Register middleware @@ -562,8 +557,8 @@ impl TestRequest { cookies, payload, } = self; - let req = HttpRequest::new(method, uri, version, headers, payload); - req.as_mut().cookies = cookies; + let mut req = HttpRequest::new(method, uri, version, headers, payload); + req.set_cookies(cookies); req.as_mut().params = params; let (router, _) = Router::new::("/", ServerSettings::default(), Vec::new()); req.with_state(Rc::new(state), router) @@ -583,8 +578,8 @@ impl TestRequest { payload, } = self; - let req = HttpRequest::new(method, uri, version, headers, payload); - req.as_mut().cookies = cookies; + let mut req = HttpRequest::new(method, uri, version, headers, payload); + req.set_cookies(cookies); req.as_mut().params = params; req.with_state(Rc::new(state), router) } From d6787e6c561e416f08d1dc5c1c4125aa17730c69 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 15 May 2018 10:20:32 -0700 Subject: [PATCH 37/47] prepare release --- Cargo.toml | 2 +- README.md | 2 +- src/lib.rs | 20 ++++++++++++++++++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6fd073553..595fc9a11 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web" -version = "0.6.4" +version = "0.6.5" authors = ["Nikolay Kim "] description = "Actix web is a simple, pragmatic and extremely fast web framework for Rust." readme = "README.md" diff --git a/README.md b/README.md index 00d9953ae..51a3ae35c 100644 --- a/README.md +++ b/README.md @@ -18,8 +18,8 @@ Actix web is a simple, pragmatic and extremely fast web framework for Rust. [DefaultHeaders](https://actix.rs/book/actix-web/sec-9-middlewares.html#default-headers), [CORS](https://actix.rs/actix-web/actix_web/middleware/cors/index.html), [CSRF](https://actix.rs/actix-web/actix_web/middleware/csrf/index.html)) -* Built on top of [Actix actor framework](https://github.com/actix/actix) * Includes an asynchronous [HTTP client](https://github.com/actix/actix-web/blob/master/src/client/mod.rs) +* Built on top of [Actix actor framework](https://github.com/actix/actix) ## Documentation & community resources diff --git a/src/lib.rs b/src/lib.rs index e050130fe..8ef1e2df8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,7 +60,21 @@ //! * Middlewares (`Logger`, `Session`, `CORS`, `CSRF`, `DefaultHeaders`) //! * Built on top of [Actix actor framework](https://github.com/actix/actix) //! * Supported Rust version: 1.24 or later - +//! +//! ## Package feature +//! +//! * `tls` - enables ssl support via `native-tls` crate +//! * `alpn` - enables ssl support via `openssl` crate, require for `http/2` +//! support +//! * `session` - enables session support, includes `ring` crate as +//! dependency +//! * `brotli` - enables `brotli` compression support, requires `c` +//! compiler +//! * `flate-c` - enables `gzip`, `deflate` compression support, requires +//! `c` compiler +//! * `flate-rust` - experimental rust based implementation for +//! `gzip`, `deflate` compression. +//! #![cfg_attr(actix_nightly, feature( specialization, // for impl ErrorResponse for std::error::Error ))] @@ -169,7 +183,9 @@ pub use body::{Binary, Body}; pub use context::HttpContext; pub use error::{Error, ResponseError, Result}; pub use extractor::{Form, Path, Query}; -pub use handler::{AsyncResponder, Either, FromRequest, FutureResponse, Responder, State}; +pub use handler::{ + AsyncResponder, Either, FromRequest, FutureResponse, Responder, State, +}; pub use httpmessage::HttpMessage; pub use httprequest::HttpRequest; pub use httpresponse::HttpResponse; From f82fa08d723005d5e79c71797d7aa6df2bb4705e Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 15 May 2018 16:41:46 -0700 Subject: [PATCH 38/47] various optimizations --- src/body.rs | 18 ++++++++ src/httpresponse.rs | 95 ++++++++++++++---------------------------- src/pipeline.rs | 28 +++++++++---- src/server/encoding.rs | 81 +++++++++++++++++++---------------- src/server/h1.rs | 34 ++++++++++++++- src/server/h1writer.rs | 20 +++++---- 6 files changed, 161 insertions(+), 115 deletions(-) diff --git a/src/body.rs b/src/body.rs index 063c93ac5..5ce0d1292 100644 --- a/src/body.rs +++ b/src/body.rs @@ -62,10 +62,28 @@ impl Body { } } + /// Is this binary empy. + #[inline] + pub fn is_empty(&self) -> bool { + match *self { + Body::Empty => true, + _ => false, + } + } + /// Create body from slice (copy) pub fn from_slice(s: &[u8]) -> Body { Body::Binary(Binary::Bytes(Bytes::from(s))) } + + /// Is this binary body. + #[inline] + pub(crate) fn binary(self) -> Binary { + match self { + Body::Binary(b) => b, + _ => panic!(), + } + } } impl PartialEq for Body { diff --git a/src/httpresponse.rs b/src/httpresponse.rs index 8097fc93b..a71f53fbb 100644 --- a/src/httpresponse.rs +++ b/src/httpresponse.rs @@ -466,10 +466,7 @@ impl HttpResponseBuilder { jar.add(cookie.into_owned()); self.cookies = Some(jar) } else { - self.cookies - .as_mut() - .unwrap() - .add(cookie.into_owned()); + self.cookies.as_mut().unwrap().add(cookie.into_owned()); } self } @@ -534,9 +531,7 @@ impl HttpResponseBuilder { if let Some(e) = self.err.take() { return Error::from(e).into(); } - let mut response = self.response - .take() - .expect("cannot reuse response builder"); + let mut response = self.response.take().expect("cannot reuse response builder"); if let Some(ref jar) = self.cookies { for cookie in jar.delta() { match HeaderValue::from_str(&cookie.to_string()) { @@ -558,9 +553,7 @@ impl HttpResponseBuilder { S: Stream + 'static, E: Into, { - self.body(Body::Streaming(Box::new( - stream.map_err(|e| e.into()), - ))) + self.body(Body::Streaming(Box::new(stream.map_err(|e| e.into())))) } /// Set a json body and generate `HttpResponse` @@ -607,7 +600,8 @@ impl HttpResponseBuilder { #[inline] #[cfg_attr(feature = "cargo-clippy", allow(borrowed_box))] fn parts<'a>( - parts: &'a mut Option>, err: &Option, + parts: &'a mut Option>, + err: &Option, ) -> Option<&'a mut Box> { if err.is_some() { return None; @@ -822,14 +816,15 @@ thread_local!(static POOL: Rc> = HttpResponsePool:: impl HttpResponsePool { pub fn pool() -> Rc> { - Rc::new(UnsafeCell::new(HttpResponsePool( - VecDeque::with_capacity(128), - ))) + Rc::new(UnsafeCell::new(HttpResponsePool(VecDeque::with_capacity( + 128, + )))) } #[inline] pub fn get_builder( - pool: &Rc>, status: StatusCode, + pool: &Rc>, + status: StatusCode, ) -> HttpResponseBuilder { let p = unsafe { &mut *pool.as_ref().get() }; if let Some(mut msg) = p.0.pop_front() { @@ -853,7 +848,9 @@ impl HttpResponsePool { #[inline] pub fn get_response( - pool: &Rc>, status: StatusCode, body: Body, + pool: &Rc>, + status: StatusCode, + body: Body, ) -> HttpResponse { let p = unsafe { &mut *pool.as_ref().get() }; if let Some(mut msg) = p.0.pop_front() { @@ -879,7 +876,8 @@ impl HttpResponsePool { #[inline(always)] #[cfg_attr(feature = "cargo-clippy", allow(boxed_local, inline_always))] fn release( - pool: &Rc>, mut inner: Box, + pool: &Rc>, + mut inner: Box, ) { let pool = unsafe { &mut *pool.as_ref().get() }; if pool.0.len() < 128 { @@ -975,9 +973,7 @@ mod tests { #[test] fn test_force_close() { - let resp = HttpResponse::build(StatusCode::OK) - .force_close() - .finish(); + let resp = HttpResponse::build(StatusCode::OK).force_close().finish(); assert!(!resp.keep_alive().unwrap()) } @@ -986,10 +982,7 @@ mod tests { let resp = HttpResponse::build(StatusCode::OK) .content_type("text/plain") .body(Body::Empty); - assert_eq!( - resp.headers().get(CONTENT_TYPE).unwrap(), - "text/plain" - ) + assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "text/plain") } #[test] @@ -1036,10 +1029,10 @@ mod tests { } impl Body { - pub(crate) fn binary(&self) -> Option<&Binary> { + pub(crate) fn bin_ref(&self) -> &Binary { match *self { - Body::Binary(ref bin) => Some(bin), - _ => None, + Body::Binary(ref bin) => bin, + _ => panic!(), } } } @@ -1055,7 +1048,7 @@ mod tests { HeaderValue::from_static("text/plain; charset=utf-8") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!(resp.body().binary().unwrap(), &Binary::from("test")); + assert_eq!(resp.body().bin_ref(), &Binary::from("test")); let resp: HttpResponse = "test".respond_to(&req).ok().unwrap(); assert_eq!(resp.status(), StatusCode::OK); @@ -1064,7 +1057,7 @@ mod tests { HeaderValue::from_static("text/plain; charset=utf-8") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!(resp.body().binary().unwrap(), &Binary::from("test")); + assert_eq!(resp.body().bin_ref(), &Binary::from("test")); let resp: HttpResponse = b"test".as_ref().into(); assert_eq!(resp.status(), StatusCode::OK); @@ -1073,10 +1066,7 @@ mod tests { HeaderValue::from_static("application/octet-stream") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!( - resp.body().binary().unwrap(), - &Binary::from(b"test".as_ref()) - ); + assert_eq!(resp.body().bin_ref(), &Binary::from(b"test".as_ref())); let resp: HttpResponse = b"test".as_ref().respond_to(&req).ok().unwrap(); assert_eq!(resp.status(), StatusCode::OK); @@ -1085,10 +1075,7 @@ mod tests { HeaderValue::from_static("application/octet-stream") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!( - resp.body().binary().unwrap(), - &Binary::from(b"test".as_ref()) - ); + assert_eq!(resp.body().bin_ref(), &Binary::from(b"test".as_ref())); let resp: HttpResponse = "test".to_owned().into(); assert_eq!(resp.status(), StatusCode::OK); @@ -1097,10 +1084,7 @@ mod tests { HeaderValue::from_static("text/plain; charset=utf-8") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!( - resp.body().binary().unwrap(), - &Binary::from("test".to_owned()) - ); + assert_eq!(resp.body().bin_ref(), &Binary::from("test".to_owned())); let resp: HttpResponse = "test".to_owned().respond_to(&req).ok().unwrap(); assert_eq!(resp.status(), StatusCode::OK); @@ -1109,10 +1093,7 @@ mod tests { HeaderValue::from_static("text/plain; charset=utf-8") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!( - resp.body().binary().unwrap(), - &Binary::from("test".to_owned()) - ); + assert_eq!(resp.body().bin_ref(), &Binary::from("test".to_owned())); let resp: HttpResponse = (&"test".to_owned()).into(); assert_eq!(resp.status(), StatusCode::OK); @@ -1121,10 +1102,7 @@ mod tests { HeaderValue::from_static("text/plain; charset=utf-8") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!( - resp.body().binary().unwrap(), - &Binary::from(&"test".to_owned()) - ); + assert_eq!(resp.body().bin_ref(), &Binary::from(&"test".to_owned())); let resp: HttpResponse = (&"test".to_owned()).respond_to(&req).ok().unwrap(); assert_eq!(resp.status(), StatusCode::OK); @@ -1133,10 +1111,7 @@ mod tests { HeaderValue::from_static("text/plain; charset=utf-8") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!( - resp.body().binary().unwrap(), - &Binary::from(&"test".to_owned()) - ); + assert_eq!(resp.body().bin_ref(), &Binary::from(&"test".to_owned())); let b = Bytes::from_static(b"test"); let resp: HttpResponse = b.into(); @@ -1147,7 +1122,7 @@ mod tests { ); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( - resp.body().binary().unwrap(), + resp.body().bin_ref(), &Binary::from(Bytes::from_static(b"test")) ); @@ -1160,7 +1135,7 @@ mod tests { ); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( - resp.body().binary().unwrap(), + resp.body().bin_ref(), &Binary::from(Bytes::from_static(b"test")) ); @@ -1172,10 +1147,7 @@ mod tests { HeaderValue::from_static("application/octet-stream") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!( - resp.body().binary().unwrap(), - &Binary::from(BytesMut::from("test")) - ); + assert_eq!(resp.body().bin_ref(), &Binary::from(BytesMut::from("test"))); let b = BytesMut::from("test"); let resp: HttpResponse = b.respond_to(&req).ok().unwrap(); @@ -1185,10 +1157,7 @@ mod tests { HeaderValue::from_static("application/octet-stream") ); assert_eq!(resp.status(), StatusCode::OK); - assert_eq!( - resp.body().binary().unwrap(), - &Binary::from(BytesMut::from("test")) - ); + assert_eq!(resp.body().bin_ref(), &Binary::from(BytesMut::from("test"))); } #[test] diff --git a/src/pipeline.rs b/src/pipeline.rs index e00c06178..4d5d405c0 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -29,7 +29,9 @@ pub(crate) trait PipelineHandler { fn encoding(&self) -> ContentEncoding; fn handle( - &mut self, req: HttpRequest, htype: HandlerType, + &mut self, + req: HttpRequest, + htype: HandlerType, ) -> AsyncResult; } @@ -120,8 +122,10 @@ impl PipelineInfo { impl> Pipeline { pub fn new( - req: HttpRequest, mws: Rc>>>, - handler: Rc>, htype: HandlerType, + req: HttpRequest, + mws: Rc>>>, + handler: Rc>, + htype: HandlerType, ) -> Pipeline { let mut info = PipelineInfo { mws, @@ -148,6 +152,7 @@ impl Pipeline<(), Inner<()>> { } impl Pipeline { + #[inline] fn is_done(&self) -> bool { match self.1 { PipelineState::None @@ -192,7 +197,9 @@ impl> HttpHandlerTask for Pipeline { match self.1 { PipelineState::None => return Ok(Async::Ready(true)), PipelineState::Error => { - return Err(io::Error::new(io::ErrorKind::Other, "Internal error").into()) + return Err( + io::Error::new(io::ErrorKind::Other, "Internal error").into() + ) } _ => (), } @@ -236,7 +243,9 @@ struct StartMiddlewares { impl> StartMiddlewares { fn init( - info: &mut PipelineInfo, hnd: Rc>, htype: HandlerType, + info: &mut PipelineInfo, + hnd: Rc>, + htype: HandlerType, ) -> PipelineState { // execute middlewares, we need this stage because middlewares could be // non-async and we can move to next state immediately @@ -313,7 +322,8 @@ struct WaitingResponse { impl WaitingResponse { #[inline] fn init( - info: &mut PipelineInfo, reply: AsyncResult, + info: &mut PipelineInfo, + reply: AsyncResult, ) -> PipelineState { match reply.into() { AsyncResultItem::Err(err) => RunMiddlewares::init(info, err.into()), @@ -344,6 +354,7 @@ struct RunMiddlewares { } impl RunMiddlewares { + #[inline] fn init(info: &mut PipelineInfo, mut resp: HttpResponse) -> PipelineState { if info.count == 0 { return ProcessResponse::init(resp); @@ -464,7 +475,9 @@ impl ProcessResponse { } fn poll_io( - mut self, io: &mut Writer, info: &mut PipelineInfo, + mut self, + io: &mut Writer, + info: &mut PipelineInfo, ) -> Result, PipelineState> { loop { if self.drain.is_none() && self.running != RunningState::Paused { @@ -676,6 +689,7 @@ struct FinishingMiddlewares { } impl FinishingMiddlewares { + #[inline] fn init(info: &mut PipelineInfo, resp: HttpResponse) -> PipelineState { if info.count == 0 { Completed::init(info) diff --git a/src/server/encoding.rs b/src/server/encoding.rs index ae69ae07f..662a8c4b7 100644 --- a/src/server/encoding.rs +++ b/src/server/encoding.rs @@ -12,8 +12,10 @@ use flate2::read::GzDecoder; use flate2::write::{DeflateDecoder, DeflateEncoder, GzEncoder}; #[cfg(feature = "flate2")] use flate2::Compression; -use http::header::{HeaderMap, HeaderValue, ACCEPT_ENCODING, CONTENT_ENCODING, - CONTENT_LENGTH, TRANSFER_ENCODING}; +use http::header::{ + HeaderMap, HeaderValue, ACCEPT_ENCODING, CONTENT_ENCODING, CONTENT_LENGTH, + TRANSFER_ENCODING, +}; use http::{HttpTryFrom, Method, Version}; use body::{Binary, Body}; @@ -378,16 +380,19 @@ impl ContentEncoder { } pub fn for_server( - buf: SharedBytes, req: &HttpInnerMessage, resp: &mut HttpResponse, + buf: SharedBytes, + req: &HttpInnerMessage, + resp: &mut HttpResponse, response_encoding: ContentEncoding, ) -> ContentEncoder { let version = resp.version().unwrap_or_else(|| req.version); let is_head = req.method == Method::HEAD; - let mut body = resp.replace_body(Body::Empty); - let has_body = match body { + let mut len = 0; + let has_body = match resp.body() { Body::Empty => false, Body::Binary(ref bin) => { - !(response_encoding == ContentEncoding::Auto && bin.len() < 96) + len = bin.len(); + !(response_encoding == ContentEncoding::Auto && len < 96) } _ => true, }; @@ -421,14 +426,14 @@ impl ContentEncoder { ContentEncoding::Identity }; - let mut transfer = match body { + let mut transfer = match resp.body() { Body::Empty => { if req.method != Method::HEAD { resp.headers_mut().remove(CONTENT_LENGTH); } TransferEncoding::length(0, buf) } - Body::Binary(ref mut bytes) => { + Body::Binary(_) => { if !(encoding == ContentEncoding::Identity || encoding == ContentEncoding::Auto) { @@ -448,19 +453,26 @@ impl ContentEncoder { ContentEncoding::Br => { ContentEncoder::Br(BrotliEncoder::new(transfer, 3)) } - ContentEncoding::Identity => ContentEncoder::Identity(transfer), - ContentEncoding::Auto => unreachable!(), + ContentEncoding::Identity | ContentEncoding::Auto => { + unreachable!() + } }; - // TODO return error! - let _ = enc.write(bytes.clone()); - let _ = enc.write_eof(); - *bytes = Binary::from(tmp.take()); + let bin = resp.replace_body(Body::Empty).binary(); + + // TODO return error! + let _ = enc.write(bin); + let _ = enc.write_eof(); + let body = tmp.take(); + len = body.len(); + encoding = ContentEncoding::Identity; + resp.replace_body(Binary::from(body)); } + if is_head { let mut b = BytesMut::new(); - let _ = write!(b, "{}", bytes.len()); + let _ = write!(b, "{}", len); resp.headers_mut().insert( CONTENT_LENGTH, HeaderValue::try_from(b.freeze()).unwrap(), @@ -485,11 +497,10 @@ impl ContentEncoder { } } }; - // + // check for head response if is_head { + resp.set_body(Body::Empty); transfer.kind = TransferEncodingKind::Length(0); - } else { - resp.replace_body(body); } match encoding { @@ -511,7 +522,9 @@ impl ContentEncoder { } fn streaming_encoding( - buf: SharedBytes, version: Version, resp: &mut HttpResponse, + buf: SharedBytes, + version: Version, + resp: &mut HttpResponse, ) -> TransferEncoding { match resp.chunked() { Some(true) => { @@ -590,7 +603,7 @@ impl ContentEncoder { #[cfg_attr(feature = "cargo-clippy", allow(inline_always))] #[inline(always)] - pub fn write_eof(&mut self) -> Result<(), io::Error> { + pub fn write_eof(&mut self) -> Result { let encoder = mem::replace( self, ContentEncoder::Identity(TransferEncoding::eof(SharedBytes::empty())), @@ -602,7 +615,7 @@ impl ContentEncoder { Ok(mut writer) => { writer.encode_eof(); *self = ContentEncoder::Identity(writer); - Ok(()) + Ok(true) } Err(err) => Err(err), }, @@ -611,7 +624,7 @@ impl ContentEncoder { Ok(mut writer) => { writer.encode_eof(); *self = ContentEncoder::Identity(writer); - Ok(()) + Ok(true) } Err(err) => Err(err), }, @@ -620,14 +633,14 @@ impl ContentEncoder { Ok(mut writer) => { writer.encode_eof(); *self = ContentEncoder::Identity(writer); - Ok(()) + Ok(true) } Err(err) => Err(err), }, ContentEncoder::Identity(mut writer) => { - writer.encode_eof(); + let res = writer.encode_eof(); *self = ContentEncoder::Identity(writer); - Ok(()) + Ok(res) } } } @@ -763,8 +776,7 @@ impl TransferEncoding { return Ok(*remaining == 0); } let len = cmp::min(*remaining, msg.len() as u64); - self.buffer - .extend(msg.take().split_to(len as usize).into()); + self.buffer.extend(msg.take().split_to(len as usize).into()); *remaining -= len as u64; Ok(*remaining == 0) @@ -777,14 +789,16 @@ impl TransferEncoding { /// Encode eof. Return `EOF` state of encoder #[inline] - pub fn encode_eof(&mut self) { + pub fn encode_eof(&mut self) -> bool { match self.kind { - TransferEncodingKind::Eof | TransferEncodingKind::Length(_) => (), + TransferEncodingKind::Eof => true, + TransferEncodingKind::Length(rem) => rem == 0, TransferEncodingKind::Chunked(ref mut eof) => { if !*eof { *eof = true; self.buffer.extend_from_slice(b"0\r\n\r\n"); } + true } } } @@ -848,10 +862,7 @@ impl AcceptEncoding { Err(_) => 0.0, }, }; - Some(AcceptEncoding { - encoding, - quality, - }) + Some(AcceptEncoding { encoding, quality }) } /// Parse a raw Accept-Encoding header value into an ordered list. @@ -879,9 +890,7 @@ mod tests { fn test_chunked_te() { let bytes = SharedBytes::default(); let mut enc = TransferEncoding::chunked(bytes.clone()); - assert!(!enc.encode(Binary::from(b"test".as_ref())) - .ok() - .unwrap()); + assert!(!enc.encode(Binary::from(b"test".as_ref())).ok().unwrap()); assert!(enc.encode(Binary::from(b"".as_ref())).ok().unwrap()); assert_eq!( bytes.get_mut().take().freeze(), diff --git a/src/server/h1.rs b/src/server/h1.rs index 933ce0a80..9418616cd 100644 --- a/src/server/h1.rs +++ b/src/server/h1.rs @@ -148,6 +148,7 @@ where } #[inline] + /// read data from stream pub fn poll_io(&mut self) { // read io from socket if !self.flags.intersects(Flags::ERROR) @@ -210,7 +211,7 @@ where if ready { item.flags.insert(EntryFlags::EOF | EntryFlags::FINISHED); } else { - item.flags.insert(EntryFlags::FINISHED); + item.flags.insert(EntryFlags::EOF); } } // no more IO for this iteration @@ -326,7 +327,36 @@ where // search handler for request for h in self.settings.handlers().iter_mut() { req = match h.handle(req) { - Ok(pipe) => { + Ok(mut pipe) => { + if self.tasks.is_empty() { + match pipe.poll_io(&mut self.stream) { + Ok(Async::Ready(ready)) => { + // override keep-alive state + if self.stream.keepalive() { + self.flags.insert(Flags::KEEPALIVE); + } else { + self.flags.remove(Flags::KEEPALIVE); + } + // prepare stream for next response + self.stream.reset(); + + if !ready { + let item = Entry { + pipe, + flags: EntryFlags::EOF, + }; + self.tasks.push_back(item); + } + continue 'outer; + } + Ok(Async::NotReady) => {} + Err(err) => { + error!("Unhandled error: {}", err); + self.flags.intersects(Flags::ERROR); + return; + } + } + } self.tasks.push_back(Entry { pipe, flags: EntryFlags::empty(), diff --git a/src/server/h1writer.rs b/src/server/h1writer.rs index c0fa0609f..ec5bfde18 100644 --- a/src/server/h1writer.rs +++ b/src/server/h1writer.rs @@ -42,7 +42,9 @@ pub(crate) struct H1Writer { impl H1Writer { pub fn new( - stream: T, buf: SharedBytes, settings: Rc>, + stream: T, + buf: SharedBytes, + settings: Rc>, ) -> H1Writer { H1Writer { flags: Flags::empty(), @@ -101,7 +103,9 @@ impl Writer for H1Writer { } fn start( - &mut self, req: &mut HttpInnerMessage, msg: &mut HttpResponse, + &mut self, + req: &mut HttpInnerMessage, + msg: &mut HttpResponse, encoding: ContentEncoding, ) -> io::Result { // prepare task @@ -138,7 +142,9 @@ impl Writer for H1Writer { let reason = msg.reason().as_bytes(); let mut is_bin = if let Body::Binary(ref bytes) = body { buffer.reserve( - 256 + msg.headers().len() * AVERAGE_HEADER_SIZE + bytes.len() + 256 + + msg.headers().len() * AVERAGE_HEADER_SIZE + + bytes.len() + reason.len(), ); true @@ -255,9 +261,7 @@ impl Writer for H1Writer { } fn write_eof(&mut self) -> io::Result { - self.encoder.write_eof()?; - - if !self.encoder.is_eof() { + if !self.encoder.write_eof()? { Err(io::Error::new( io::ErrorKind::Other, "Last payload item, but eof is not reached", @@ -276,7 +280,9 @@ impl Writer for H1Writer { unsafe { &mut *(self.buffer.as_ref() as *const _ as *mut _) }; let written = self.write_data(buf)?; let _ = self.buffer.split_to(written); - if self.buffer.len() > self.buffer_capacity { + if shutdown && !self.buffer.is_empty() + || (self.buffer.len() > self.buffer_capacity) + { return Ok(Async::NotReady); } } From 0d36b8f82602b1d9a5bddd0dcd47a6b4a0abb3c9 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 15 May 2018 19:07:43 -0700 Subject: [PATCH 39/47] fix 1.24 compatibility --- src/error.rs | 7 ++----- src/server/encoding.rs | 10 +++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/error.rs b/src/error.rs index fe9796727..1ec394e33 100644 --- a/src/error.rs +++ b/src/error.rs @@ -88,7 +88,7 @@ impl fmt::Debug for Error { } } -/// `HttpResponse` for `Error` +/// Convert `Error` to a `HttpResponse` instance impl From for HttpResponse { fn from(err: Error) -> Self { HttpResponse::from_error(err) @@ -317,10 +317,7 @@ pub enum HttpRangeError { /// Return `BadRequest` for `HttpRangeError` impl ResponseError for HttpRangeError { fn error_response(&self) -> HttpResponse { - HttpResponse::with_body( - StatusCode::BAD_REQUEST, - "Invalid Range header provided", - ) + HttpResponse::with_body(StatusCode::BAD_REQUEST, "Invalid Range header provided") } } diff --git a/src/server/encoding.rs b/src/server/encoding.rs index 662a8c4b7..07438b50b 100644 --- a/src/server/encoding.rs +++ b/src/server/encoding.rs @@ -389,8 +389,8 @@ impl ContentEncoder { let is_head = req.method == Method::HEAD; let mut len = 0; let has_body = match resp.body() { - Body::Empty => false, - Body::Binary(ref bin) => { + &Body::Empty => false, + &Body::Binary(ref bin) => { len = bin.len(); !(response_encoding == ContentEncoding::Auto && len < 96) } @@ -427,13 +427,13 @@ impl ContentEncoder { }; let mut transfer = match resp.body() { - Body::Empty => { + &Body::Empty => { if req.method != Method::HEAD { resp.headers_mut().remove(CONTENT_LENGTH); } TransferEncoding::length(0, buf) } - Body::Binary(_) => { + &Body::Binary(_) => { if !(encoding == ContentEncoding::Identity || encoding == ContentEncoding::Auto) { @@ -482,7 +482,7 @@ impl ContentEncoder { } TransferEncoding::eof(buf) } - Body::Streaming(_) | Body::Actor(_) => { + &Body::Streaming(_) | &Body::Actor(_) => { if resp.upgrade() { if version == Version::HTTP_2 { error!("Connection upgrade is forbidden for HTTP/2"); From 03e758cee4973208d0b957cc2dbe1bd3abcef756 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 15 May 2018 19:08:34 -0700 Subject: [PATCH 40/47] bump version --- CHANGES.md | 5 +++++ Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index a8c4e1e50..f450c70d6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,10 @@ # Changes +## 0.6.6 (2018-05-xx) + +.. + + ## 0.6.5 (2018-05-15) * Fix error handling during request decoding #222 diff --git a/Cargo.toml b/Cargo.toml index 595fc9a11..2ba3cc0a5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web" -version = "0.6.5" +version = "0.6.6" authors = ["Nikolay Kim "] description = "Actix web is a simple, pragmatic and extremely fast web framework for Rust." readme = "README.md" From 6e976153e79be73a25f3ab3c25666c4606062dbf Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 16 May 2018 15:20:47 +0200 Subject: [PATCH 41/47] Add support for listen_tls/listen_ssl --- src/server/srv.rs | 58 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/src/server/srv.rs b/src/server/srv.rs index df3978411..c76ec749e 100644 --- a/src/server/srv.rs +++ b/src/server/srv.rs @@ -25,6 +25,20 @@ use super::worker::{Conn, SocketInfo, StopWorker, StreamHandlerType, Worker}; use super::{IntoHttpHandler, IoStream, KeepAlive}; use super::{PauseServer, ResumeServer, StopServer}; +#[cfg(feature = "alpn")] +fn configure_alpn(builder: &mut SslAcceptorBuilder) -> io::Result<()> { + builder.set_alpn_protos(b"\x02h2\x08http/1.1")?; + builder.set_alpn_select_callback(|_, protos| { + const H2: &[u8] = b"\x02h2"; + if protos.windows(3).any(|window| window == H2) { + Ok(b"h2") + } else { + Err(AlpnError::NOACK) + } + }); + Ok(()) +} + /// An HTTP Server pub struct HttpServer where @@ -211,6 +225,40 @@ where self } + #[cfg(feature = "tls")] + /// Use listener for accepting incoming tls connection requests + /// + /// HttpServer does not change any configuration for TcpListener, + /// it needs to be configured before passing it to listen() method. + pub fn listen_tls(mut self, lst: net::TcpListener, acceptor: TlsAcceptor) -> Self { + let addr = lst.local_addr().unwrap(); + self.sockets.push(Socket { + addr, + lst, + tp: StreamHandlerType::Tls(acceptor.clone()), + }); + self + } + + #[cfg(feature = "alpn")] + /// Use listener for accepting incoming tls connection requests + /// + /// This method sets alpn protocols to "h2" and "http/1.1" + pub fn listen_ssl(mut self, lst: net::TcpListener, mut builder: SslAcceptorBuilder) -> io::Result { + // alpn support + if !self.no_http2 { + configure_alpn(&mut builder)?; + } + let acceptor = builder.build(); + let addr = lst.local_addr().unwrap(); + self.sockets.push(Socket { + addr, + lst, + tp: StreamHandlerType::Alpn(acceptor.clone()), + }); + Ok(self) + } + fn bind2(&mut self, addr: S) -> io::Result> { let mut err = None; let mut succ = false; @@ -277,15 +325,7 @@ where ) -> io::Result { // alpn support if !self.no_http2 { - builder.set_alpn_protos(b"\x02h2\x08http/1.1")?; - builder.set_alpn_select_callback(|_, protos| { - const H2: &[u8] = b"\x02h2"; - if protos.windows(3).any(|window| window == H2) { - Ok(b"h2") - } else { - Err(AlpnError::NOACK) - } - }); + configure_alpn(&mut builder)?; } let acceptor = builder.build(); From 7bb7d85c1d1c19002b5887e3e319a08c07565ee0 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 16 May 2018 16:17:27 +0200 Subject: [PATCH 42/47] Added support for returning addresses plus scheme from the server --- src/server/srv.rs | 10 ++++++++++ src/server/worker.rs | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/server/srv.rs b/src/server/srv.rs index c76ec749e..30b7b4e45 100644 --- a/src/server/srv.rs +++ b/src/server/srv.rs @@ -211,6 +211,16 @@ where self.sockets.iter().map(|s| s.addr).collect() } + /// Get addresses of bound sockets and the scheme for it. + /// + /// This is useful when the server is bound from different sources + /// with some sockets listening on http and some listening on https + /// and the user should be presented with an enumeration of which + /// socket requires which protocol. + pub fn addrs_with_scheme(&self) -> Vec<(net::SocketAddr, &str)> { + self.sockets.iter().map(|s| (s.addr, s.tp.scheme())).collect() + } + /// Use listener for accepting incoming connection requests /// /// HttpServer does not change any configuration for TcpListener, diff --git a/src/server/worker.rs b/src/server/worker.rs index 67f4645c0..a926a6c8f 100644 --- a/src/server/worker.rs +++ b/src/server/worker.rs @@ -239,4 +239,14 @@ impl StreamHandlerType { } } } + + pub(crate) fn scheme(&self) -> &'static str { + match *self { + StreamHandlerType::Normal => "http", + #[cfg(feature = "tls")] + StreamHandlerType::Tls(ref acceptor) => "https", + #[cfg(feature = "alpn")] + StreamHandlerType::Alpn(ref acceptor) => "https", + } + } } From b393ddf879b0d955b3bbea74291534680519850b Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 16 May 2018 11:00:29 -0700 Subject: [PATCH 43/47] fix panic during middleware execution #226 --- CHANGES.md | 4 +- src/pipeline.rs | 12 +-- src/route.rs | 47 ++++++----- src/scope.rs | 35 ++++---- tests/test_middleware.rs | 169 ++++++++++++++++++++++++++++++--------- 5 files changed, 185 insertions(+), 82 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f450c70d6..aa4d7c455 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,8 +1,8 @@ # Changes -## 0.6.6 (2018-05-xx) +## 0.6.6 (2018-05-16) -.. +* Panic during middleware execution #226 ## 0.6.5 (2018-05-15) diff --git a/src/pipeline.rs b/src/pipeline.rs index 4d5d405c0..82ec45a74 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -284,12 +284,12 @@ impl> StartMiddlewares { if let Some(resp) = resp { return Some(RunMiddlewares::init(info, resp)); } - if info.count == len { - let reply = unsafe { &mut *self.hnd.get() } - .handle(info.req().clone(), self.htype); - return Some(WaitingResponse::init(info, reply)); - } else { - loop { + loop { + if info.count == len { + let reply = unsafe { &mut *self.hnd.get() } + .handle(info.req().clone(), self.htype); + return Some(WaitingResponse::init(info, reply)); + } else { match info.mws[info.count as usize].start(info.req_mut()) { Ok(Started::Done) => info.count += 1, Ok(Started::Response(resp)) => { diff --git a/src/route.rs b/src/route.rs index 1322d1087..b109fd609 100644 --- a/src/route.rs +++ b/src/route.rs @@ -5,13 +5,17 @@ use std::rc::Rc; use futures::{Async, Future, Poll}; use error::Error; -use handler::{AsyncHandler, AsyncResult, AsyncResultItem, FromRequest, Handler, - Responder, RouteHandler, WrapHandler}; +use handler::{ + AsyncHandler, AsyncResult, AsyncResultItem, FromRequest, Handler, Responder, + RouteHandler, WrapHandler, +}; use http::StatusCode; use httprequest::HttpRequest; use httpresponse::HttpResponse; -use middleware::{Finished as MiddlewareFinished, Middleware, - Response as MiddlewareResponse, Started as MiddlewareStarted}; +use middleware::{ + Finished as MiddlewareFinished, Middleware, Response as MiddlewareResponse, + Started as MiddlewareStarted, +}; use pred::Predicate; use with::{ExtractorConfig, With, With2, With3, WithAsync}; @@ -51,7 +55,9 @@ impl Route { #[inline] pub(crate) fn compose( - &mut self, req: HttpRequest, mws: Rc>>>, + &mut self, + req: HttpRequest, + mws: Rc>>>, ) -> AsyncResult { AsyncResult::async(Box::new(Compose::new(req, mws, self.handler.clone()))) } @@ -242,7 +248,8 @@ impl Route { /// } /// ``` pub fn with2( - &mut self, handler: F, + &mut self, + handler: F, ) -> (ExtractorConfig, ExtractorConfig) where F: Fn(T1, T2) -> R + 'static, @@ -263,7 +270,8 @@ impl Route { #[doc(hidden)] /// Set handler function, use request extractor for all parameters. pub fn with3( - &mut self, handler: F, + &mut self, + handler: F, ) -> ( ExtractorConfig, ExtractorConfig, @@ -296,9 +304,7 @@ struct InnerHandler(Rc>>>); impl InnerHandler { #[inline] fn new>(h: H) -> Self { - InnerHandler(Rc::new(UnsafeCell::new(Box::new(WrapHandler::new( - h, - ))))) + InnerHandler(Rc::new(UnsafeCell::new(Box::new(WrapHandler::new(h))))) } #[inline] @@ -309,9 +315,7 @@ impl InnerHandler { R: Responder + 'static, E: Into + 'static, { - InnerHandler(Rc::new(UnsafeCell::new(Box::new(AsyncHandler::new( - h, - ))))) + InnerHandler(Rc::new(UnsafeCell::new(Box::new(AsyncHandler::new(h))))) } #[inline] @@ -364,7 +368,9 @@ impl ComposeState { impl Compose { fn new( - req: HttpRequest, mws: Rc>>>, handler: InnerHandler, + req: HttpRequest, + mws: Rc>>>, + handler: InnerHandler, ) -> Self { let mut info = ComposeInfo { count: 0, @@ -440,11 +446,11 @@ impl StartMiddlewares { if let Some(resp) = resp { return Some(RunMiddlewares::init(info, resp)); } - if info.count == len { - let reply = info.handler.handle(info.req.clone()); - return Some(WaitingResponse::init(info, reply)); - } else { - loop { + loop { + if info.count == len { + let reply = info.handler.handle(info.req.clone()); + return Some(WaitingResponse::init(info, reply)); + } else { match info.mws[info.count].start(&mut info.req) { Ok(MiddlewareStarted::Done) => info.count += 1, Ok(MiddlewareStarted::Response(resp)) => { @@ -479,7 +485,8 @@ struct WaitingResponse { impl WaitingResponse { #[inline] fn init( - info: &mut ComposeInfo, reply: AsyncResult, + info: &mut ComposeInfo, + reply: AsyncResult, ) -> ComposeState { match reply.into() { AsyncResultItem::Err(err) => RunMiddlewares::init(info, err.into()), diff --git a/src/scope.rs b/src/scope.rs index aecfd6bfa..00bcadad9 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -10,8 +10,10 @@ use handler::{AsyncResult, AsyncResultItem, FromRequest, Responder, RouteHandler use http::Method; use httprequest::HttpRequest; use httpresponse::HttpResponse; -use middleware::{Finished as MiddlewareFinished, Middleware, - Response as MiddlewareResponse, Started as MiddlewareStarted}; +use middleware::{ + Finished as MiddlewareFinished, Middleware, Response as MiddlewareResponse, + Started as MiddlewareStarted, +}; use pred::Predicate; use resource::ResourceHandler; use router::Resource; @@ -400,8 +402,7 @@ struct Wrapper { impl RouteHandler for Wrapper { fn handle(&mut self, req: HttpRequest) -> AsyncResult { - self.scope - .handle(req.change_state(Rc::clone(&self.state))) + self.scope.handle(req.change_state(Rc::clone(&self.state))) } } @@ -458,7 +459,8 @@ impl ComposeState { impl Compose { fn new( - req: HttpRequest, mws: Rc>>>, + req: HttpRequest, + mws: Rc>>>, resource: Rc>>, default: Option>>>, ) -> Self { @@ -543,17 +545,17 @@ impl StartMiddlewares { if let Some(resp) = resp { return Some(RunMiddlewares::init(info, resp)); } - if info.count == len { - let resource = unsafe { &mut *info.resource.get() }; - let reply = if let Some(ref default) = info.default { - let d = unsafe { &mut *default.as_ref().get() }; - resource.handle(info.req.clone(), Some(d)) + loop { + if info.count == len { + let resource = unsafe { &mut *info.resource.get() }; + let reply = if let Some(ref default) = info.default { + let d = unsafe { &mut *default.as_ref().get() }; + resource.handle(info.req.clone(), Some(d)) + } else { + resource.handle(info.req.clone(), None) + }; + return Some(WaitingResponse::init(info, reply)); } else { - resource.handle(info.req.clone(), None) - }; - return Some(WaitingResponse::init(info, reply)); - } else { - loop { match info.mws[info.count].start(&mut info.req) { Ok(MiddlewareStarted::Done) => info.count += 1, Ok(MiddlewareStarted::Response(resp)) => { @@ -583,7 +585,8 @@ struct WaitingResponse { impl WaitingResponse { #[inline] fn init( - info: &mut ComposeInfo, reply: AsyncResult, + info: &mut ComposeInfo, + reply: AsyncResult, ) -> ComposeState { match reply.into() { AsyncResultItem::Ok(resp) => RunMiddlewares::init(info, resp), diff --git a/tests/test_middleware.rs b/tests/test_middleware.rs index 99151afd7..2c9160b61 100644 --- a/tests/test_middleware.rs +++ b/tests/test_middleware.rs @@ -21,28 +21,24 @@ struct MiddlewareTest { impl middleware::Middleware for MiddlewareTest { fn start(&self, _: &mut HttpRequest) -> Result { - self.start.store( - self.start.load(Ordering::Relaxed) + 1, - Ordering::Relaxed, - ); + self.start + .store(self.start.load(Ordering::Relaxed) + 1, Ordering::Relaxed); Ok(middleware::Started::Done) } fn response( - &self, _: &mut HttpRequest, resp: HttpResponse, + &self, + _: &mut HttpRequest, + resp: HttpResponse, ) -> Result { - self.response.store( - self.response.load(Ordering::Relaxed) + 1, - Ordering::Relaxed, - ); + self.response + .store(self.response.load(Ordering::Relaxed) + 1, Ordering::Relaxed); Ok(middleware::Response::Done(resp)) } fn finish(&self, _: &mut HttpRequest, _: &HttpResponse) -> middleware::Finished { - self.finish.store( - self.finish.load(Ordering::Relaxed) + 1, - Ordering::Relaxed, - ); + self.finish + .store(self.finish.load(Ordering::Relaxed) + 1, Ordering::Relaxed); middleware::Finished::Done } } @@ -187,10 +183,7 @@ fn test_scope_middleware() { }) }); - let request = srv.get() - .uri(srv.url("/scope/test")) - .finish() - .unwrap(); + let request = srv.get().uri(srv.url("/scope/test")).finish().unwrap(); let response = srv.execute(request.send()).unwrap(); assert!(response.status().is_success()); @@ -226,10 +219,7 @@ fn test_scope_middleware_multiple() { }) }); - let request = srv.get() - .uri(srv.url("/scope/test")) - .finish() - .unwrap(); + let request = srv.get().uri(srv.url("/scope/test")).finish().unwrap(); let response = srv.execute(request.send()).unwrap(); assert!(response.status().is_success()); @@ -337,10 +327,7 @@ fn test_scope_middleware_async_handler() { }) }); - let request = srv.get() - .uri(srv.url("/scope/test")) - .finish() - .unwrap(); + let request = srv.get().uri(srv.url("/scope/test")).finish().unwrap(); let response = srv.execute(request.send()).unwrap(); assert!(response.status().is_success()); @@ -402,10 +389,7 @@ fn test_scope_middleware_async_error() { }) }); - let request = srv.get() - .uri(srv.url("/scope/test")) - .finish() - .unwrap(); + let request = srv.get().uri(srv.url("/scope/test")).finish().unwrap(); let response = srv.execute(request.send()).unwrap(); assert_eq!(response.status(), http::StatusCode::BAD_REQUEST); @@ -466,7 +450,9 @@ impl middleware::Middleware for MiddlewareAsyncTest { } fn response( - &self, _: &mut HttpRequest, resp: HttpResponse, + &self, + _: &mut HttpRequest, + resp: HttpResponse, ) -> Result { let to = Timeout::new(Duration::from_millis(10), &Arbiter::handle()).unwrap(); @@ -555,6 +541,42 @@ fn test_async_middleware_multiple() { assert_eq!(num3.load(Ordering::Relaxed), 2); } +#[test] +fn test_async_sync_middleware_multiple() { + let num1 = Arc::new(AtomicUsize::new(0)); + let num2 = Arc::new(AtomicUsize::new(0)); + let num3 = Arc::new(AtomicUsize::new(0)); + + let act_num1 = Arc::clone(&num1); + let act_num2 = Arc::clone(&num2); + let act_num3 = Arc::clone(&num3); + + let mut srv = test::TestServer::with_factory(move || { + App::new() + .middleware(MiddlewareAsyncTest { + start: Arc::clone(&act_num1), + response: Arc::clone(&act_num2), + finish: Arc::clone(&act_num3), + }) + .middleware(MiddlewareTest { + start: Arc::clone(&act_num1), + response: Arc::clone(&act_num2), + finish: Arc::clone(&act_num3), + }) + .resource("/test", |r| r.f(|_| HttpResponse::Ok())) + }); + + let request = srv.get().uri(srv.url("/test")).finish().unwrap(); + let response = srv.execute(request.send()).unwrap(); + assert!(response.status().is_success()); + + assert_eq!(num1.load(Ordering::Relaxed), 2); + assert_eq!(num2.load(Ordering::Relaxed), 2); + + thread::sleep(Duration::from_millis(50)); + assert_eq!(num3.load(Ordering::Relaxed), 2); +} + #[test] fn test_async_scope_middleware() { let num1 = Arc::new(AtomicUsize::new(0)); @@ -577,10 +599,7 @@ fn test_async_scope_middleware() { }) }); - let request = srv.get() - .uri(srv.url("/scope/test")) - .finish() - .unwrap(); + let request = srv.get().uri(srv.url("/scope/test")).finish().unwrap(); let response = srv.execute(request.send()).unwrap(); assert!(response.status().is_success()); @@ -618,10 +637,45 @@ fn test_async_scope_middleware_multiple() { }) }); - let request = srv.get() - .uri(srv.url("/scope/test")) - .finish() - .unwrap(); + let request = srv.get().uri(srv.url("/scope/test")).finish().unwrap(); + let response = srv.execute(request.send()).unwrap(); + assert!(response.status().is_success()); + + assert_eq!(num1.load(Ordering::Relaxed), 2); + assert_eq!(num2.load(Ordering::Relaxed), 2); + + thread::sleep(Duration::from_millis(20)); + assert_eq!(num3.load(Ordering::Relaxed), 2); +} + +#[test] +fn test_async_async_scope_middleware_multiple() { + let num1 = Arc::new(AtomicUsize::new(0)); + let num2 = Arc::new(AtomicUsize::new(0)); + let num3 = Arc::new(AtomicUsize::new(0)); + + let act_num1 = Arc::clone(&num1); + let act_num2 = Arc::clone(&num2); + let act_num3 = Arc::clone(&num3); + + let mut srv = test::TestServer::with_factory(move || { + App::new().scope("/scope", |scope| { + scope + .middleware(MiddlewareAsyncTest { + start: Arc::clone(&act_num1), + response: Arc::clone(&act_num2), + finish: Arc::clone(&act_num3), + }) + .middleware(MiddlewareTest { + start: Arc::clone(&act_num1), + response: Arc::clone(&act_num2), + finish: Arc::clone(&act_num3), + }) + .resource("/test", |r| r.f(|_| HttpResponse::Ok())) + }) + }); + + let request = srv.get().uri(srv.url("/scope/test")).finish().unwrap(); let response = srv.execute(request.send()).unwrap(); assert!(response.status().is_success()); @@ -703,3 +757,42 @@ fn test_async_resource_middleware_multiple() { thread::sleep(Duration::from_millis(40)); assert_eq!(num3.load(Ordering::Relaxed), 2); } + +#[test] +fn test_async_sync_resource_middleware_multiple() { + let num1 = Arc::new(AtomicUsize::new(0)); + let num2 = Arc::new(AtomicUsize::new(0)); + let num3 = Arc::new(AtomicUsize::new(0)); + + let act_num1 = Arc::clone(&num1); + let act_num2 = Arc::clone(&num2); + let act_num3 = Arc::clone(&num3); + + let mut srv = test::TestServer::with_factory(move || { + let mw1 = MiddlewareAsyncTest { + start: Arc::clone(&act_num1), + response: Arc::clone(&act_num2), + finish: Arc::clone(&act_num3), + }; + let mw2 = MiddlewareTest { + start: Arc::clone(&act_num1), + response: Arc::clone(&act_num2), + finish: Arc::clone(&act_num3), + }; + App::new().resource("/test", move |r| { + r.middleware(mw1); + r.middleware(mw2); + r.h(|_| HttpResponse::Ok()); + }) + }); + + let request = srv.get().uri(srv.url("/test")).finish().unwrap(); + let response = srv.execute(request.send()).unwrap(); + assert!(response.status().is_success()); + + assert_eq!(num1.load(Ordering::Relaxed), 2); + assert_eq!(num2.load(Ordering::Relaxed), 2); + + thread::sleep(Duration::from_millis(40)); + assert_eq!(num3.load(Ordering::Relaxed), 2); +} From fe2b50a9efaaa50f7024fd304c9de0c70ab8b292 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 16 May 2018 11:02:50 -0700 Subject: [PATCH 44/47] update changelog --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index aa4d7c455..39467e016 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,8 @@ * Panic during middleware execution #226 +* Add support for listen_tls/listen_ssl #224 + ## 0.6.5 (2018-05-15) From b4252f8fd18f8ae30bba066f771a019fc12260ab Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 16 May 2018 21:02:51 -0700 Subject: [PATCH 45/47] implement extractor for Session --- CHANGES.md | 4 ++- src/middleware/session.rs | 58 ++++++++++++++++++++++++++++++--------- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 39467e016..07bd7b32a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,11 +1,13 @@ # Changes -## 0.6.6 (2018-05-16) +## 0.6.6 (2018-05-17) * Panic during middleware execution #226 * Add support for listen_tls/listen_ssl #224 +* Implement extractor for `Session` + ## 0.6.5 (2018-05-15) diff --git a/src/middleware/session.rs b/src/middleware/session.rs index b926842f5..4565cc119 100644 --- a/src/middleware/session.rs +++ b/src/middleware/session.rs @@ -80,6 +80,7 @@ use serde_json::error::Error as JsonError; use time::Duration; use error::{Error, ResponseError, Result}; +use handler::FromRequest; use httprequest::HttpRequest; use httpresponse::HttpResponse; use middleware::{Middleware, Response, Started}; @@ -190,6 +191,16 @@ impl Session { } } +impl FromRequest for Session { + type Config = (); + type Result = Session; + + #[inline] + fn from_request(req: &HttpRequest, _: &Self::Config) -> Self::Result { + req.session() + } +} + struct SessionImplCell(RefCell>); #[doc(hidden)] @@ -226,22 +237,21 @@ impl> Middleware for SessionStorage { fn start(&self, req: &mut HttpRequest) -> Result { let mut req = req.clone(); - let fut = self.0 - .from_request(&mut req) - .then(move |res| match res { - Ok(sess) => { - req.extensions_mut().insert(Arc::new(SessionImplCell( - RefCell::new(Box::new(sess)), - ))); - FutOk(None) - } - Err(err) => FutErr(err), - }); + let fut = self.0.from_request(&mut req).then(move |res| match res { + Ok(sess) => { + req.extensions_mut() + .insert(Arc::new(SessionImplCell(RefCell::new(Box::new(sess))))); + FutOk(None) + } + Err(err) => FutErr(err), + }); Ok(Started::Future(Box::new(fut))) } fn response( - &self, req: &mut HttpRequest, resp: HttpResponse, + &self, + req: &mut HttpRequest, + resp: HttpResponse, ) -> Result { if let Some(s_box) = req.extensions_mut().remove::>() { s_box.0.borrow_mut().write(resp) @@ -357,7 +367,9 @@ impl CookieSessionInner { } fn set_cookie( - &self, resp: &mut HttpResponse, state: &HashMap, + &self, + resp: &mut HttpResponse, + state: &HashMap, ) -> Result<()> { let value = serde_json::to_string(&state).map_err(CookieSessionError::Serialize)?; @@ -551,4 +563,24 @@ mod tests { let response = srv.execute(request.send()).unwrap(); assert!(response.cookie("actix-session").is_some()); } + + #[test] + fn cookie_session_extractor() { + let mut srv = test::TestServer::with_factory(|| { + App::new() + .middleware(SessionStorage::new( + CookieSessionBackend::signed(&[0; 32]).secure(false), + )) + .resource("/", |r| { + r.with(|ses: Session| { + let _ = ses.set("counter", 100); + "test" + }) + }) + }); + + let request = srv.get().uri(srv.url("/")).finish().unwrap(); + let response = srv.execute(request.send()).unwrap(); + assert!(response.cookie("actix-session").is_some()); + } } From 8de1f60347638bb29080a16042758ac883a6bad4 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 16 May 2018 21:05:59 -0700 Subject: [PATCH 46/47] add session extractor doc api --- src/middleware/session.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/middleware/session.rs b/src/middleware/session.rs index 4565cc119..6225bc34f 100644 --- a/src/middleware/session.rs +++ b/src/middleware/session.rs @@ -191,6 +191,24 @@ impl Session { } } +/// Extractor implementation for Session type. +/// +/// ```rust +/// # use actix_web::*; +/// use actix_web::middleware::session::Session; +/// +/// fn index(session: Session) -> Result<&'static str> { +/// // access session data +/// if let Some(count) = session.get::("counter")? { +/// session.set("counter", count+1)?; +/// } else { +/// session.set("counter", 1)?; +/// } +/// +/// Ok("Welcome!") +/// } +/// # fn main() {} +/// ``` impl FromRequest for Session { type Config = (); type Result = Session; From f3ece74406b4cbccac79bc5be2dadae8ac3d7b7f Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 17 May 2018 10:58:08 -0700 Subject: [PATCH 47/47] better error handling --- src/server/channel.rs | 23 ++++++++++++----------- src/server/h1.rs | 3 +-- src/server/h1decoder.rs | 39 +++++++++++++++++++++------------------ 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/src/server/channel.rs b/src/server/channel.rs index e5d226eda..9c30fe01c 100644 --- a/src/server/channel.rs +++ b/src/server/channel.rs @@ -38,7 +38,9 @@ where H: HttpHandler + 'static, { pub(crate) fn new( - settings: Rc>, mut io: T, peer: Option, + settings: Rc>, + mut io: T, + peer: Option, http2: bool, ) -> HttpChannel { settings.add_channel(); @@ -61,7 +63,7 @@ where settings, peer, io, - BytesMut::with_capacity(4096), + BytesMut::with_capacity(8192), )), } } @@ -93,12 +95,12 @@ where let el = self as *mut _; self.node = Some(Node::new(el)); let _ = match self.proto { - Some(HttpProtocol::H1(ref mut h1)) => self.node - .as_ref() - .map(|n| h1.settings().head().insert(n)), - Some(HttpProtocol::H2(ref mut h2)) => self.node - .as_ref() - .map(|n| h2.settings().head().insert(n)), + Some(HttpProtocol::H1(ref mut h1)) => { + self.node.as_ref().map(|n| h1.settings().head().insert(n)) + } + Some(HttpProtocol::H2(ref mut h2)) => { + self.node.as_ref().map(|n| h2.settings().head().insert(n)) + } Some(HttpProtocol::Unknown(ref mut settings, _, _, _)) => { self.node.as_ref().map(|n| settings.head().insert(n)) } @@ -168,9 +170,8 @@ where if let Some(HttpProtocol::Unknown(settings, addr, io, buf)) = self.proto.take() { match kind { ProtocolKind::Http1 => { - self.proto = Some(HttpProtocol::H1(h1::Http1::new( - settings, io, addr, buf, - ))); + self.proto = + Some(HttpProtocol::H1(h1::Http1::new(settings, io, addr, buf))); return self.poll(); } ProtocolKind::Http2 => { diff --git a/src/server/h1.rs b/src/server/h1.rs index 9418616cd..46ec3473e 100644 --- a/src/server/h1.rs +++ b/src/server/h1.rs @@ -162,7 +162,6 @@ where entry.pipe.disconnected() } // kill keepalive - self.flags.remove(Flags::KEEPALIVE); self.keepalive_timer.take(); // on parse error, stop reading stream but tasks need to be @@ -352,7 +351,7 @@ where Ok(Async::NotReady) => {} Err(err) => { error!("Unhandled error: {}", err); - self.flags.intersects(Flags::ERROR); + self.flags.insert(Flags::ERROR); return; } } diff --git a/src/server/h1decoder.rs b/src/server/h1decoder.rs index 375923d06..0d83bfbdd 100644 --- a/src/server/h1decoder.rs +++ b/src/server/h1decoder.rs @@ -46,7 +46,9 @@ impl H1Decoder { } pub fn decode( - &mut self, src: &mut BytesMut, settings: &WorkerSettings, + &mut self, + src: &mut BytesMut, + settings: &WorkerSettings, ) -> Result, DecoderError> { // read payload if self.decoder.is_some() { @@ -64,18 +66,11 @@ impl H1Decoder { .map_err(DecoderError::Error)? { Async::Ready((msg, decoder)) => { - if let Some(decoder) = decoder { - self.decoder = Some(decoder); - Ok(Some(Message::Message { - msg, - payload: true, - })) - } else { - Ok(Some(Message::Message { - msg, - payload: false, - })) - } + self.decoder = decoder; + Ok(Some(Message::Message { + msg, + payload: self.decoder.is_some(), + })) } Async::NotReady => { if src.len() >= MAX_BUFFER_SIZE { @@ -89,7 +84,9 @@ impl H1Decoder { } fn parse_message( - &self, buf: &mut BytesMut, settings: &WorkerSettings, + &self, + buf: &mut BytesMut, + settings: &WorkerSettings, ) -> Poll<(SharedHttpInnerMessage, Option), ParseError> { // Parse http message let mut has_upgrade = false; @@ -148,7 +145,7 @@ impl H1Decoder { header::CONTENT_LENGTH => { if let Ok(s) = value.to_str() { if let Ok(len) = s.parse::() { - content_length = Some(len) + content_length = Some(len); } else { debug!("illegal Content-Length: {:?}", len); return Err(ParseError::Header); @@ -351,7 +348,10 @@ macro_rules! byte ( impl ChunkedState { fn step( - &self, body: &mut BytesMut, size: &mut u64, buf: &mut Option, + &self, + body: &mut BytesMut, + size: &mut u64, + buf: &mut Option, ) -> Poll { use self::ChunkedState::*; match *self { @@ -414,7 +414,8 @@ impl ChunkedState { } } fn read_size_lf( - rdr: &mut BytesMut, size: &mut u64, + rdr: &mut BytesMut, + size: &mut u64, ) -> Poll { match byte!(rdr) { b'\n' if *size > 0 => Ok(Async::Ready(ChunkedState::Body)), @@ -427,7 +428,9 @@ impl ChunkedState { } fn read_body( - rdr: &mut BytesMut, rem: &mut u64, buf: &mut Option, + rdr: &mut BytesMut, + rem: &mut u64, + buf: &mut Option, ) -> Poll { trace!("Chunked read, remaining={:?}", rem);