tests for NormalizePath

src/application.rs

@@ -21,7 +21,11 @@ pub struct HttpApplication<S> {
 
 impl<S: 'static> HttpApplication<S> {
 
-    fn run(&self, mut req: HttpRequest<S>) -> Reply {
+    pub(crate) fn prepare_request(&self, req: HttpRequest) -> HttpRequest<S> {
+        req.with_state(Rc::clone(&self.state), self.router.clone())
+    }
+
+    pub(crate) fn run(&self, mut req: HttpRequest<S>) -> Reply {
         if let Some(h) = self.router.recognize(&mut req) {
             h.handle(req)
         } else {
@@ -34,8 +38,7 @@ impl<S: 'static> HttpHandler for HttpApplication<S> {
 
     fn handle(&self, req: HttpRequest) -> Result<Box<HttpHandlerTask>, 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<S>| 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<HttpResponse> {
-            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();
+        let resp = app.run(req);
-        assert_eq!(resp.status(), StatusCode::OK);
+        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();
+        let resp = app.run(req);
-        assert_eq!(resp.status(), StatusCode::NOT_FOUND);
+        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();
+        let resp = app.run(req);
-        assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED);
+        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!(
+        let resp = app.run(req);
-            app.run(req).msg().unwrap().status(), StatusCode::OK);
+        assert_eq!(resp.as_response().unwrap().status(), StatusCode::OK);
     }
 }

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<S> Handler<S> for NormalizePath {
 
     fn handle(&self, req: HttpRequest<S>) -> 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<S> Handler<S> 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<S> Handler<S> 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<S> Handler<S> 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());
+            }
+        }
+    }
+
+
 }

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<S>(self, state: Rc<S>, router: Router<S>) -> HttpRequest<S> {
         HttpRequest(self.0, state, Some(router))