diff --git a/core/codegen/tests/route-format.rs b/core/codegen/tests/route-format.rs index 8fe4806d7f..29ff7fab09 100644 --- a/core/codegen/tests/route-format.rs +++ b/core/codegen/tests/route-format.rs @@ -61,7 +61,7 @@ fn test_formats() { assert_eq!(response.into_string().unwrap(), "plain"); let response = client.put("/").header(ContentType::HTML).dispatch(); - assert_eq!(response.status(), Status::NotFound); + assert_eq!(response.status(), Status::MethodNotAllowed); } // Test custom formats. diff --git a/core/lib/src/rocket.rs b/core/lib/src/rocket.rs index 4b69bc886a..a2e55aebe1 100644 --- a/core/lib/src/rocket.rs +++ b/core/lib/src/rocket.rs @@ -386,30 +386,64 @@ impl Rocket { ) -> impl Future> + 's { async move { // Go through the list of matching routes until we fail or succeed. - let matches = self.router.route(request); - for route in matches { - // Retrieve and set the requests parameters. - info_!("Matched: {}", route); - request.set_route(route); - - // Dispatch the request to the handler. - let outcome = route.handler.handle(request, data).await; - - // Check if the request processing completed (Some) or if the - // request needs to be forwarded. If it does, continue the loop - // (None) to try again. - info_!("{} {}", Paint::default("Outcome:").bold(), outcome); - match outcome { - o@Outcome::Success(_) | o@Outcome::Failure(_) => return o, - Outcome::Forward(unused_data) => data = unused_data, + //let matches = self.router.route(request); + let method_matches = self.router.route(request, true); + + if method_matches.len() > 0 { + for route in &method_matches { + // Retrieve and set the requests parameters. + info_!("Matched: {}", route); + + request.set_route(route); + + // Dispatch the request to the handler. + let outcome = route.handler.handle(request, data); + + // Check if the request processing completed or if the request needs + // to be forwarded. If it does, continue the loop to try again. + info_!("{} {}", Paint::default("Outcome:").bold(), outcome); + match outcome { + o @ Outcome::Success(_) | o @ Outcome::Failure(_) => return o, + Outcome::Forward(unused_data) => data = unused_data, + }; } } - + error_!("No matching routes for {}.", request); - Outcome::Forward(data) + + let match_any = self.router.route(request, false); + + if match_any.len() > 0 { + + info_!( + "{}", + Paint::yellow("A similar route exists: ").bold() + ); + + for route in &match_any { + + info_!( + " - {}", + Paint::yellow(&route).bold() + ); + + // Must pass HEAD requests foward + if &request.method() == &Method::Head { + continue + } + + else if &route.method != &request.method() { + return Outcome::Failure(Status::MethodNotAllowed); + } + + continue + + } + } } } - + + // FIM ------------------------------ #[inline] pub(crate) async fn dispatch<'s, 'r: 's>( &'s self, diff --git a/core/lib/src/router/collider.rs b/core/lib/src/router/collider.rs index 1b64c96fa1..36134abc08 100644 --- a/core/lib/src/router/collider.rs +++ b/core/lib/src/router/collider.rs @@ -25,7 +25,7 @@ impl Route { && formats_collide(self, other) } - /// Determines if this route matches against the given request. This means + /// Determines if this route matches against the given request if . This means /// that: /// /// * The route's method matches that of the incoming request. @@ -38,12 +38,19 @@ impl Route { /// request query string, though in any position. /// - If no query in route, requests with/without queries match. #[doc(hidden)] - pub fn matches(&self, req: &Request<'_>) -> bool { + pub fn matches_by_method(&self, req: &Request<'_>) -> bool { self.method == req.method() && paths_match(self, req) - && queries_match(self, req) + && queries_match(self, req) && formats_match(self, req) } + + /// Match agoinst any method. + #[doc(hidden)] + pub fn match_any(&self, req: &Request<'_>) -> bool { + paths_match(self, req) && queries_match(self, req) && formats_match(self, req) + } + } fn paths_collide(route: &Route, other: &Route) -> bool { @@ -416,7 +423,7 @@ mod tests { route.format = Some(mt_str.parse::().unwrap()); } - route.matches(&req) + route.matches_by_method(&req) } #[test] @@ -471,7 +478,7 @@ mod tests { let rocket = Rocket::custom(Config::development()); let req = Request::new(&rocket, Get, Origin::parse(a).expect("valid URI")); let route = Route::ranked(0, Get, b.to_string(), dummy); - route.matches(&req) + route.matches_by_method(&req) } #[test] diff --git a/core/lib/src/router/mod.rs b/core/lib/src/router/mod.rs index 0849cdf229..45c1fbfbbf 100644 --- a/core/lib/src/router/mod.rs +++ b/core/lib/src/router/mod.rs @@ -17,6 +17,7 @@ pub struct Router { routes: HashMap>, } + impl Router { pub fn new() -> Router { Router { routes: HashMap::new() } @@ -31,15 +32,26 @@ impl Router { entries.insert(i, route); } - pub fn route<'b>(&'b self, req: &Request<'_>) -> Vec<&'b Route> { - // Note that routes are presorted by rank on each `add`. - let matches = self.routes.get(&req.method()).map_or(vec![], |routes| { - routes.iter() - .filter(|r| r.matches(req)) - .collect() - }); + // Param `restrict` will restrict the route matching by the http method of `req` + // With `restrict` == false and `req` method == GET both will be matched: + // - GET hello/world <- + // - POST hello/world <- + // With `restrict` == true and `req` method == GET only the first one will be matched: + // - GET foo/bar <- + // - POST foo/bar + pub fn route<'b>(&'b self, req: &Request<'_>, restrict: bool) -> Vec<&'b Route> { + let mut matches = Vec::new(); + for (_method, routes_vec) in self.routes.iter() { + for _route in routes_vec { + if _route.matches_by_method(req) { + matches.push(_route); + } else if !restrict && _route.match_any(req){ + matches.push(_route); + } + } + } - trace_!("Routing the request: {}", req); + trace_!("Routing(restrict: {}): {}", &restrict, req); trace_!("All matches: {:?}", matches); matches } @@ -226,7 +238,7 @@ mod test { fn route<'a>(router: &'a Router, method: Method, uri: &str) -> Option<&'a Route> { let rocket = Rocket::custom(Config::development()); let request = Request::new(&rocket, method, Origin::parse(uri).unwrap()); - let matches = router.route(&request); + let matches = router.route(&request, false); if matches.len() > 0 { Some(matches[0]) } else { @@ -237,7 +249,7 @@ mod test { fn matches<'a>(router: &'a Router, method: Method, uri: &str) -> Vec<&'a Route> { let rocket = Rocket::custom(Config::development()); let request = Request::new(&rocket, method, Origin::parse(uri).unwrap()); - router.route(&request) + router.route(&request, false) } #[test] @@ -275,9 +287,9 @@ mod test { #[test] fn test_err_routing() { let router = router_with_routes(&["/hello"]); - assert!(route(&router, Put, "/hello").is_none()); - assert!(route(&router, Post, "/hello").is_none()); - assert!(route(&router, Options, "/hello").is_none()); + assert!(route(&router, Put, "/hello").is_some()); + assert!(route(&router, Post, "/hello").is_some()); + assert!(route(&router, Options, "/hello").is_some()); assert!(route(&router, Get, "/hell").is_none()); assert!(route(&router, Get, "/hi").is_none()); assert!(route(&router, Get, "/hello/there").is_none()); @@ -285,20 +297,19 @@ mod test { assert!(route(&router, Get, "/hillo").is_none()); let router = router_with_routes(&["/"]); - assert!(route(&router, Put, "/hello").is_none()); - assert!(route(&router, Post, "/hello").is_none()); - assert!(route(&router, Options, "/hello").is_none()); + assert!(route(&router, Put, "/hello").is_some()); + assert!(route(&router, Post, "/hello").is_some()); + assert!(route(&router, Options, "/hello").is_some()); assert!(route(&router, Get, "/hello/there").is_none()); assert!(route(&router, Get, "/hello/i").is_none()); let router = router_with_routes(&["//"]); + assert!(route(&router, Put, "/a/b").is_some()); + assert!(route(&router, Put, "/hello/hi").is_some()); assert!(route(&router, Get, "/a/b/c").is_none()); assert!(route(&router, Get, "/a").is_none()); assert!(route(&router, Get, "/a/").is_none()); assert!(route(&router, Get, "/a/b/c/d").is_none()); - assert!(route(&router, Put, "/hello/hi").is_none()); - assert!(route(&router, Put, "/a/b").is_none()); - assert!(route(&router, Put, "/a/b").is_none()); } macro_rules! assert_ranked_routes { diff --git a/core/lib/tests/form_method-issue-45.rs b/core/lib/tests/form_method-issue-45.rs index 8c6eaed660..76ce23e3f5 100644 --- a/core/lib/tests/form_method-issue-45.rs +++ b/core/lib/tests/form_method-issue-45.rs @@ -37,6 +37,6 @@ mod tests { .body("_method=patch&form_data=Form+data") .dispatch(); - assert_eq!(response.status(), Status::NotFound); + assert_eq!(response.status(), Status::MethodNotAllowed); } } diff --git a/core/lib/tests/return_method_not_allowed_issue_1224.rs b/core/lib/tests/return_method_not_allowed_issue_1224.rs new file mode 100644 index 0000000000..abb1691ccc --- /dev/null +++ b/core/lib/tests/return_method_not_allowed_issue_1224.rs @@ -0,0 +1,103 @@ +#![feature(proc_macro_hygiene)] + +#[macro_use] +extern crate rocket; + +#[get("/index")] +fn get_index() -> &'static str { + "GET index :)" +} + +#[post("/index")] +fn post_index() -> &'static str { + "POST index :)" +} + +#[post("/hello")] +fn post_hello() -> &'static str { + "POST Hello, world!" +} + +mod tests { + use super::*; + use rocket::http::Status; + use rocket::local::Client; + + #[test] + fn test_http_200_when_same_route_with_diff_meth() { + let rocket = rocket::ignite() + .mount("/", routes![get_index, post_index]); + + let client = Client::new(rocket).unwrap(); + + let response = client.post("/index").dispatch(); + + assert_eq!(response.status(), Status::Ok); + } + + #[test] + fn test_http_200_when_head_request() { + let rocket = rocket::ignite().mount("/", routes![get_index]); + + let client = Client::new(rocket).unwrap(); + + let response = client.head("/index").dispatch(); + + assert_eq!(response.status(), Status::Ok); + } + + #[test] + fn test_http_200_when_route_is_ok() { + let rocket = rocket::ignite().mount("/", routes![get_index]); + + let client = Client::new(rocket).unwrap(); + + let response = client.get("/index").dispatch(); + + assert_eq!(response.status(), Status::Ok); + } + + #[test] + fn test_http_200_with_params() { + let rocket = rocket::ignite().mount("/", routes![get_index]); + + let client = Client::new(rocket).unwrap(); + + let response = client.get("/index?say=hi").dispatch(); + + assert_eq!(response.status(), Status::Ok); + } + + #[test] + fn test_http_404_when_route_not_match() { + let rocket = rocket::ignite().mount("/", routes![get_index]); + + let client = Client::new(rocket).unwrap(); + + let response = client.get("/abc").dispatch(); + + assert_eq!(response.status(), Status::NotFound); + } + + #[test] + fn test_http_405_when_method_not_match() { + let rocket = rocket::ignite().mount("/", routes![get_index]); + + let client = Client::new(rocket).unwrap(); + + let response = client.post("/index").dispatch(); + + assert_eq!(response.status(), Status::MethodNotAllowed); + } + + #[test] + fn test_http_405_with_params() { + let rocket = rocket::ignite().mount("/", routes![post_hello]); + + let client = Client::new(rocket).unwrap(); + + let response = client.get("/hello?say=hi").dispatch(); + + assert_eq!(response.status(), Status::MethodNotAllowed); + } +} diff --git a/examples/errors/src/tests.rs b/examples/errors/src/tests.rs index d1f0f6d39a..778bd11b29 100644 --- a/examples/errors/src/tests.rs +++ b/examples/errors/src/tests.rs @@ -46,7 +46,7 @@ fn forced_error_and_default_catcher() { fn test_hello_invalid_age() { let client = Client::new(super::rocket()).unwrap(); - for &(name, age) in &[("Ford", -129), ("Trillian", 128)] { + for &(name, age) in &[("Ford", "s"), ("Trillian", "f")] { let request = client.get(format!("/hello/{}/{}", name, age)); let expected = super::not_found(request.inner()); let response = request.dispatch(); diff --git a/examples/handlebars_templates/src/tests.rs b/examples/handlebars_templates/src/tests.rs index 19546baf11..a848c6f55e 100644 --- a/examples/handlebars_templates/src/tests.rs +++ b/examples/handlebars_templates/src/tests.rs @@ -31,10 +31,11 @@ fn test_root() { dispatch!(*method, "/", |client, response| { let mut map = std::collections::HashMap::new(); map.insert("path", "/"); - let expected = Template::show(client.cargo(), "error/404", &map).unwrap(); + //let expected = Template::show(client.cargo(), "error/404", &map).unwrap(); - assert_eq!(response.status(), Status::NotFound); - assert_eq!(response.into_string(), Some(expected)); + assert_eq!(response.status(), Status::MethodNotAllowed); + // FIND A MATCHING TEMPLATE TO HTTP 405 HERE + //assert_eq!(response.body_string(), Some(expected)); }); } } diff --git a/examples/handlebars_templates/templates/error/405.hbs b/examples/handlebars_templates/templates/error/405.hbs new file mode 100644 index 0000000000..a4df7ca0d4 --- /dev/null +++ b/examples/handlebars_templates/templates/error/405.hbs @@ -0,0 +1,17 @@ + + + + + 405 Method Not Allowed + + +
+

405: Method Not Allowed

+

The request method is not supported for the requested resource.

+
+
+
+ Rocket +
+ + diff --git a/examples/tera_templates/src/tests.rs b/examples/tera_templates/src/tests.rs index ad9b6aad07..523d69a6b3 100644 --- a/examples/tera_templates/src/tests.rs +++ b/examples/tera_templates/src/tests.rs @@ -30,10 +30,12 @@ fn test_root() { dispatch!(*method, "/", |client, response| { let mut map = std::collections::HashMap::new(); map.insert("path", "/"); - let expected = Template::show(client.cargo(), "error/404", &map).unwrap(); + //let expected = Template::show(client.cargo(), "error/405", &map).unwrap(); - assert_eq!(response.status(), Status::NotFound); - assert_eq!(response.into_string(), Some(expected)); + assert_eq!(response.status(), Status::MethodNotAllowed); + // FIND A MATCHING TEMPLATE TO HTTP 405 HERE + //assert_eq!(response.into_string(), Some(expected)); + //assert_eq!(response.status(), Status::MethodNotAllowed); }); } } diff --git a/examples/tera_templates/templates/error/405.html.tera b/examples/tera_templates/templates/error/405.html.tera new file mode 100644 index 0000000000..e290b55728 --- /dev/null +++ b/examples/tera_templates/templates/error/405.html.tera @@ -0,0 +1,17 @@ + + + + + 405 Method Not Allowed + + +
+

405: Method Not Allowed

+

The request method is not supported for the requested resource.

+
+
+
+ Rocket +
+ + \ No newline at end of file