From 20605dac14e4f38a9b7f25ceee967eb11283061e Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 26 Mar 2021 19:41:00 -0700 Subject: [PATCH] Set default route rank using "colorings". This new system colors paths and queries in one of three ways: 1. `static`, meaning all components are static 2. `partial`, meaning at least one component is dynamic 3. `wild`, meaning all components are dynamic Static paths carry more weight than static queries. The same is true for partial and wild paths. This results in the following default rankings: | path | query | rank | |---------|---------|------| | static | static | -12 | | static | partial | -11 | | static | wild | -10 | | static | none | -9 | | partial | static | -8 | | partial | partial | -7 | | partial | wild | -6 | | partial | none | -5 | | wild | static | -4 | | wild | partial | -3 | | wild | wild | -2 | | wild | none | -1 | --- core/lib/src/router/collider.rs | 33 +++++---- core/lib/src/router/mod.rs | 17 ++++- core/lib/src/router/route.rs | 115 ++++++++++++++++++++++---------- core/lib/src/router/uri.rs | 93 ++++++++++++++++++-------- site/guide/4-requests.md | 41 ++++++++---- 5 files changed, 204 insertions(+), 95 deletions(-) diff --git a/core/lib/src/router/collider.rs b/core/lib/src/router/collider.rs index 233e1042..6cb9d2df 100644 --- a/core/lib/src/router/collider.rs +++ b/core/lib/src/router/collider.rs @@ -1,4 +1,4 @@ -use super::Route; +use super::{Route, uri::Color}; use crate::http::MediaType; use crate::request::Request; @@ -45,10 +45,6 @@ impl Route { } fn paths_collide(route: &Route, other: &Route) -> bool { - if route.uri.metadata.wild_path || other.uri.metadata.wild_path { - return true; - } - let a_segments = &route.uri.metadata.path_segs; let b_segments = &other.uri.metadata.path_segs; for (seg_a, seg_b) in a_segments.iter().zip(b_segments.iter()) { @@ -56,10 +52,12 @@ fn paths_collide(route: &Route, other: &Route) -> bool { return true; } - if !seg_a.dynamic && !seg_b.dynamic { - if seg_a.value != seg_b.value { - return false; - } + if seg_a.dynamic || seg_b.dynamic { + continue; + } + + if seg_a.value != seg_b.value { + return false; } } @@ -71,11 +69,19 @@ fn paths_collide(route: &Route, other: &Route) -> bool { fn paths_match(route: &Route, req: &Request<'_>) -> bool { let route_segments = &route.uri.metadata.path_segs; let req_segments = req.uri().path_segments(); - if route_segments.len() > req_segments.len() + 1 { + + if route.uri.metadata.trailing_path { + // The last route segment can be trailing, which is allowed to be empty. + // So we can have one more segment in `route` than in `req` and match. + // ok if: req_segments.len() >= route_segments.len() - 1 + if req_segments.len() + 1 < route_segments.len() { + return false; + } + } else if route_segments.len() != req_segments.len() { return false; } - if route.uri.metadata.wild_path { + if route.uri.metadata.path_color == Color::Wild { return true; } @@ -89,12 +95,11 @@ fn paths_match(route: &Route, req: &Request<'_>) -> bool { } } - route_segments.get(req_segments.len()).map_or(false, |s| s.trailing) - || route_segments.len() == req_segments.len() + true } fn queries_match(route: &Route, req: &Request<'_>) -> bool { - if route.uri.metadata.wild_query { + if matches!(route.uri.metadata.query_color, None | Some(Color::Wild)) { return true; } diff --git a/core/lib/src/router/mod.rs b/core/lib/src/router/mod.rs index a25fe35c..faa0945e 100644 --- a/core/lib/src/router/mod.rs +++ b/core/lib/src/router/mod.rs @@ -79,7 +79,7 @@ mod test { fn router_with_routes(routes: &[&'static str]) -> Router { let mut router = Router::new(); for route in routes { - let route = Route::new(Get, route, dummy); + let route = dbg!(Route::new(Get, route, dummy)); router.add(route); } @@ -176,6 +176,7 @@ mod test { assert!(rankless_route_collisions(&["//<_..>", "/<_>"])); assert!(rankless_route_collisions(&["////", "/a/"])); assert!(rankless_route_collisions(&["////", "/a/"])); + assert!(rankless_route_collisions(&["/", "/hello"])); } #[test] @@ -215,17 +216,26 @@ mod test { assert!(!default_rank_route_collisions(&["/", "/"])); assert!(!default_rank_route_collisions(&["/<_>/<_>", "/foo/bar"])); assert!(!default_rank_route_collisions(&["/foo/<_>", "/foo/bar"])); + + assert!(!default_rank_route_collisions(&["//", "/hello/"])); + assert!(!default_rank_route_collisions(&["//", "/hello/"])); + assert!(!default_rank_route_collisions(&["/", "/hello/"])); + assert!(!default_rank_route_collisions(&["/", "/hello"])); + assert!(!default_rank_route_collisions(&["/", "/a/"])); + assert!(!default_rank_route_collisions(&["/a//c", "//"])); } #[test] fn test_collision_when_ranked() { - assert!(default_rank_route_collisions(&["/", "/a/"])); + assert!(default_rank_route_collisions(&["/a//", "/a/"])); + assert!(default_rank_route_collisions(&["//b", "/a/"])); } #[test] fn test_collision_when_ranked_query() { assert!(default_rank_route_collisions(&["/a?a=b", "/a?c=d"])); - assert!(default_rank_route_collisions(&["/?a=b", "/?c=d&"])); + assert!(default_rank_route_collisions(&["/a?a=b&", "/a?&c=d"])); + assert!(default_rank_route_collisions(&["/a?a=b&", "/a?&c=d"])); } #[test] @@ -234,6 +244,7 @@ mod test { assert!(!default_rank_route_collisions(&["/hi", "/hi?"])); assert!(!default_rank_route_collisions(&["/hi", "/hi?c"])); assert!(!default_rank_route_collisions(&["/hi?", "/hi?c"])); + assert!(!default_rank_route_collisions(&["/?a=b", "/?c=d&"])); } fn route<'a>(router: &'a Router, method: Method, uri: &'a str) -> Option<&'a Route> { diff --git a/core/lib/src/router/route.rs b/core/lib/src/router/route.rs index 84853450..67f96768 100644 --- a/core/lib/src/router/route.rs +++ b/core/lib/src/router/route.rs @@ -32,53 +32,97 @@ impl Route { /// /// # Ranking /// - /// The route's rank is set so that routes with static paths (no dynamic - /// parameters) have lower ranks (higher precedence) than routes with - /// dynamic paths, routes with query strings with static segments have lower - /// ranks than routes with fully dynamic queries, and routes with queries - /// have lower ranks than routes without queries. This default ranking is - /// summarized by the table below: + /// The default rank prefers static components over dynamic components in + /// both paths and queries: the _more_ static a route's path and query are, + /// the higher its precedence. /// - /// | static path | query | rank | - /// |-------------|---------------|------| - /// | yes | partly static | -6 | - /// | yes | fully dynamic | -5 | - /// | yes | none | -4 | - /// | no | partly static | -3 | - /// | no | fully dynamic | -2 | - /// | no | none | -1 | + /// There are three "colors" to paths and queries: + /// 1. `static`, meaning all components are static + /// 2. `partial`, meaning at least one component is dynamic + /// 3. `wild`, meaning all components are dynamic + /// + /// Static paths carry more weight than static queries. The same is true for + /// partial and wild paths. This results in the following default ranking + /// table: + /// + /// | path | query | rank | + /// |---------|---------|------| + /// | static | static | -12 | + /// | static | partial | -11 | + /// | static | wild | -10 | + /// | static | none | -9 | + /// | partial | static | -8 | + /// | partial | partial | -7 | + /// | partial | wild | -6 | + /// | partial | none | -5 | + /// | wild | static | -4 | + /// | wild | partial | -3 | + /// | wild | wild | -2 | + /// | wild | none | -1 | + /// + /// Note that _lower_ ranks have _higher_ precedence. /// /// # Example /// /// ```rust /// use rocket::Route; /// use rocket::http::Method; - /// # use rocket::{Request, Data}; - /// # use rocket::handler::{dummy as handler, Outcome, HandlerFuture}; + /// # use rocket::handler::{dummy as handler}; /// - /// // this is rank -6 (static path, ~static query) - /// let route = Route::new(Method::Get, "/foo?bar=baz&", handler); - /// assert_eq!(route.rank, -6); + /// macro_rules! assert_rank { + /// ($($uri:expr => $rank:expr,)*) => {$( + /// let route = Route::new(Method::Get, $uri, handler); + /// assert_eq!(route.rank, $rank); + /// )*} + /// } /// - /// // this is rank -5 (static path, fully dynamic query) - /// let route = Route::new(Method::Get, "/foo?", handler); - /// assert_eq!(route.rank, -5); + /// assert_rank! { + /// "/?foo" => -12, // static path, static query + /// "/foo/bar?a=b&bob" => -12, // static path, static query + /// "/?a=b&bob" => -12, // static path, static query /// - /// // this is a rank -4 route (static path, no query) - /// let route = Route::new(Method::Get, "/", handler); - /// assert_eq!(route.rank, -4); + /// "/?a&" => -11, // static path, partial query + /// "/foo?a&" => -11, // static path, partial query + /// "/?a&" => -11, // static path, partial query /// - /// // this is a rank -3 route (dynamic path, ~static query) - /// let route = Route::new(Method::Get, "/foo/?blue", handler); - /// assert_eq!(route.rank, -3); + /// "/?" => -10, // static path, wild query + /// "/foo?" => -10, // static path, wild query + /// "/foo?&" => -10, // static path, wild query /// - /// // this is a rank -2 route (dynamic path, fully dynamic query) - /// let route = Route::new(Method::Get, "/?", handler); - /// assert_eq!(route.rank, -2); + /// "/" => -9, // static path, no query + /// "/foo/bar" => -9, // static path, no query /// - /// // this is a rank -1 route (dynamic path, no query) - /// let route = Route::new(Method::Get, "//foo/", handler); - /// assert_eq!(route.rank, -1); + /// "/a/?foo" => -8, // partial path, static query + /// "/a/?foo" => -8, // partial path, static query + /// "//b?foo" => -8, // partial path, static query + /// + /// "/a/?&c" => -7, // partial path, partial query + /// "/a/?a&" => -7, // partial path, partial query + /// + /// "/a/?" => -6, // partial path, wild query + /// "/a/?&" => -6, // partial path, wild query + /// "/a/?" => -6, // partial path, wild query + /// + /// "/a/" => -5, // partial path, no query + /// "//b" => -5, // partial path, no query + /// "/a/" => -5, // partial path, no query + /// + /// "//?foo&bar" => -4, // wild path, static query + /// "//?foo" => -4, // wild path, static query + /// "/?cat" => -4, // wild path, static query + /// + /// "//?&bar" => -3, // wild path, partial query + /// "//?a&" => -3, // wild path, partial query + /// "/?cat&" => -3, // wild path, partial query + /// + /// "//?" => -2, // wild path, wild query + /// "//?" => -2, // wild path, wild query + /// "/?&" => -2, // wild path, wild query + /// + /// "//" => -1, // wild path, no query + /// "//" => -1, // wild path, no query + /// "/" => -1, // wild path, no query + /// } /// ``` /// /// # Panics @@ -96,8 +140,7 @@ impl Route { /// ```rust /// use rocket::Route; /// use rocket::http::Method; - /// # use rocket::{Request, Data}; - /// # use rocket::handler::{dummy as handler, Outcome, HandlerFuture}; + /// # use rocket::handler::{dummy as handler}; /// /// // this is a rank 1 route matching requests to `GET /` /// let index = Route::ranked(1, Method::Get, "/", handler); diff --git a/core/lib/src/router/uri.rs b/core/lib/src/router/uri.rs index d4fb81bb..73ec5c04 100644 --- a/core/lib/src/router/uri.rs +++ b/core/lib/src/router/uri.rs @@ -67,7 +67,17 @@ pub struct RouteUri<'a> { pub(crate) metadata: Metadata, } -#[derive(Debug, Default, Clone)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Color { + /// Fully static: no dynamic components. + Static = 3, + /// Partially static/dynamic: some, but not all, dynamic components. + Partial = 2, + /// Fully dynamic: no static components. + Wild = 1, +} + +#[derive(Debug, Clone)] pub(crate) struct Metadata { /// Segments in the base. pub base_segs: Vec, @@ -77,14 +87,12 @@ pub(crate) struct Metadata { pub query_segs: Vec, /// `(name, value)` of the query segments that are static. pub static_query_fields: Vec<(String, String)>, - /// Whether the path is completely static. - pub static_path: bool, - /// Whether the path is completely dynamic. - pub wild_path: bool, + /// The "color" of the route path. + pub path_color: Color, + /// The "color" of the route query, if there is query. + pub query_color: Option, /// Whether the path has a `` parameter. pub trailing_path: bool, - /// Whether the query is completely dynamic. - pub wild_query: bool, } type Result = std::result::Result>; @@ -206,17 +214,28 @@ impl<'a> RouteUri<'a> { /// The route's default rank is determined based on the presence or absence /// of static and dynamic paths and queries. See the documentation for /// [`Route::new`][`crate::Route::new`] for a table summarizing the exact default ranks. + /// + /// | path | query | rank | + /// |---------|---------|------| + /// | static | static | -12 | + /// | static | partial | -11 | + /// | static | wild | -10 | + /// | static | none | -9 | + /// | partial | static | -8 | + /// | partial | partial | -7 | + /// | partial | wild | -6 | + /// | partial | none | -5 | + /// | wild | static | -4 | + /// | wild | partial | -3 | + /// | wild | wild | -2 | + /// | wild | none | -1 | pub(crate) fn default_rank(&self) -> isize { - let static_path = self.metadata.static_path; - let wild_query = self.query().map(|_| self.metadata.wild_query); - match (static_path, wild_query) { - (true, Some(false)) => -6, // static path, partly static query - (true, Some(true)) => -5, // static path, fully dynamic query - (true, None) => -4, // static path, no query - (false, Some(false)) => -3, // dynamic path, partly static query - (false, Some(true)) => -2, // dynamic path, fully dynamic query - (false, None) => -1, // dynamic path, no query - } + let raw_path_weight = self.metadata.path_color as u8; + let raw_query_weight = self.metadata.query_color.map_or(0, |c| c as u8); + let raw_weight = (raw_path_weight << 2) | raw_query_weight; + + // We subtract `3` because `raw_path` is never `0`: 0b0100 = 4 - 3 = 1. + -((raw_weight as isize) - 3) } } @@ -234,19 +253,34 @@ impl Metadata { .map(Segment::from) .collect::>(); + let static_query_fields = query_segs.iter().filter(|s| !s.dynamic) + .map(|s| ValueField::parse(&s.value)) + .map(|f| (f.name.source().to_string(), f.value.to_string())) + .collect(); + + let static_path = path_segs.iter().all(|s| !s.dynamic); + let wild_path = !path_segs.is_empty() && path_segs.iter().all(|s| s.dynamic); + let path_color = match (static_path, wild_path) { + (true, _) => Color::Static, + (_, true) => Color::Wild, + (_, _) => Color::Partial + }; + + let query_color = (!query_segs.is_empty()).then(|| { + let static_query = query_segs.iter().all(|s| !s.dynamic); + let wild_query = query_segs.iter().all(|s| s.dynamic); + match (static_query, wild_query) { + (true, _) => Color::Static, + (_, true) => Color::Wild, + (_, _) => Color::Partial + } + }); + + let trailing_path = path_segs.last().map_or(false, |p| p.trailing); + Metadata { - static_path: path_segs.iter().all(|s| !s.dynamic), - wild_path: path_segs.iter().all(|s| s.dynamic) - && path_segs.last().map_or(false, |p| p.trailing), - trailing_path: path_segs.last().map_or(false, |p| p.trailing), - wild_query: query_segs.iter().all(|s| s.dynamic), - static_query_fields: query_segs.iter().filter(|s| !s.dynamic) - .map(|s| ValueField::parse(&s.value)) - .map(|f| (f.name.source().to_string(), f.value.to_string())) - .collect(), - path_segs, - query_segs, - base_segs, + static_query_fields, path_color, query_color, trailing_path, + path_segs, query_segs, base_segs, } } } @@ -271,6 +305,7 @@ impl fmt::Debug for RouteUri<'_> { .field("base", &self.base) .field("unmounted_origin", &self.unmounted_origin) .field("origin", &self.origin) + .field("metadata", &self.metadata) .finish() } } diff --git a/site/guide/4-requests.md b/site/guide/4-requests.md index 718715b2..fc34045f 100644 --- a/site/guide/4-requests.md +++ b/site/guide/4-requests.md @@ -243,20 +243,35 @@ parameter resolves this collision. ### Default Ranking -If a rank is not explicitly specified, Rocket assigns a default ranking. By -default, routes with static paths and query strings have lower ranks (higher -precedence) while routes with dynamic paths and without query strings have -higher ranks (lower precedence). The table below describes the default ranking -of a route given its properties. +If a rank is not explicitly specified, Rocket assigns a default rank. The +default rank prefers static segments over dynamic segments in both paths and +queries: the _more_ static a route's path and query are, the higher its +precedence. -| static path | query | rank | example | -|-------------|---------------|------|---------------------| -| yes | partly static | -6 | `/hello?world=true` | -| yes | fully dynamic | -5 | `/hello/?` | -| yes | none | -4 | `/hello` | -| no | partly static | -3 | `/?world=true` | -| no | fully dynamic | -2 | `/?` | -| no | none | -1 | `/` | +There are three "colors" to paths and queries: + 1. `static`, meaning all components are static + 2. `partial`, meaning at least one component is dynamic + 3. `wild`, meaning all components are dynamic + +Static paths carry more weight than static queries. The same is true for partial +and wild paths. This results in the following default ranking table: + +| path color | query color | default rank | +|------------|-------------|--------------| +| static | static | -12 | +| static | partial | -11 | +| static | wild | -10 | +| static | none | -9 | +| partial | static | -8 | +| partial | partial | -7 | +| partial | wild | -6 | +| partial | none | -5 | +| wild | static | -4 | +| wild | partial | -3 | +| wild | wild | -2 | +| wild | none | -1 | + +Recall that _lower_ ranks have _higher_ precedence. ## Request Guards