A route with unspecified query parameters accepts any.

This is a breaking change. It modifies collisions with respect to query
parameters as well as the default ranking of routes.

A route that does not specify query parameters will now match against
requests with _and without_ query parameters, assuming all other
elements of the route match as well. A route that _does_ specify query
parameters will only match requests with query parameters; this remains
true.

To accommodate this change in the most natural manner possible, the
default rankings of routes have changed as illustrated below:

  |-------------+-------+----------+---------------|
  | static path | query | new rank | previous rank |
  |-------------+-------+----------+---------------|
  | yes         | yes   | -4       | 0             |
  | yes         | no    | -3       | 0             |
  | no          | yes   | -2       | 1             |
  | no          | no    | -1       | 1             |
  |-------------+-------+----------+---------------|

In other words, the most specific routes, with preference for paths over
queries, are ranked highest (lower number).

examples/static_files/src/tests.rs

@@ -34,20 +34,26 @@ fn read_file_content(path: &str) -> Vec<u8> {
 #[test]
 fn test_index_html() {
     test_query_file("/", "static/index.html", Status::Ok);
+    test_query_file("/?v=1", "static/index.html", Status::Ok);
+    test_query_file("/?this=should&be=ignored", "static/index.html", Status::Ok);
 }
 
 #[test]
 fn test_hidden_file() {
     test_query_file("/hidden/hi.txt", "static/hidden/hi.txt", Status::Ok);
+    test_query_file("/hidden/hi.txt?v=1", "static/hidden/hi.txt", Status::Ok);
+    test_query_file("/hidden/hi.txt?v=1&a=b", "static/hidden/hi.txt", Status::Ok);
 }
 
 #[test]
 fn test_icon_file() {
     test_query_file("/rocket-icon.jpg", "static/rocket-icon.jpg", Status::Ok);
+    test_query_file("/rocket-icon.jpg", "static/rocket-icon.jpg", Status::Ok);
 }
 
 #[test]
 fn test_invalid_path() {
     test_query_file("/thou_shalt_not_exist", None, Status::NotFound);
     test_query_file("/thou/shalt/not/exist", None, Status::NotFound);
+    test_query_file("/thou/shalt/not/exist?a=b&c=d", None, Status::NotFound);
 }

lib/src/http/parse/accept.rs

@@ -35,8 +35,8 @@ pub fn parse_accept(mut input: &str) -> Result<Accept, ParseError<&str>> {
 
 #[cfg(test)]
 mod test {
-    use http::{Accept, MediaType, WeightedMediaType};
-    use super::{ParseResult, parse_accept};
+    use http::MediaType;
+    use super::parse_accept;
 
     macro_rules! assert_no_parse {
         ($string:expr) => ({

lib/src/router/collider.rs

@@ -50,12 +50,9 @@ impl<'a> Collider<str> for &'a str {
     }
 }
 
+// This _only_ checks the `path` component of the URI.
 impl<'a, 'b> Collider<URI<'b>> for URI<'a> {
     fn collides_with(&self, other: &URI<'b>) -> bool {
-        if self.query().is_some() != other.query().is_some() {
-            return false;
-        }
-
         for (seg_a, seg_b) in self.segments().zip(other.segments()) {
             if seg_a.ends_with("..>") || seg_b.ends_with("..>") {
                 return true;
@@ -83,8 +80,13 @@ impl Collider for ContentType  {
 
 // This implementation is used at initialization to check if two user routes
 // collide before launching. Format collisions works like this:
-//   * If route a specifies format, it only gets requests for that format.
-//   * If a route doesn't specify format, it gets requests for any format.
+//   * If route specifies format, it only gets requests for that format.
+//   * If route doesn't specify format, it gets requests for any format.
+// Query collisions work like this:
+//   * If route specifies qeury, it only gets request that have queries.
+//   * If route doesn't specify qeury, requests with and without queries match.
+// As a result, as long as everything else collides, whether a route has a query
+// or not is irrelevant: it will collide.
 impl Collider for Route {
     fn collides_with(&self, b: &Route) -> bool {
         self.method == b.method
@@ -101,12 +103,16 @@ impl Collider for Route {
 
 // This implementation is used at runtime to check if a given request is
 // intended for this Route. Format collisions works like this:
-//   * If route a specifies format, it only gets requests for that format.
-//   * If a route doesn't specify format, it gets requests for any format.
+//   * If route specifies format, it only gets requests for that format.
+//   * If route doesn't specify format, it gets requests for any format.
+// Query collisions work like this:
+//   * If route specifies a query, it only gets request that have queries.
+//   * If route doesn't specify query, requests with & without queries collide.
 impl<'r> Collider<Request<'r>> for Route {
     fn collides_with(&self, req: &Request<'r>) -> bool {
         self.method == req.method()
-            && req.uri().collides_with(&self.path)
+            && self.path.collides_with(req.uri())
+            && self.path.query().map_or(true, |_| req.uri().query().is_some())
             // FIXME: On payload requests, check Content-Type, else Accept.
             && match (req.content_type().as_ref(), self.format.as_ref()) {
                 (Some(ct_a), Some(ct_b)) => ct_a.collides_with(ct_b),
@@ -198,7 +204,6 @@ mod tests {
         assert!(unranked_collide("/<a..>", "///a///"));
     }
 
-
     #[test]
     fn query_collisions() {
         assert!(unranked_collide("/?<a>", "/?<a>"));
@@ -207,6 +212,11 @@ mod tests {
         assert!(unranked_collide("/<r>?<a>", "/<r>?<a>"));
         assert!(unranked_collide("/a/b/c?<a>", "/a/b/c?<a>"));
         assert!(unranked_collide("/<a>/b/c?<d>", "/a/b/<c>?<d>"));
+        assert!(unranked_collide("/?<a>", "/"));
+        assert!(unranked_collide("/a?<a>", "/a"));
+        assert!(unranked_collide("/a?<a>", "/a"));
+        assert!(unranked_collide("/a/b?<a>", "/a/b"));
+        assert!(unranked_collide("/a/b", "/a/b?<c>"));
     }
 
     #[test]
@@ -231,12 +241,11 @@ mod tests {
 
     #[test]
     fn query_non_collisions() {
-        assert!(!unranked_collide("/?<a>", "/"));
+        assert!(!unranked_collide("/a?<b>", "/b"));
+        assert!(!unranked_collide("/a/b", "/a?<b>"));
+        assert!(!unranked_collide("/a/b/c?<d>", "/a/b/c/d"));
+        assert!(!unranked_collide("/a/hello", "/a/?<hello>"));
         assert!(!unranked_collide("/?<a>", "/hi"));
-        assert!(!unranked_collide("/?<a>", "/a"));
-        assert!(!unranked_collide("/a?<a>", "/a"));
-        assert!(!unranked_collide("/a/b?<a>", "/a/b"));
-        assert!(!unranked_collide("/a/b", "/a/b/?<c>"));
     }
 
     #[test]
@@ -353,7 +362,7 @@ mod tests {
         assert!(!r_ct_ct_collide(Get, "text/html", Get, "text/css"));
     }
 
-    fn req_route_collide<S1, S2>(m1: Method, ct1: S1, m2: Method, ct2: S2) -> bool
+    fn req_route_ct_collide<S1, S2>(m1: Method, ct1: S1, m2: Method, ct2: S2) -> bool
         where S1: Into<Option<&'static str>>, S2: Into<Option<&'static str>>
     {
         let mut req = Request::new(m1, "/");
@@ -371,23 +380,49 @@ mod tests {
 
     #[test]
     fn test_req_route_ct_collisions() {
-        assert!(req_route_collide(Get, "application/json", Get, "application/json"));
-        assert!(req_route_collide(Get, "application/json", Get, "application/*"));
-        assert!(req_route_collide(Get, "application/json", Get, "*/json"));
-        assert!(req_route_collide(Get, "text/html", Get, "text/html"));
-        assert!(req_route_collide(Get, "text/html", Get, "*/*"));
+        assert!(req_route_ct_collide(Get, "application/json", Get, "application/json"));
+        assert!(req_route_ct_collide(Get, "application/json", Get, "application/*"));
+        assert!(req_route_ct_collide(Get, "application/json", Get, "*/json"));
+        assert!(req_route_ct_collide(Get, "text/html", Get, "text/html"));
+        assert!(req_route_ct_collide(Get, "text/html", Get, "*/*"));
 
-        assert!(req_route_collide(Get, "text/html", Get, None));
-        assert!(req_route_collide(Get, None, Get, None));
-        assert!(req_route_collide(Get, "application/json", Get, None));
-        assert!(req_route_collide(Get, "x-custom/anything", Get, None));
+        assert!(req_route_ct_collide(Get, "text/html", Get, None));
+        assert!(req_route_ct_collide(Get, None, Get, None));
+        assert!(req_route_ct_collide(Get, "application/json", Get, None));
+        assert!(req_route_ct_collide(Get, "x-custom/anything", Get, None));
 
-        assert!(!req_route_collide(Get, "application/json", Get, "text/html"));
-        assert!(!req_route_collide(Get, "application/json", Get, "text/*"));
-        assert!(!req_route_collide(Get, "application/json", Get, "*/xml"));
+        assert!(!req_route_ct_collide(Get, "application/json", Get, "text/html"));
+        assert!(!req_route_ct_collide(Get, "application/json", Get, "text/*"));
+        assert!(!req_route_ct_collide(Get, "application/json", Get, "*/xml"));
 
-        assert!(!req_route_collide(Get, None, Get, "text/html"));
-        assert!(!req_route_collide(Get, None, Get, "*/*"));
-        assert!(!req_route_collide(Get, None, Get, "application/json"));
+        assert!(!req_route_ct_collide(Get, None, Get, "text/html"));
+        assert!(!req_route_ct_collide(Get, None, Get, "*/*"));
+        assert!(!req_route_ct_collide(Get, None, Get, "application/json"));
+    }
+
+    fn req_route_path_collide(a: &'static str, b: &'static str) -> bool {
+        let req = Request::new(Get, a.to_string());
+        let route = Route::ranked(0, Get, b.to_string(), dummy_handler);
+        route.collides_with(&req)
+    }
+
+    #[test]
+    fn test_req_route_query_collisions() {
+        assert!(req_route_path_collide("/a/b?a=b", "/a/b?<c>"));
+        assert!(req_route_path_collide("/a/b?a=b", "/<a>/b?<c>"));
+        assert!(req_route_path_collide("/a/b?a=b", "/<a>/<b>?<c>"));
+        assert!(req_route_path_collide("/a/b?a=b", "/a/<b>?<c>"));
+        assert!(req_route_path_collide("/?b=c", "/?<b>"));
+
+        assert!(req_route_path_collide("/a/b?a=b", "/a/b"));
+        assert!(req_route_path_collide("/a/b", "/a/b"));
+        assert!(req_route_path_collide("/a/b/c/d?", "/a/b/c/d"));
+        assert!(req_route_path_collide("/a/b/c/d?v=1&v=2", "/a/b/c/d"));
+
+        assert!(!req_route_path_collide("/a/b", "/a/b?<c>"));
+        assert!(!req_route_path_collide("/a/b/c", "/a/b?<c>"));
+        assert!(!req_route_path_collide("/a?b=c", "/a/b?<c>"));
+        assert!(!req_route_path_collide("/?b=c", "/a/b?<c>"));
+        assert!(!req_route_path_collide("/?b=c", "/a?<c>"));
     }
 }

lib/src/router/mod.rs

@@ -245,6 +245,14 @@ mod test {
         assert_ranked_routes!(&["/<a>/b", "/hi/c"], "/hi/c", "/hi/c");
         assert_ranked_routes!(&["/<a>/<b>", "/hi/a"], "/hi/c", "/<a>/<b>");
         assert_ranked_routes!(&["/hi/a", "/hi/<c>"], "/hi/c", "/hi/<c>");
+        assert_ranked_routes!(&["/a", "/a?<b>"], "/a?b=c", "/a?<b>");
+        assert_ranked_routes!(&["/a", "/a?<b>"], "/a", "/a");
+        assert_ranked_routes!(&["/a", "/<a>", "/a?<b>", "/<a>?<b>"], "/a", "/a");
+        assert_ranked_routes!(&["/a", "/<a>", "/a?<b>", "/<a>?<b>"], "/b", "/<a>");
+        assert_ranked_routes!(&["/a", "/<a>", "/a?<b>", "/<a>?<b>"],
+                              "/b?v=1", "/<a>?<b>");
+        assert_ranked_routes!(&["/a", "/<a>", "/a?<b>", "/<a>?<b>"],
+                              "/a?b=c", "/a?<b>");
     }
 
     fn ranked_collisions(routes: &[(isize, &'static str)]) -> bool {
@@ -341,6 +349,45 @@ mod test {
         );
     }
 
+    macro_rules! assert_default_ranked_routing {
+        (to: $to:expr, with: $routes:expr, expect: $($want:expr),+) => ({
+            let router = router_with_routes(&$routes);
+            let routed_to = matches(&router, Get, $to);
+            let expected = &[$($want),+];
+            assert!(routed_to.len() == expected.len());
+            for (got, expected) in routed_to.iter().zip(expected.iter()) {
+                assert_eq!(got.path.as_str() as &str, expected as &str);
+            }
+        })
+    }
+
+    #[test]
+    fn test_default_ranked_routing() {
+        assert_default_ranked_routing!(
+            to: "a/b?v=1",
+            with: ["a/<b>", "a/b"],
+            expect: "a/b", "a/<b>"
+        );
+
+        assert_default_ranked_routing!(
+            to: "a/b?v=1",
+            with: ["a/<b>", "a/b", "a/b?<v>"],
+            expect: "a/b?<v>", "a/b", "a/<b>"
+        );
+
+        assert_default_ranked_routing!(
+            to: "a/b?v=1",
+            with: ["a/<b>", "a/b", "a/b?<v>", "a/<b>?<v>"],
+            expect: "a/b?<v>", "a/b", "a/<b>?<v>", "a/<b>"
+        );
+
+        assert_default_ranked_routing!(
+            to: "a/b",
+            with: ["a/<b>", "a/b", "a/b?<v>", "a/<b>?<v>"],
+            expect: "a/b", "a/<b>"
+        );
+    }
+
     fn match_params(router: &Router, path: &str, expected: &[&str]) -> bool {
         println!("Testing: {} (expect: {:?})", path, expected);
         route(router, Get, path).map_or(false, |route| {

lib/src/router/route.rs

@@ -23,10 +23,16 @@ pub struct Route {
     pub format: Option<ContentType>,
 }
 
-fn default_rank(path: &str) -> isize {
-    // The rank for a given path is 0 if it is a static route (it doesn't
-    // contain any dynamic <segmants>) or 1 if it is dynamic.
-    path.contains('<') as isize
+#[inline(always)]
+fn default_rank(uri: &URI) -> isize {
+    // static path, query = -4; static path, no query = -3
+    // dynamic path, query = -2; dynamic path, no query = -1
+    match (!uri.path().contains('<'),  uri.query().is_some()) {
+        (true, true) => -4,
+        (true, false) => -3,
+        (false, true) => -2,
+        (false, false) => -1,
+    }
 }
 
 impl Route {
@@ -37,11 +43,12 @@ impl Route {
     pub fn new<S>(m: Method, path: S, handler: Handler) -> Route
         where S: AsRef<str>
     {
+        let uri = URI::from(path.as_ref().to_string());
         Route {
             method: m,
             handler: handler,
-            rank: default_rank(path.as_ref()),
-            path: URI::from(path.as_ref().to_string()),
+            rank: default_rank(&uri),
+            path: uri,
             format: None,
         }
     }