diff --git a/src/application.rs b/src/application.rs index 291b33d8a..4d18cb6fa 100644 --- a/src/application.rs +++ b/src/application.rs @@ -21,7 +21,11 @@ pub struct HttpApplication { impl HttpApplication { - fn run(&self, mut req: HttpRequest) -> Reply { + pub(crate) fn prepare_request(&self, req: HttpRequest) -> HttpRequest { + req.with_state(Rc::clone(&self.state), self.router.clone()) + } + + pub(crate) fn run(&self, mut req: HttpRequest) -> Reply { if let Some(h) = self.router.recognize(&mut req) { h.handle(req) } else { @@ -34,8 +38,7 @@ impl HttpHandler for HttpApplication { fn handle(&self, req: HttpRequest) -> Result, HttpRequest> { if req.path().starts_with(&self.prefix) { - let req = req.with_state(Rc::clone(&self.state), self.router.clone()); - + let req = self.prepare_request(req); Ok(Box::new(Pipeline::new(req, Rc::clone(&self.middlewares), &|req: HttpRequest| self.run(req)))) } else { @@ -266,21 +269,10 @@ mod tests { use std::str::FromStr; use http::{Method, Version, Uri, HeaderMap, StatusCode}; use super::*; - use handler::ReplyItem; use httprequest::HttpRequest; - use httpresponse::HttpResponse; use payload::Payload; use httpcodes; - impl Reply { - fn msg(self) -> Option { - match self.into() { - ReplyItem::Message(resp) => Some(resp), - _ => None, - } - } - } - #[test] fn test_default_resource() { let app = Application::new("/") @@ -290,14 +282,14 @@ mod tests { let req = HttpRequest::new( Method::GET, Uri::from_str("/test").unwrap(), Version::HTTP_11, HeaderMap::new(), Payload::empty()); - let resp = app.run(req).msg().unwrap(); - assert_eq!(resp.status(), StatusCode::OK); + let resp = app.run(req); + assert_eq!(resp.as_response().unwrap().status(), StatusCode::OK); let req = HttpRequest::new( Method::GET, Uri::from_str("/blah").unwrap(), Version::HTTP_11, HeaderMap::new(), Payload::empty()); - let resp = app.run(req).msg().unwrap(); - assert_eq!(resp.status(), StatusCode::NOT_FOUND); + let resp = app.run(req); + assert_eq!(resp.as_response().unwrap().status(), StatusCode::NOT_FOUND); let app = Application::new("/") .default_resource(|r| r.h(httpcodes::HTTPMethodNotAllowed)) @@ -305,8 +297,8 @@ mod tests { let req = HttpRequest::new( Method::GET, Uri::from_str("/blah").unwrap(), Version::HTTP_11, HeaderMap::new(), Payload::empty()); - let resp = app.run(req).msg().unwrap(); - assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED); + let resp = app.run(req); + assert_eq!(resp.as_response().unwrap().status(), StatusCode::METHOD_NOT_ALLOWED); } #[test] @@ -323,7 +315,7 @@ mod tests { .resource("/", |r| r.h(httpcodes::HTTPOk)) .finish(); let req = HttpRequest::default().with_state(Rc::clone(&app.state), app.router.clone()); - assert_eq!( - app.run(req).msg().unwrap().status(), StatusCode::OK); + let resp = app.run(req); + assert_eq!(resp.as_response().unwrap().status(), StatusCode::OK); } } diff --git a/src/handler.rs b/src/handler.rs index 16c066f5d..24cb15a7c 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -79,6 +79,14 @@ impl Reply { pub(crate) fn into(self) -> ReplyItem { self.0 } + + #[cfg(test)] + pub(crate) fn as_response(&self) -> Option<&HttpResponse> { + match self.0 { + ReplyItem::Message(ref resp) => Some(resp), + _ => None, + } + } } impl FromRequest for Reply { @@ -342,11 +350,13 @@ impl Handler for NormalizePath { fn handle(&self, req: HttpRequest) -> Self::Result { if let Some(router) = req.router() { + let query = req.query_string(); if self.merge { // merge slashes let p = self.re_merge.replace_all(req.path(), "/"); if p.len() != req.path().len() { if router.has_route(p.as_ref()) { + let p = if !query.is_empty() { p + "?" + query } else { p }; return HttpResponse::build(self.redirect) .header(header::LOCATION, p.as_ref()) .body(Body::Empty); @@ -355,6 +365,7 @@ impl Handler for NormalizePath { if self.append && !p.ends_with('/') { let p = p.as_ref().to_owned() + "/"; if router.has_route(&p) { + let p = if !query.is_empty() { p + "?" + query } else { p }; return HttpResponse::build(self.redirect) .header(header::LOCATION, p.as_str()) .body(Body::Empty); @@ -366,6 +377,7 @@ impl Handler for NormalizePath { if self.append && !req.path().ends_with('/') { let p = req.path().to_owned() + "/"; if router.has_route(&p) { + let p = if !query.is_empty() { p + "?" + query } else { p }; return HttpResponse::build(self.redirect) .header(header::LOCATION, p.as_str()) .body(Body::Empty); @@ -379,7 +391,8 @@ impl Handler for NormalizePath { #[cfg(test)] mod tests { use super::*; - use http::header; + use http::{header, Method}; + use application::Application; #[derive(Serialize)] struct MyObj { @@ -392,4 +405,161 @@ mod tests { let resp = json.from_request(HttpRequest::default()).unwrap(); assert_eq!(resp.headers().get(header::CONTENT_TYPE).unwrap(), "application/json"); } + + fn index(_req: HttpRequest) -> HttpResponse { + HttpResponse::new(StatusCode::OK, Body::Empty) + } + + #[test] + fn test_normalize_path_trailing_slashes() { + let app = Application::new("/") + .resource("/resource1", |r| r.method(Method::GET).f(index)) + .resource("/resource2/", |r| r.method(Method::GET).f(index)) + .default_resource(|r| r.h(NormalizePath::default())) + .finish(); + + // trailing slashes + let params = vec![("/resource1", "", StatusCode::OK), + ("/resource1/", "", StatusCode::NOT_FOUND), + ("/resource2", "/resource2/", StatusCode::MOVED_PERMANENTLY), + ("/resource2/", "", StatusCode::OK), + ("/resource1?p1=1&p2=2", "", StatusCode::OK), + ("/resource1/?p1=1&p2=2", "", StatusCode::NOT_FOUND), + ("/resource2?p1=1&p2=2", "/resource2/?p1=1&p2=2", + StatusCode::MOVED_PERMANENTLY), + ("/resource2/?p1=1&p2=2", "", StatusCode::OK) + ]; + for (path, target, code) in params { + let req = app.prepare_request(HttpRequest::from_path(path)); + let resp = app.run(req); + let r = resp.as_response().unwrap(); + assert_eq!(r.status(), code); + if !target.is_empty() { + assert_eq!( + target, + r.headers().get(header::LOCATION).unwrap().to_str().unwrap()); + } + } + } + + #[test] + fn test_normalize_path_trailing_slashes_disabled() { + let app = Application::new("/") + .resource("/resource1", |r| r.method(Method::GET).f(index)) + .resource("/resource2/", |r| r.method(Method::GET).f(index)) + .default_resource(|r| r.h( + NormalizePath::new(false, true, StatusCode::MOVED_PERMANENTLY))) + .finish(); + + // trailing slashes + let params = vec![("/resource1", StatusCode::OK), + ("/resource1/", StatusCode::NOT_FOUND), + ("/resource2", StatusCode::NOT_FOUND), + ("/resource2/", StatusCode::OK), + ("/resource1?p1=1&p2=2", StatusCode::OK), + ("/resource1/?p1=1&p2=2", StatusCode::NOT_FOUND), + ("/resource2?p1=1&p2=2", StatusCode::NOT_FOUND), + ("/resource2/?p1=1&p2=2", StatusCode::OK) + ]; + for (path, code) in params { + let req = app.prepare_request(HttpRequest::from_path(path)); + let resp = app.run(req); + let r = resp.as_response().unwrap(); + assert_eq!(r.status(), code); + } + } + + #[test] + fn test_normalize_path_merge_slashes() { + let app = Application::new("/") + .resource("/resource1", |r| r.method(Method::GET).f(index)) + .resource("/resource1/a/b", |r| r.method(Method::GET).f(index)) + .default_resource(|r| r.h(NormalizePath::default())) + .finish(); + + // trailing slashes + let params = vec![ + ("/resource1/a/b", "", StatusCode::OK), + ("//resource1//a//b", "/resource1/a/b", StatusCode::MOVED_PERMANENTLY), + ("//resource1//a//b/", "", StatusCode::NOT_FOUND), + ("///resource1//a//b", "/resource1/a/b", StatusCode::MOVED_PERMANENTLY), + ("/////resource1/a///b", "/resource1/a/b", StatusCode::MOVED_PERMANENTLY), + ("/////resource1/a//b/", "", StatusCode::NOT_FOUND), + ("/resource1/a/b?p=1", "", StatusCode::OK), + ("//resource1//a//b?p=1", "/resource1/a/b?p=1", StatusCode::MOVED_PERMANENTLY), + ("//resource1//a//b/?p=1", "", StatusCode::NOT_FOUND), + ("///resource1//a//b?p=1", "/resource1/a/b?p=1", StatusCode::MOVED_PERMANENTLY), + ("/////resource1/a///b?p=1", "/resource1/a/b?p=1", StatusCode::MOVED_PERMANENTLY), + ("/////resource1/a//b/?p=1", "", StatusCode::NOT_FOUND), + ]; + for (path, target, code) in params { + let req = app.prepare_request(HttpRequest::from_path(path)); + let resp = app.run(req); + let r = resp.as_response().unwrap(); + assert_eq!(r.status(), code); + if !target.is_empty() { + assert_eq!( + target, + r.headers().get(header::LOCATION).unwrap().to_str().unwrap()); + } + } + } + + #[test] + fn test_normalize_path_merge_and_append_slashes() { + let app = Application::new("/") + .resource("/resource1", |r| r.method(Method::GET).f(index)) + .resource("/resource2/", |r| r.method(Method::GET).f(index)) + .resource("/resource1/a/b", |r| r.method(Method::GET).f(index)) + .resource("/resource2/a/b/", |r| r.method(Method::GET).f(index)) + .default_resource(|r| r.h(NormalizePath::default())) + .finish(); + + // trailing slashes + let params = vec![ + ("/resource1/a/b", "", StatusCode::OK), + ("/resource1/a/b/", "", StatusCode::NOT_FOUND), + ("//resource2//a//b", "/resource2/a/b/", StatusCode::MOVED_PERMANENTLY), + ("//resource2//a//b/", "/resource2/a/b/", StatusCode::MOVED_PERMANENTLY), + ("///resource1//a//b", "/resource1/a/b", StatusCode::MOVED_PERMANENTLY), + ("///resource1//a//b/", "", StatusCode::NOT_FOUND), + ("/////resource1/a///b", "/resource1/a/b", StatusCode::MOVED_PERMANENTLY), + ("/////resource1/a///b/", "", StatusCode::NOT_FOUND), + ("/resource2/a/b", "/resource2/a/b/", StatusCode::MOVED_PERMANENTLY), + ("/resource2/a/b/", "", StatusCode::OK), + ("//resource2//a//b", "/resource2/a/b/", StatusCode::MOVED_PERMANENTLY), + ("//resource2//a//b/", "/resource2/a/b/", StatusCode::MOVED_PERMANENTLY), + ("///resource2//a//b", "/resource2/a/b/", StatusCode::MOVED_PERMANENTLY), + ("///resource2//a//b/", "/resource2/a/b/", StatusCode::MOVED_PERMANENTLY), + ("/////resource2/a///b", "/resource2/a/b/", StatusCode::MOVED_PERMANENTLY), + ("/////resource2/a///b/", "/resource2/a/b/", StatusCode::MOVED_PERMANENTLY), + ("/resource1/a/b?p=1", "", StatusCode::OK), + ("/resource1/a/b/?p=1", "", StatusCode::NOT_FOUND), + ("//resource2//a//b?p=1", "/resource2/a/b/?p=1", StatusCode::MOVED_PERMANENTLY), + ("//resource2//a//b/?p=1", "/resource2/a/b/?p=1", StatusCode::MOVED_PERMANENTLY), + ("///resource1//a//b?p=1", "/resource1/a/b?p=1", StatusCode::MOVED_PERMANENTLY), + ("///resource1//a//b/?p=1", "", StatusCode::NOT_FOUND), + ("/////resource1/a///b?p=1", "/resource1/a/b?p=1", StatusCode::MOVED_PERMANENTLY), + ("/////resource1/a///b/?p=1", "", StatusCode::NOT_FOUND), + ("/resource2/a/b?p=1", "/resource2/a/b/?p=1", StatusCode::MOVED_PERMANENTLY), + ("//resource2//a//b?p=1", "/resource2/a/b/?p=1", StatusCode::MOVED_PERMANENTLY), + ("//resource2//a//b/?p=1", "/resource2/a/b/?p=1", StatusCode::MOVED_PERMANENTLY), + ("///resource2//a//b?p=1", "/resource2/a/b/?p=1", StatusCode::MOVED_PERMANENTLY), + ("///resource2//a//b/?p=1", "/resource2/a/b/?p=1", StatusCode::MOVED_PERMANENTLY), + ("/////resource2/a///b?p=1", "/resource2/a/b/?p=1", StatusCode::MOVED_PERMANENTLY), + ("/////resource2/a///b/?p=1", "/resource2/a/b/?p=1", StatusCode::MOVED_PERMANENTLY), + ]; + for (path, target, code) in params { + let req = app.prepare_request(HttpRequest::from_path(path)); + let resp = app.run(req); + let r = resp.as_response().unwrap(); + assert_eq!(r.status(), code); + if !target.is_empty() { + assert_eq!( + target, r.headers().get(header::LOCATION).unwrap().to_str().unwrap()); + } + } + } + + } diff --git a/src/httprequest.rs b/src/httprequest.rs index 092299594..3752cdf36 100644 --- a/src/httprequest.rs +++ b/src/httprequest.rs @@ -5,9 +5,9 @@ use std::net::SocketAddr; use std::collections::HashMap; use bytes::BytesMut; use futures::{Async, Future, Stream, Poll}; -use url::{Url, form_urlencoded}; use cookie::Cookie; use http_range::HttpRange; +use url::{Url, form_urlencoded}; use http::{header, Uri, Method, Version, HeaderMap, Extensions}; use info::ConnectionInfo; @@ -98,6 +98,31 @@ impl HttpRequest<()> { ) } + /// Construct a new Request. + #[inline] + #[cfg(test)] + pub fn from_path(path: &str) -> HttpRequest + { + use std::str::FromStr; + + HttpRequest( + Rc::new(HttpMessage { + method: Method::GET, + uri: Uri::from_str(path).unwrap(), + version: Version::HTTP_11, + headers: HeaderMap::new(), + params: Params::default(), + cookies: None, + addr: None, + payload: Payload::empty(), + extensions: Extensions::new(), + info: None, + }), + Rc::new(()), + None, + ) + } + /// Construct new http request with state. pub fn with_state(self, state: Rc, router: Router) -> HttpRequest { HttpRequest(self.0, state, Some(router))