From 713675b019b85b094a5059e14938fbac71416415 Mon Sep 17 00:00:00 2001 From: Matthew Pomes Date: Mon, 13 May 2024 21:33:14 -0500 Subject: [PATCH 1/5] Initial thoughts for unique-property --- core/lib/src/router/mod.rs | 1 + core/lib/src/router/unique_property.rs | 65 ++++++++++++++++++++++++++ tmp.md | 11 +++++ 3 files changed, 77 insertions(+) create mode 100644 core/lib/src/router/unique_property.rs create mode 100644 tmp.md diff --git a/core/lib/src/router/mod.rs b/core/lib/src/router/mod.rs index c0bbccfb6b..836f10f1c8 100644 --- a/core/lib/src/router/mod.rs +++ b/core/lib/src/router/mod.rs @@ -3,6 +3,7 @@ mod router; mod collider; mod matcher; +mod unique_property; pub(crate) use router::*; pub(crate) use collider::*; diff --git a/core/lib/src/router/unique_property.rs b/core/lib/src/router/unique_property.rs new file mode 100644 index 0000000000..741fee1d4f --- /dev/null +++ b/core/lib/src/router/unique_property.rs @@ -0,0 +1,65 @@ +use std::any::Any; + +use crate::{request::{FromRequest, Outcome}, Request}; + + + +pub trait UniqueProperty: Any { + fn as_any(&self) -> &dyn Any; + + fn matches(&self, other: &dyn Any) -> Option; + + // TODO: matches request. We want this so we can make routing decisions later, although we could choose to rely + // on the handler to do this for us. +} + +impl UniqueProperty for T { + fn as_any(&self) -> &dyn Any { + self + } + + fn matches(&self, other: &dyn Any) -> Option { + other.downcast_ref().map(|other: &Self| other == self) + } +} + +// pub struct UniqueInstance { +// val: Arc, +// matcher: Box Option>, +// } + +// impl UniqueInstance { +// pub fn new(val: T) -> Self { +// let arc = Arc::new(val); +// Self { +// val: arc.clone(), +// matcher: Box::new(move |v| v.downcast_ref().map(|v: &T| v == arc.as_ref())), +// } +// } + +// /// Returns true iff `self` and `val` hold values of the same type, and have different +// /// values. +// pub fn prevents_collision(&self, val: &Self) -> bool { +// (self.matcher)(val.val.as_ref()).map_or(false, |is_eq| !is_eq) +// } +// } + +// async fn matches_request<'a, 'r, T: UniqueProperty + Eq + 'a>(val: &dyn UniqueProperty, req: &'r Request<'_>) -> bool where &'a T: FromRequest<'r> { +// match <&T as FromRequest>::from_request(req).await { +// Outcome::Success(v) => val.matches(&v).map_or(false, |v| v), +// _ => false, +// } +// } + + +#[cfg(test)] +mod tests { + use super::*; + use crate::http::uri::Host; + use crate::uri; + + #[test] + fn basic_api() { + // let instance = UniqueInstance::new(Host::new(uri!("my.example.com"))); + } +} diff --git a/tmp.md b/tmp.md new file mode 100644 index 0000000000..e1cb3aead7 --- /dev/null +++ b/tmp.md @@ -0,0 +1,11 @@ +# Pluggable routing + +A 'unique property' is a type that implements `FromRequest` and `Eq`. + +A route may have any number of 'unique properties', and routes A and B +are considered colliding if they have colliding paths, and they do not +share a 'unique property' type with different values. + +E.g.: `Format(MediaType)`. + + From a6fc1c71dd7a25fc855b53b9a429e8affcbedd92 Mon Sep 17 00:00:00 2001 From: Matthew Pomes Date: Tue, 14 May 2024 09:49:00 -0500 Subject: [PATCH 2/5] Fix implementation for tests --- core/lib/src/route/route.rs | 41 +++++-- core/lib/src/router/collider.rs | 65 +++++------ core/lib/src/router/matcher.rs | 42 +++---- core/lib/src/router/mod.rs | 1 + core/lib/src/router/unique_property.rs | 153 +++++++++++++++++++------ 5 files changed, 200 insertions(+), 102 deletions(-) diff --git a/core/lib/src/route/route.rs b/core/lib/src/route/route.rs index e78275b91b..613f0e63b1 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(crate) 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(crate) 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() } } @@ -407,9 +429,12 @@ impl From for Route { method: info.method, handler: Box::new(info.handler), rank: info.rank.unwrap_or_else(|| uri.default_rank()), - format: info.format, + // format: info.format.clone(), sentinels: info.sentinels.into_iter().collect(), uri, + unique_properties: [ + info.format.map(|f| Box::new(f) as Box) + ].into_iter().filter_map(|v| v).collect(), } } } 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 836f10f1c8..ddead3afc2 100644 --- a/core/lib/src/router/mod.rs +++ b/core/lib/src/router/mod.rs @@ -7,3 +7,4 @@ mod unique_property; pub(crate) use router::*; pub(crate) use collider::*; +pub(crate) use unique_property::*; diff --git a/core/lib/src/router/unique_property.rs b/core/lib/src/router/unique_property.rs index 741fee1d4f..e0afdb374a 100644 --- a/core/lib/src/router/unique_property.rs +++ b/core/lib/src/router/unique_property.rs @@ -1,65 +1,142 @@ use std::any::Any; +use std::fmt::{self, Display}; -use crate::{request::{FromRequest, Outcome}, Request}; +use super::Collide; +use crate::{Request, Route}; +use crate::http::{uri::Host, MediaType}; +use sealed::Sealed; +mod sealed { + pub trait Sealed {} +} -pub trait UniqueProperty: Any { +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; - - fn matches(&self, other: &dyn Any) -> Option; - - // TODO: matches request. We want this so we can make routing decisions later, although we could choose to rely - // on the handler to do this for us. } -impl UniqueProperty for T { +impl AsAny for T { fn as_any(&self) -> &dyn Any { self } +} + +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 + Display + Sealed + 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; +} + +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 Sealed for Host<'static> {} + +impl UniqueProperty for MediaType { + fn collides(&self, self_route: &Route, other_route: &Route) -> Option { + match (self_route.method.allows_request_body(), other_route.method.allows_request_body()) { + (Some(true), Some(true)) => other_route.get_unique_prop().map(|other| self.collides_with(other)), + _ => Some(true), + } + } - fn matches(&self, other: &dyn Any) -> Option { - other.downcast_ref().map(|other: &Self| other == self) + 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 + } + } } } +impl Sealed for MediaType {} -// pub struct UniqueInstance { -// val: Arc, -// matcher: Box Option>, -// } - -// impl UniqueInstance { -// pub fn new(val: T) -> Self { -// let arc = Arc::new(val); -// Self { -// val: arc.clone(), -// matcher: Box::new(move |v| v.downcast_ref().map(|v: &T| v == arc.as_ref())), -// } -// } - -// /// Returns true iff `self` and `val` hold values of the same type, and have different -// /// values. -// pub fn prevents_collision(&self, val: &Self) -> bool { -// (self.matcher)(val.val.as_ref()).map_or(false, |is_eq| !is_eq) -// } -// } - -// async fn matches_request<'a, 'r, T: UniqueProperty + Eq + 'a>(val: &dyn UniqueProperty, req: &'r Request<'_>) -> bool where &'a T: FromRequest<'r> { -// match <&T as FromRequest>::from_request(req).await { -// Outcome::Success(v) => val.matches(&v).map_or(false, |v| v), -// _ => false, -// } -// } +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 + // as AsRef>::as_ref(b).as_any() + // todo!() +} +/// 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 { + for prop_b in &b.unique_properties { + // Check that they have the same type_id, which prevents checking other properties (and potentially + // avoids the need to check the reverse) + dbg!((prop_a.type_id(), prop_b.type_id(), prop_a.type_id() == prop_b.type_id())); + dbg!(prop_a.collides(a, b)); + // `prop_b.as_ref()` is needed to ensure we do not convert `Box` into `&dyn Any`, but rather + // get the inner type + // assert_eq!(std::any::TypeId::of::(), dyn_box_any(prop_b).type_id()); + if dyn_box_any(prop_a).type_id() == dyn_box_any(prop_b).type_id() && prop_a.collides(a, b) == 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 instance = UniqueInstance::new(Host::new(uri!("my.example.com"))); + let host= Host::new(uri!("my.example.com")); + let v: &dyn UniqueProperty = &host; + assert_eq!(v.type_id(), TypeId::of::>()) } } From 0d9a5ec694d13e210c43efabd237e4be3cff67c8 Mon Sep 17 00:00:00 2001 From: Matthew Pomes Date: Tue, 14 May 2024 09:49:56 -0500 Subject: [PATCH 3/5] Cleanup --- tmp.md | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 tmp.md diff --git a/tmp.md b/tmp.md deleted file mode 100644 index e1cb3aead7..0000000000 --- a/tmp.md +++ /dev/null @@ -1,11 +0,0 @@ -# Pluggable routing - -A 'unique property' is a type that implements `FromRequest` and `Eq`. - -A route may have any number of 'unique properties', and routes A and B -are considered colliding if they have colliding paths, and they do not -share a 'unique property' type with different values. - -E.g.: `Format(MediaType)`. - - From 216f7943ce309cfdb71e992f2f8d6b29d69e3f50 Mon Sep 17 00:00:00 2001 From: Matthew Pomes Date: Fri, 17 May 2024 11:40:23 -0500 Subject: [PATCH 4/5] Clean up implementation, and prep for draft PR --- core/lib/src/router/unique_property.rs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/core/lib/src/router/unique_property.rs b/core/lib/src/router/unique_property.rs index e0afdb374a..19a3df1796 100644 --- a/core/lib/src/router/unique_property.rs +++ b/core/lib/src/router/unique_property.rs @@ -76,8 +76,10 @@ impl Sealed for Host<'static> {} impl UniqueProperty for MediaType { fn collides(&self, self_route: &Route, other_route: &Route) -> Option { match (self_route.method.allows_request_body(), other_route.method.allows_request_body()) { - (Some(true), Some(true)) => other_route.get_unique_prop().map(|other| self.collides_with(other)), - _ => Some(true), + (Some(true), Some(true)) => other_route + .get_unique_prop() + .map(|other| self.collides_with(other)), + _ => None, // Does not differentiate routes } } @@ -101,25 +103,15 @@ pub(crate) fn dyn_box_any(b: &Box) -> &dyn Any { let any = b.as_any(); assert_eq!(b.type_id(), any.type_id()); any - // as AsRef>::as_ref(b).as_any() - // todo!() } /// 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 { - for prop_b in &b.unique_properties { - // Check that they have the same type_id, which prevents checking other properties (and potentially - // avoids the need to check the reverse) - dbg!((prop_a.type_id(), prop_b.type_id(), prop_a.type_id() == prop_b.type_id())); - dbg!(prop_a.collides(a, b)); - // `prop_b.as_ref()` is needed to ensure we do not convert `Box` into `&dyn Any`, but rather - // get the inner type - // assert_eq!(std::any::TypeId::of::(), dyn_box_any(prop_b).type_id()); - if dyn_box_any(prop_a).type_id() == dyn_box_any(prop_b).type_id() && prop_a.collides(a, b) == Some(false) { - return false; - } + // TODO: we should consider checking the inverse, i.e., does b collide with a + if prop_a.collides(a, b) == Some(false) { + return false; } } true From 0b03c232f38291676647db5ff317396be4522cf4 Mon Sep 17 00:00:00 2001 From: Matthew Pomes Date: Sun, 16 Jun 2024 18:29:08 -0500 Subject: [PATCH 5/5] Update static info to use `unique_properties` Also updates codegen, router, and tests to use the new API --- benchmarks/src/routing.rs | 8 +- core/codegen/src/attribute/route/mod.rs | 5 +- core/lib/fuzz/targets/collision-matching.rs | 3 +- core/lib/src/route/route.rs | 13 ++- core/lib/src/router/mod.rs | 1 + core/lib/src/router/unique_property.rs | 90 ++++++++++++++++++--- 6 files changed, 93 insertions(+), 27 deletions(-) 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 613f0e63b1..db17c36e54 100644 --- a/core/lib/src/route/route.rs +++ b/core/lib/src/route/route.rs @@ -302,14 +302,14 @@ impl Route { self } - pub(crate) fn add_unique_prop(&mut self, prop: T) -> &mut 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(crate) fn get_unique_prop(&self) -> Option<&T> { + 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); @@ -407,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, @@ -429,12 +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.clone(), sentinels: info.sentinels.into_iter().collect(), uri, - unique_properties: [ - info.format.map(|f| Box::new(f) as Box) - ].into_iter().filter_map(|v| v).collect(), + unique_properties: info.unique_properties, } } } diff --git a/core/lib/src/router/mod.rs b/core/lib/src/router/mod.rs index ddead3afc2..8d21ad9675 100644 --- a/core/lib/src/router/mod.rs +++ b/core/lib/src/router/mod.rs @@ -8,3 +8,4 @@ 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 index 19a3df1796..dd227eabf4 100644 --- a/core/lib/src/router/unique_property.rs +++ b/core/lib/src/router/unique_property.rs @@ -1,10 +1,11 @@ use std::any::Any; -use std::fmt::{self, Display}; +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 { @@ -17,11 +18,12 @@ pub trait AsAny: Any { fn as_any(&self) -> &dyn Any; } -impl AsAny for T { +impl AsAny for T { fn as_any(&self) -> &dyn Any { self } } +impl Sealed for T {} pub trait DynClone { fn clone_box(&self) -> Box; @@ -45,7 +47,7 @@ impl fmt::Debug for dyn UniqueProperty { } } -pub trait UniqueProperty: AsAny + DynClone + Display + Sealed + Send + Sync { +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. /// @@ -59,25 +61,83 @@ pub trait UniqueProperty: AsAny + DynClone + Display + Sealed + Send + Sync { fn matches_request(&self, req: &Request<'_>) -> bool; } -impl UniqueProperty for Host<'static> { +#[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| other == self) + 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) => self == host, + Some(host) => match self { + Self::Full(h) => h == host, + Self::Wildcard(h) => is_prefix(host, h), + }, None => false, } } } -impl Sealed for Host<'static> {} + +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 } @@ -96,12 +156,11 @@ impl UniqueProperty for MediaType { } } } -impl Sealed for MediaType {} 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()); + // assert_eq!(b.type_id(), any.type_id()); any } @@ -109,11 +168,18 @@ pub(crate) fn dyn_box_any(b: &Box) -> &dyn Any { /// a different value. pub(crate) fn collides(a: &Route, b: &Route) -> bool { for prop_a in &a.unique_properties { - // TODO: we should consider checking the inverse, i.e., does b collide with a 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 } @@ -127,8 +193,8 @@ mod tests { #[test] fn basic_api() { - let host= Host::new(uri!("my.example.com")); + let host= WildcardHost::Full(Host::new(uri!("my.example.com"))); let v: &dyn UniqueProperty = &host; - assert_eq!(v.type_id(), TypeId::of::>()) + assert_eq!(v.type_id(), TypeId::of::()) } }