From 2479b14aba31f535ff014db0370f0be51c82d28a Mon Sep 17 00:00:00 2001 From: Aleksey Ivanov Date: Wed, 23 May 2018 19:07:42 +0300 Subject: [PATCH 1/6] Fix TestServer::post --- src/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test.rs b/src/test.rs index 135135f7e..5e99652b2 100644 --- a/src/test.rs +++ b/src/test.rs @@ -217,7 +217,7 @@ impl TestServer { /// Create `POST` request pub fn post(&self) -> ClientRequestBuilder { - ClientRequest::get(self.url("/").as_str()) + ClientRequest::post(self.url("/").as_str()) } /// Create `HEAD` request From eb5dbd43aee2cbb161e2e5f65e4a811f6d796254 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 23 May 2018 10:37:17 -0700 Subject: [PATCH 2/6] update changelog --- CHANGES.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index afa86b7a0..4bcc5a725 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,12 @@ # Changes +## [0.6.10] - 2018-05-xx + +### Fixed + +* `TestServer::post()` actually sends `GET` request #240 + + ## 0.6.9 (2018-05-22) * Drop connection if request's payload is not fully consumed #236 From 72757887c9ca53f342955955b5db6fead453896e Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 23 May 2018 11:20:12 -0700 Subject: [PATCH 3/6] update doc links --- README.md | 14 +++++++------- src/lib.rs | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 51a3ae35c..ae0bc645e 100644 --- a/README.md +++ b/README.md @@ -2,12 +2,12 @@ Actix web is a simple, pragmatic and extremely fast web framework for Rust. -* Supported *HTTP/1.x* and [*HTTP/2.0*](https://actix.rs/book/actix-web/sec-12-http2.html) protocols +* Supported *HTTP/1.x* and [*HTTP/2.0*](https://actix.rs/docs/http2/) protocols * Streaming and pipelining * Keep-alive and slow requests handling -* Client/server [WebSockets](https://actix.rs/book/actix-web/sec-11-websockets.html) support +* Client/server [WebSockets](https://actix.rs/docs/websockets/) support * Transparent content compression/decompression (br, gzip, deflate) -* Configurable [request routing](https://actix.rs/book/actix-web/sec-6-url-dispatch.html) +* Configurable [request routing](https://actix.rs/docs/url-dispatch/) * Graceful server shutdown * Multipart streams * Static assets @@ -18,12 +18,12 @@ 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)) -* Includes an asynchronous [HTTP client](https://github.com/actix/actix-web/blob/master/src/client/mod.rs) +* Includes an asynchronous [HTTP client](https://actix.rs/actix-web/actix_web/client/index.html) * Built on top of [Actix actor framework](https://github.com/actix/actix) ## Documentation & community resources -* [User Guide](https://actix.rs/book/actix-web/) +* [User Guide](https://actix.rs/docs/) * [API Documentation (Development)](https://actix.rs/actix-web/actix_web/) * [API Documentation (Releases)](https://docs.rs/actix-web/) * [Chat on gitter](https://gitter.im/actix/actix) @@ -34,9 +34,9 @@ Actix web is a simple, pragmatic and extremely fast web framework for Rust. ```rust extern crate actix_web; -use actix_web::{http, server, App, Path}; +use actix_web::{http, server, App, Path, Responder}; -fn index(info: Path<(u32, String)>) -> String { +fn index(info: Path<(u32, String)>) -> impl Responder { format!("Hello {}! id:{}", info.1, info.0) } diff --git a/src/lib.rs b/src/lib.rs index 8ef1e2df8..9912d4ada 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,7 +25,7 @@ //! Besides the API documentation (which you are currently looking //! at!), several other resources are available: //! -//! * [User Guide](https://actix.rs/book/actix-web/) +//! * [User Guide](https://actix.rs/docs/) //! * [Chat on gitter](https://gitter.im/actix/actix) //! * [GitHub repository](https://github.com/actix/actix-web) //! * [Cargo package](https://crates.io/crates/actix-web) From 68eb2f26c9d0b3f4c07344697adb5889a1fdd334 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 23 May 2018 13:21:29 -0700 Subject: [PATCH 4/6] Allow to use path without traling slashes for scope registration #241 --- CHANGES.md | 4 ++ src/application.rs | 16 +------ src/fs.rs | 10 ++-- src/param.rs | 5 +- src/router.rs | 10 +++- src/scope.rs | 102 +++++++++++++++++++++++++++++++---------- src/server/encoding.rs | 2 +- src/server/h1.rs | 2 +- src/server/h1writer.rs | 2 +- src/server/mod.rs | 2 +- tests/test_handlers.rs | 11 ++--- 11 files changed, 107 insertions(+), 59 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 4bcc5a725..0bf976833 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,10 @@ ## [0.6.10] - 2018-05-xx +### Added + +* Allow to use path without traling slashes for scope registration #241 + ### Fixed * `TestServer::post()` actually sends `GET` request #240 diff --git a/src/application.rs b/src/application.rs index 481430aab..94fb70fb2 100644 --- a/src/application.rs +++ b/src/application.rs @@ -107,16 +107,12 @@ impl HttpApplication { } } - let prefix_len = inner.prefix + prefix_len - 1; + 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().set("tail", "/"); - } else { - req.match_info_mut().set("tail", path); - } + req.match_info_mut().set("tail", path); return HandlerType::Handler(idx); } } @@ -369,14 +365,6 @@ where { { 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('/') { - path.insert(0, '/') - } - if !path.ends_with('/') { - path.push('/'); - } let parts = self.parts.as_mut().expect("Use after finish"); let filters = scope.take_filters(); diff --git a/src/fs.rs b/src/fs.rs index 779b419a4..cebad4922 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -20,7 +20,7 @@ use mime_guess::{get_mime_type, guess_mime_type}; use error::Error; use handler::{AsyncResult, Handler, Responder, RouteHandler, WrapHandler}; use header; -use http::{HttpRange, Method, StatusCode, ContentEncoding}; +use http::{ContentEncoding, HttpRange, Method, StatusCode}; use httpmessage::HttpMessage; use httprequest::HttpRequest; use httpresponse::HttpResponse; @@ -304,11 +304,11 @@ impl Responder for NamedFile { resp.header( header::CONTENT_RANGE, format!( - "bytes {}-{}/{}", - offset, - offset + length - 1, + "bytes {}-{}/{}", + offset, + offset + length - 1, self.md.len() - ) + ), ); } else { resp.header(header::CONTENT_RANGE, format!("bytes */{}", length)); diff --git a/src/param.rs b/src/param.rs index 119ad59c3..fd07acf4d 100644 --- a/src/param.rs +++ b/src/param.rs @@ -58,12 +58,11 @@ impl<'a> Params<'a> { self.0.push((name, value)); } - pub(crate) fn remove(&mut self, name: &str) - { + pub(crate) fn remove(&mut self, name: &str) { for idx in (0..self.0.len()).rev() { if self.0[idx].0 == name { self.0.remove(idx); - return + return; } } } diff --git a/src/router.rs b/src/router.rs index 44fde0a40..602326f8a 100644 --- a/src/router.rs +++ b/src/router.rs @@ -311,8 +311,16 @@ impl Resource { None } } - PatternType::Prefix(ref s) => if path.starts_with(s) { + PatternType::Prefix(ref s) => if path == s { Some(s.len()) + } else if path.starts_with(s) && (s.ends_with('/') || + path.split_at(s.len()).1.starts_with('/')) + { + if s.ends_with('/') { + Some(s.len()-1) + } else { + Some(s.len()) + } } else { None }, diff --git a/src/scope.rs b/src/scope.rs index edbf81df0..839c4746c 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -137,14 +137,6 @@ impl Scope { }; let mut scope = f(scope); - let mut path = path.trim().trim_right_matches('/').to_owned(); - if !path.is_empty() && !path.starts_with('/') { - path.insert(0, '/') - } - if !path.ends_with('/') { - path.push('/'); - } - let state = Rc::new(state); let filters: Vec>> = vec![Box::new(FiltersWrapper { state: Rc::clone(&state), @@ -191,14 +183,6 @@ impl Scope { }; let mut scope = f(scope); - let mut path = path.trim().trim_right_matches('/').to_owned(); - if !path.is_empty() && !path.starts_with('/') { - path.insert(0, '/') - } - if !path.ends_with('/') { - path.push('/'); - } - let filters = scope.take_filters(); self.nested.push(( Resource::prefix("", &path), @@ -253,7 +237,12 @@ impl Scope { let mut handler = ResourceHandler::default(); handler.method(method).with(f); - let pattern = Resource::new(handler.get_name(), path); + let pattern = Resource::with_prefix( + handler.get_name(), + path, + if path.is_empty() { "" } else { "/" }, + false, + ); Rc::get_mut(&mut self.resources) .expect("Can not use after configuration") .push((pattern, Rc::new(UnsafeCell::new(handler)))); @@ -293,7 +282,12 @@ impl Scope { let mut handler = ResourceHandler::default(); f(&mut handler); - let pattern = Resource::new(handler.get_name(), path); + let pattern = Resource::with_prefix( + handler.get_name(), + path, + if path.is_empty() { "" } else { "/" }, + false, + ); Rc::get_mut(&mut self.resources) .expect("Can not use after configuration") .push((pattern, Rc::new(UnsafeCell::new(handler)))); @@ -329,7 +323,6 @@ impl Scope { impl RouteHandler for Scope { fn handle(&mut self, mut req: HttpRequest) -> AsyncResult { let path = unsafe { &*(&req.match_info()["tail"] as *const _) }; - let path = if path == "" { "/" } else { path }; // recognize resources for &(ref pattern, ref resource) in self.resources.iter() { @@ -364,16 +357,12 @@ impl RouteHandler for Scope { continue 'outer; } } - let prefix_len = len + prefix_len - 1; + let prefix_len = len + 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().set("tail", "/"); - } else { - req.match_info_mut().set("tail", path); - } + req.match_info_mut().set("tail", path); let hnd: &mut RouteHandler<_> = unsafe { (&mut *(handler.get())).as_mut() }; @@ -784,6 +773,25 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::OK); } + #[test] + fn test_scope_root() { + let mut app = App::new() + .scope("/app", |scope| { + scope + .resource("", |r| r.f(|_| HttpResponse::Ok())) + .resource("/", |r| r.f(|_| HttpResponse::Created())) + }) + .finish(); + + let req = TestRequest::with_uri("/app").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + + let req = TestRequest::with_uri("/app/").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::CREATED); + } + #[test] fn test_scope_route() { let mut app = App::new() @@ -885,6 +893,29 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::CREATED); } + #[test] + fn test_scope_with_state_root() { + struct State; + + let mut app = App::new() + .scope("/app", |scope| { + scope.with_state("/t1", State, |scope| { + scope + .resource("", |r| r.f(|_| HttpResponse::Ok())) + .resource("/", |r| r.f(|_| HttpResponse::Created())) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/t1").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + + let req = TestRequest::with_uri("/app/t1/").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::CREATED); + } + #[test] fn test_scope_with_state_filter() { struct State; @@ -927,6 +958,27 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::CREATED); } + #[test] + fn test_nested_scope_root() { + let mut app = App::new() + .scope("/app", |scope| { + scope.nested("/t1", |scope| { + scope + .resource("", |r| r.f(|_| HttpResponse::Ok())) + .resource("/", |r| r.f(|_| HttpResponse::Created())) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/t1").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + + let req = TestRequest::with_uri("/app/t1/").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::CREATED); + } + #[test] fn test_nested_scope_filter() { let mut app = App::new() diff --git a/src/server/encoding.rs b/src/server/encoding.rs index 34c589fc8..4379a4ba2 100644 --- a/src/server/encoding.rs +++ b/src/server/encoding.rs @@ -506,7 +506,7 @@ impl ContentEncoder { TransferEncoding::eof(buf) } else { if !(encoding == ContentEncoding::Identity - || encoding == ContentEncoding::Auto) + || encoding == ContentEncoding::Auto) { resp.headers_mut().remove(CONTENT_LENGTH); } diff --git a/src/server/h1.rs b/src/server/h1.rs index d7edd2dcf..7b4d8a973 100644 --- a/src/server/h1.rs +++ b/src/server/h1.rs @@ -273,7 +273,7 @@ where Ok(Async::Ready(_)) => { // non consumed payload in that case close connection if self.payload.is_some() && self.tasks.is_empty() { - return Ok(Async::Ready(false)) + return Ok(Async::Ready(false)); } } } diff --git a/src/server/h1writer.rs b/src/server/h1writer.rs index 6f10fc71a..648a164f3 100644 --- a/src/server/h1writer.rs +++ b/src/server/h1writer.rs @@ -1,6 +1,6 @@ #![cfg_attr(feature = "cargo-clippy", allow(redundant_field_names))] -use bytes::{BytesMut, BufMut}; +use bytes::{BufMut, BytesMut}; use futures::{Async, Poll}; use std::io; use std::rc::Rc; diff --git a/src/server/mod.rs b/src/server/mod.rs index 51ec32160..7347b7254 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -136,7 +136,7 @@ impl HttpHandler for Box { #[doc(hidden)] pub trait HttpHandlerTask { /// Poll task, this method is used before or after *io* object is available - fn poll(&mut self) -> Poll<(), Error>{ + fn poll(&mut self) -> Poll<(), Error> { Ok(Async::Ready(())) } diff --git a/tests/test_handlers.rs b/tests/test_handlers.rs index cc6524d41..9507f1e9a 100644 --- a/tests/test_handlers.rs +++ b/tests/test_handlers.rs @@ -374,9 +374,7 @@ fn test_scope_and_path_extractor() { App::new().scope("/sc", |scope| { scope.resource("/{num}/index.html", |r| { r.route() - .with(|p: Path<(usize,)>| { - format!("Welcome {}!", p.0) - }) + .with(|p: Path<(usize,)>| format!("Welcome {}!", p.0)) }) }) }); @@ -410,10 +408,9 @@ fn test_nested_scope_and_path_extractor() { App::new().scope("/sc", |scope| { scope.nested("/{num}", |scope| { scope.resource("/{num}/index.html", |r| { - r.route() - .with(|p: Path<(usize, usize)>| { - format!("Welcome {} {}!", p.0, p.1) - }) + r.route().with(|p: Path<(usize, usize)>| { + format!("Welcome {} {}!", p.0, p.1) + }) }) }) }) From 3b08b16c113b398e630790c205d3bad1246476a2 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 23 May 2018 13:21:54 -0700 Subject: [PATCH 5/6] bump version --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2d721da23..2b561acb6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-web" -version = "0.6.9" +version = "0.6.10" authors = ["Nikolay Kim "] description = "Actix web is a simple, pragmatic and extremely fast web framework for Rust." readme = "README.md" From 17f1a2b92a733fc3698908edafeb6dc21d334fb2 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 23 May 2018 14:11:01 -0700 Subject: [PATCH 6/6] more scope tests --- src/router.rs | 6 ++-- src/scope.rs | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/router.rs b/src/router.rs index 602326f8a..81a78e577 100644 --- a/src/router.rs +++ b/src/router.rs @@ -313,11 +313,11 @@ impl Resource { } PatternType::Prefix(ref s) => if path == s { Some(s.len()) - } else if path.starts_with(s) && (s.ends_with('/') || - path.split_at(s.len()).1.starts_with('/')) + } else if path.starts_with(s) + && (s.ends_with('/') || path.split_at(s.len()).1.starts_with('/')) { if s.ends_with('/') { - Some(s.len()-1) + Some(s.len() - 1) } else { Some(s.len()) } diff --git a/src/scope.rs b/src/scope.rs index 839c4746c..dba490b2f 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -792,6 +792,40 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::CREATED); } + #[test] + fn test_scope_root2() { + let mut app = App::new() + .scope("/app/", |scope| { + scope.resource("", |r| r.f(|_| HttpResponse::Ok())) + }) + .finish(); + + let req = TestRequest::with_uri("/app").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + + let req = TestRequest::with_uri("/app/").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + } + + #[test] + fn test_scope_root3() { + let mut app = App::new() + .scope("/app/", |scope| { + scope.resource("/", |r| r.f(|_| HttpResponse::Ok())) + }) + .finish(); + + let req = TestRequest::with_uri("/app").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + + let req = TestRequest::with_uri("/app/").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + } + #[test] fn test_scope_route() { let mut app = App::new() @@ -916,6 +950,48 @@ mod tests { assert_eq!(resp.as_msg().status(), StatusCode::CREATED); } + #[test] + fn test_scope_with_state_root2() { + struct State; + + let mut app = App::new() + .scope("/app", |scope| { + scope.with_state("/t1/", State, |scope| { + scope.resource("", |r| r.f(|_| HttpResponse::Ok())) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/t1").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + + let req = TestRequest::with_uri("/app/t1/").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::OK); + } + + #[test] + fn test_scope_with_state_root3() { + struct State; + + let mut app = App::new() + .scope("/app", |scope| { + scope.with_state("/t1/", State, |scope| { + scope.resource("/", |r| r.f(|_| HttpResponse::Ok())) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/t1").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + + let req = TestRequest::with_uri("/app/t1/").finish(); + let resp = app.run(req); + assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND); + } + #[test] fn test_scope_with_state_filter() { struct State;