diff --git a/benchmarks/src/routing.rs b/benchmarks/src/routing.rs index 48e7b165c1..3e49c80ec5 100644 --- a/benchmarks/src/routing.rs +++ b/benchmarks/src/routing.rs @@ -3,7 +3,7 @@ use std::collections::hash_set::HashSet; use criterion::{criterion_group, Criterion}; use rocket::{route, config, Request, Data, Route, Config}; -use rocket::http::{Method, RawStr, ContentType, Accept, Status}; +use rocket::http::{Method, RawStr, ContentType, Accept, Status, MediaType}; use rocket::local::blocking::{Client, LocalRequest}; fn dummy_handler<'r>(req: &'r Request, _: Data<'r>) -> route::BoxFuture<'r> { @@ -22,7 +22,7 @@ fn parse_routes_table(table: &str) -> Vec { match component { c if c.starts_with('[') => rank = c.trim_matches(&['[', ']'][..]).parse().ok(), c if c.starts_with('(') => name = Some(c.trim_matches(&['(', ')'][..])), - c => format = c.parse().ok(), + c => format = c.parse::().ok(), } } @@ -31,7 +31,7 @@ fn parse_routes_table(table: &str) -> Vec { route.rank = rank; } - route.format = format; + if let Some(format) = format { route.add_unique_prop(format); } route.name = name.map(|s| s.to_string().into()); routes.push(route); } @@ -61,7 +61,7 @@ fn generate_matching_requests<'c>(client: &'c Client, routes: &[Route]) -> Vec() { if let Some(true) = route.method.allows_request_body() { req.add_header(ContentType::from(format.clone())); } else { diff --git a/core/codegen/src/attribute/route/mod.rs b/core/codegen/src/attribute/route/mod.rs index b5fb59687b..5db917d502 100644 --- a/core/codegen/src/attribute/route/mod.rs +++ b/core/codegen/src/attribute/route/mod.rs @@ -342,7 +342,7 @@ fn codegen_route(route: Route) -> Result { let method = &route.attr.method; let uri = route.attr.uri.to_string(); let rank = Optional(route.attr.rank); - let format = Optional(route.attr.format.as_ref()); + let format = route.attr.format.as_ref().map(|f| quote! {Box::new(#f)}); Ok(quote! { #handler_fn @@ -375,9 +375,10 @@ fn codegen_route(route: Route) -> Result { method: #method, uri: #uri, handler: monomorphized_function, - format: #format, + // format: #format, rank: #rank, sentinels: #sentinels, + unique_properties: vec![#format] } } diff --git a/core/lib/fuzz/targets/collision-matching.rs b/core/lib/fuzz/targets/collision-matching.rs index 2af226ac02..a1f3d7f572 100644 --- a/core/lib/fuzz/targets/collision-matching.rs +++ b/core/lib/fuzz/targets/collision-matching.rs @@ -60,7 +60,8 @@ impl<'c, 'a: 'c> ArbitraryRequestData<'a> { impl<'a> ArbitraryRouteData<'a> { fn into_route(self) -> Route { let mut r = Route::ranked(0, self.method.0, &self.uri.0.to_string(), dummy_handler); - r.format = self.format.map(|f| f.0); + if let Some(f) = self.format { r.add_unique_prop(f.0); } + // r.format = self.format.map(|f| f.0); r } } diff --git a/core/lib/src/route/route.rs b/core/lib/src/route/route.rs index e78275b91b..db17c36e54 100644 --- a/core/lib/src/route/route.rs +++ b/core/lib/src/route/route.rs @@ -1,3 +1,4 @@ +use std::any::Any; use std::fmt; use std::borrow::Cow; @@ -5,6 +6,7 @@ use yansi::Paint; use crate::http::{uri, Method, MediaType}; use crate::route::{Handler, RouteUri, BoxFuture}; +use crate::router::{dyn_box_any, UniqueProperty}; use crate::sentinel::Sentry; /// A request handling route. @@ -27,7 +29,7 @@ use crate::sentinel::Sentry; /// assert_eq!(route.method, Method::Get); /// assert_eq!(route.uri, "/route/?query"); /// assert_eq!(route.rank, 2); -/// assert_eq!(route.format.unwrap(), MediaType::JSON); +/// // assert_eq!(route.format.unwrap(), MediaType::JSON); /// ``` /// /// Note that the `rank` and `format` attribute parameters are optional. See @@ -174,10 +176,12 @@ pub struct Route { pub uri: RouteUri<'static>, /// The rank of this route. Lower ranks have higher priorities. pub rank: isize, - /// The media type this route matches against, if any. - pub format: Option, + // /// The media type this route matches against, if any. + // pub format: Option, /// The discovered sentinels. pub(crate) sentinels: Vec, + /// The list of unique properties + pub(crate) unique_properties: Vec>, } impl Route { @@ -249,10 +253,11 @@ impl Route { let rank = rank.into().unwrap_or_else(|| uri.default_rank()); Route { name: None, - format: None, + // format: None, sentinels: Vec::new(), handler: Box::new(handler), rank, uri, method, + unique_properties: vec![], } } @@ -297,6 +302,22 @@ impl Route { self } + pub fn add_unique_prop(&mut self, prop: T) -> &mut Self { + // Panic if we already have a unique_property with this type + assert!(self.get_unique_prop::().is_none()); + self.unique_properties.push(Box::new(prop)); + self + } + + pub fn get_unique_prop(&self) -> Option<&T> { + for prop in &self.unique_properties { + if let Some(val) = dyn_box_any(prop).downcast_ref() { + return Some(val); + } + } + None + } + /// Maps the `base` of this route using `mapper`, returning a new `Route` /// with the returned base. /// @@ -356,8 +377,8 @@ impl fmt::Display for Route { write!(f, " [{}]", self.rank.primary().bold())?; } - if let Some(ref format) = self.format { - write!(f, " {}", format.yellow())?; + for prop in &self.unique_properties { + write!(f, " {}", prop.yellow())?; } Ok(()) @@ -371,7 +392,8 @@ impl fmt::Debug for Route { .field("method", &self.method) .field("uri", &self.uri) .field("rank", &self.rank) - .field("format", &self.format) + // .field("format", &self.format) + .field("properties", &self.unique_properties) .finish() } } @@ -385,12 +407,12 @@ pub struct StaticInfo { pub method: Method, /// The route's URi, without the base mount point. pub uri: &'static str, - /// The route's format, if any. - pub format: Option, /// The route's handler, i.e, the annotated function. pub handler: for<'r> fn(&'r crate::Request<'_>, crate::Data<'r>) -> BoxFuture<'r>, /// The route's rank, if any. pub rank: Option, + /// The list of unique properties to use when routing this route + pub unique_properties: Vec>, /// Route-derived sentinels, if any. /// This isn't `&'static [SentryInfo]` because `type_name()` isn't `const`. pub sentinels: Vec, @@ -407,9 +429,9 @@ impl From for Route { method: info.method, handler: Box::new(info.handler), rank: info.rank.unwrap_or_else(|| uri.default_rank()), - format: info.format, sentinels: info.sentinels.into_iter().collect(), uri, + unique_properties: info.unique_properties, } } } diff --git a/core/lib/src/router/collider.rs b/core/lib/src/router/collider.rs index f0978e4d56..131d45f520 100644 --- a/core/lib/src/router/collider.rs +++ b/core/lib/src/router/collider.rs @@ -3,6 +3,8 @@ use crate::route::{Route, Segment, RouteUri}; use crate::http::MediaType; +use super::unique_property; + pub trait Collide { fn collides_with(&self, other: &T) -> bool; } @@ -50,11 +52,11 @@ impl Route { /// assert!(a.collides_with(&b)); /// /// // Two routes with the same method, rank, URI, and overlapping formats. - /// let mut a = Route::new(Method::Post, "/", handler); - /// a.format = Some(MediaType::new("*", "custom")); - /// let mut b = Route::new(Method::Post, "/", handler); - /// b.format = Some(MediaType::new("text", "*")); - /// assert!(a.collides_with(&b)); + /// // let mut a = Route::new(Method::Post, "/", handler); + /// // a.format = Some(MediaType::new("*", "custom")); + /// // let mut b = Route::new(Method::Post, "/", handler); + /// // b.format = Some(MediaType::new("text", "*")); + /// // assert!(a.collides_with(&b)); /// /// // Two routes with different ranks don't collide. /// let a = Route::ranked(1, Method::Get, "/", handler); @@ -72,25 +74,26 @@ impl Route { /// assert!(!a.collides_with(&b)); /// /// // Two payload-supporting routes with non-overlapping formats. - /// let mut a = Route::new(Method::Post, "/", handler); - /// a.format = Some(MediaType::HTML); - /// let mut b = Route::new(Method::Post, "/", handler); - /// b.format = Some(MediaType::JSON); - /// assert!(!a.collides_with(&b)); + /// // let mut a = Route::new(Method::Post, "/", handler); + /// // a.format = Some(MediaType::HTML); + /// // let mut b = Route::new(Method::Post, "/", handler); + /// // b.format = Some(MediaType::JSON); + /// // assert!(!a.collides_with(&b)); /// /// // Two non payload-supporting routes with non-overlapping formats /// // collide. A request with `Accept: */*` matches both. - /// let mut a = Route::new(Method::Get, "/", handler); - /// a.format = Some(MediaType::HTML); - /// let mut b = Route::new(Method::Get, "/", handler); - /// b.format = Some(MediaType::JSON); - /// assert!(a.collides_with(&b)); + /// // let mut a = Route::new(Method::Get, "/", handler); + /// // a.format = Some(MediaType::HTML); + /// // let mut b = Route::new(Method::Get, "/", handler); + /// // b.format = Some(MediaType::JSON); + /// // assert!(a.collides_with(&b)); /// ``` pub fn collides_with(&self, other: &Route) -> bool { self.method == other.method && self.rank == other.rank && self.uri.collides_with(&other.uri) - && formats_collide(self, other) + // && formats_collide(self, other) + && unique_property::collides(&self, &other) } } @@ -190,23 +193,6 @@ impl Collide for MediaType { } } -fn formats_collide(route: &Route, other: &Route) -> bool { - match (route.method.allows_request_body(), other.method.allows_request_body()) { - // Payload supporting methods match against `Content-Type` which must be - // fully specified, so the request cannot contain a format that matches - // more than one route format as long as those formats don't collide. - (Some(true), Some(true)) => match (route.format.as_ref(), other.format.as_ref()) { - (Some(a), Some(b)) => a.collides_with(b), - // A route without a `format` accepts all `Content-Type`s. - _ => true - }, - // When a request method may not support a payload, the `Accept` header - // is considered during matching. The header can always be `*/*`, which - // would match any format. Thus two such routes would always collide. - _ => true, - } -} - #[cfg(test)] mod tests { use std::str::FromStr; @@ -420,12 +406,12 @@ mod tests { { let mut route_a = Route::new(m, "/", dummy_handler); if let Some(mt_str) = mt1.into() { - route_a.format = Some(mt_str.parse::().unwrap()); + route_a.add_unique_prop(mt_str.parse::().unwrap()); } let mut route_b = Route::new(m, "/", dummy_handler); if let Some(mt_str) = mt2.into() { - route_b.format = Some(mt_str.parse::().unwrap()); + route_b.add_unique_prop(mt_str.parse::().unwrap()); } route_a.collides_with(&route_b) @@ -473,6 +459,15 @@ mod tests { assert!(!r_mt_mt_collide(Post, "other/html", "text/html")); } + #[test] + fn check_prop_collider() { + // let a = "application/*".parse::().unwrap(); + // let b = "text/*".parse::().unwrap(); + // assert_eq!(a.collides_with(&b), true); + // assert_eq!(a.collides(&b), Some(true)); + // assert!(unique_property::collides(&vec![Box::new(a)], &vec![Box::new(b)])); + } + fn catchers_collide(a: A, ap: &str, b: B, bp: &str) -> bool where A: Into>, B: Into> { diff --git a/core/lib/src/router/matcher.rs b/core/lib/src/router/matcher.rs index 2d1ee14f1c..b677b66939 100644 --- a/core/lib/src/router/matcher.rs +++ b/core/lib/src/router/matcher.rs @@ -1,5 +1,4 @@ use crate::{Route, Request, Catcher}; -use crate::router::Collide; use crate::http::Status; use crate::route::Color; @@ -69,7 +68,8 @@ impl Route { self.method == request.method() && paths_match(self, request) && queries_match(self, request) - && formats_match(self, request) + // && formats_match(self, request) + && self.unique_properties.iter().all(|p| p.matches_request(request)) } } @@ -192,24 +192,24 @@ fn queries_match(route: &Route, req: &Request<'_>) -> bool { true } -fn formats_match(route: &Route, req: &Request<'_>) -> bool { - trace!("checking format match: route {} vs. request {}", route, req); - let route_format = match route.format { - Some(ref format) => format, - None => return true, - }; - - match route.method.allows_request_body() { - Some(true) => match req.format() { - Some(f) if f.specificity() == 2 => route_format.collides_with(f), - _ => false - }, - _ => match req.format() { - Some(f) => route_format.collides_with(f), - None => true - } - } -} +// fn formats_match(route: &Route, req: &Request<'_>) -> bool { +// trace!("checking format match: route {} vs. request {}", route, req); +// let route_format = match route.format { +// Some(ref format) => format, +// None => return true, +// }; + +// match route.method.allows_request_body() { +// Some(true) => match req.format() { +// Some(f) if f.specificity() == 2 => route_format.collides_with(f), +// _ => false +// }, +// _ => match req.format() { +// Some(f) => route_format.collides_with(f), +// None => true +// } +// } +// } #[cfg(test)] mod tests { @@ -295,7 +295,7 @@ mod tests { let mut route = Route::new(m, "/", dummy_handler); if let Some(mt_str) = mt2.into() { - route.format = Some(mt_str.parse::().unwrap()); + route.add_unique_prop(mt_str.parse::().unwrap()); } route.matches(&req) diff --git a/core/lib/src/router/mod.rs b/core/lib/src/router/mod.rs index c0bbccfb6b..8d21ad9675 100644 --- a/core/lib/src/router/mod.rs +++ b/core/lib/src/router/mod.rs @@ -3,6 +3,9 @@ mod router; mod collider; mod matcher; +mod unique_property; pub(crate) use router::*; pub(crate) use collider::*; +pub(crate) use unique_property::*; +pub use unique_property::WildcardHost; diff --git a/core/lib/src/router/unique_property.rs b/core/lib/src/router/unique_property.rs new file mode 100644 index 0000000000..dd227eabf4 --- /dev/null +++ b/core/lib/src/router/unique_property.rs @@ -0,0 +1,200 @@ +use std::any::Any; +use std::fmt; + +use super::Collide; + +use crate::{Request, Route}; +use crate::http::{uri::Host, MediaType}; +use figment::value::UncasedStr; +use sealed::Sealed; + +mod sealed { + pub trait Sealed {} +} + +pub trait AsAny: Any { + /// Converts this object to a `dyn Any` type. Used as a polyfill + /// for trait upcasting. + fn as_any(&self) -> &dyn Any; +} + +impl AsAny for T { + fn as_any(&self) -> &dyn Any { + self + } +} +impl Sealed for T {} + +pub trait DynClone { + fn clone_box(&self) -> Box; +} + +impl DynClone for T { + fn clone_box(&self) -> Box { + Box::new(self.clone()) + } +} + +impl Clone for Box { + fn clone(&self) -> Self { + self.clone_box() + } +} + +impl fmt::Debug for dyn UniqueProperty { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self) + } +} + +pub trait UniqueProperty: AsAny + DynClone + fmt::Display + Send + Sync { + /// Checks whether `other` collides with self. If `other` + /// does not check the same property, we should return None. + /// + /// This should be symmetrical, so `a.collides(b) == b.collides(a)` + /// + /// Two routes are considered colliding if there is not + /// at least one property that returns `Some(false)`. + fn collides(&self, self_route: &Route, other_route: &Route) -> Option; + + /// Checks whether a request matches this property. + fn matches_request(&self, req: &Request<'_>) -> bool; +} + +#[derive(Debug, Clone)] +pub enum WildcardHost { + Wildcard(Host<'static>), + Full(Host<'static>), +} + +impl WildcardHost { + pub fn parse(s: impl Into) -> Result> { + let s = s.into(); + if let Some(host) = s.strip_prefix("*.") { + Host::parse_owned(host.into()).map(Self::Wildcard) + } else { + Host::parse_owned(s).map(Self::Full) + } + } +} + +fn is_prefix(longer: &Host<'_>, shorter: &Host<'_>) -> bool { + let longer: &UncasedStr = longer.domain(); + let shorter: &UncasedStr = shorter.domain(); + // Hack for `ends_with`. This should be added to `uncased` + &longer[longer.len().saturating_sub(shorter.len())..] == shorter +} + +impl UniqueProperty for WildcardHost { + fn collides(&self, _self_route: &Route, other_route: &Route) -> Option { + other_route.get_unique_prop::().map(|other| match (self, other) { + (Self::Full(t), Self::Full(o)) => t == o, + (Self::Wildcard(t), Self::Wildcard(o)) + => !is_prefix(t, o) && !is_prefix(o, t), + (Self::Full(t), Self::Wildcard(o)) + => !is_prefix(t, o), + (Self::Wildcard(t), Self::Full(o)) + => !is_prefix(o, t), + }) + } + + fn matches_request<'r>(&self, req: &'r Request<'_>) -> bool { + match req.host() { + Some(host) => match self { + Self::Full(h) => h == host, + Self::Wildcard(h) => is_prefix(host, h), + }, + None => false, + } + } +} + +impl fmt::Display for WildcardHost { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Wildcard(h) => write!(f, "*.{}", h), + Self::Full(h) => write!(f, "{}", h), + } + } +} + +// impl UniqueProperty for Host<'static> { +// fn collides(&self, _self_route: &Route, other_route: &Route) -> Option { +// other_route.get_unique_prop::().map(|other| other == self) +// } + +// fn matches_request<'r>(&self, req: &'r Request<'_>) -> bool { +// match req.host() { +// Some(host) => self == host, +// None => false, +// } +// } +// } + +impl UniqueProperty for MediaType { + fn collides(&self, self_route: &Route, other_route: &Route) -> Option { + // TODO: do we NEED to check methods here? + match (self_route.method.allows_request_body(), other_route.method.allows_request_body()) { + (Some(true), Some(true)) => other_route + .get_unique_prop() + // TODO: move the collides_with impl to this module + .map(|other| self.collides_with(other)), + _ => None, // Does not differentiate routes + } + } + + fn matches_request<'r>(&self, req: &'r Request<'_>) -> bool { + match req.method().allows_request_body() { + Some(true) => match req.format() { + Some(f) if f.specificity() == 2 => self.collides_with(f), + _ => false + }, + _ => match req.format() { + Some(f) => self.collides_with(f), + None => true + } + } + } +} + +pub(crate) fn dyn_box_any(b: &Box) -> &dyn Any { + let b: &dyn UniqueProperty = b.as_ref(); + let any = b.as_any(); + // assert_eq!(b.type_id(), any.type_id()); + any +} + +/// A set of properties is unambiguous iff there is at least one property shared by both sets, with +/// a different value. +pub(crate) fn collides(a: &Route, b: &Route) -> bool { + for prop_a in &a.unique_properties { + if prop_a.collides(a, b) == Some(false) { + return false; + } + } + // Check inverse. This shouldn't be needed, but I've included it for completeness + // This check is only done once (during launch), so processing time doesn't really + // matter + for prop_b in &b.unique_properties { + if prop_b.collides(b, a) == Some(false) { + return false; + } + } + true +} + +#[cfg(test)] +mod tests { + use std::any::TypeId; + + use super::*; + use crate::http::uri::Host; + use crate::uri; + + #[test] + fn basic_api() { + let host= WildcardHost::Full(Host::new(uri!("my.example.com"))); + let v: &dyn UniqueProperty = &host; + assert_eq!(v.type_id(), TypeId::of::()) + } +}