From 68eb2f26c9d0b3f4c07344697adb5889a1fdd334 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Wed, 23 May 2018 13:21:29 -0700 Subject: [PATCH] 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) + }) }) }) })