Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pluggable routing #2792

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions benchmarks/src/routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -22,7 +22,7 @@ fn parse_routes_table(table: &str) -> Vec<Route> {
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::<MediaType>().ok(),
}
}

Expand All @@ -31,7 +31,7 @@ fn parse_routes_table(table: &str) -> Vec<Route> {
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);
}
Expand Down Expand Up @@ -61,7 +61,7 @@ fn generate_matching_requests<'c>(client: &'c Client, routes: &[Route]) -> Vec<L

let uri = format!("/{}?{}", path, query);
let mut req = client.req(route.method, uri);
if let Some(ref format) = route.format {
if let Some(format) = route.get_unique_prop::<MediaType>() {
if let Some(true) = route.method.allows_request_body() {
req.add_header(ContentType::from(format.clone()));
} else {
Expand Down
5 changes: 3 additions & 2 deletions core/codegen/src/attribute/route/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ fn codegen_route(route: Route) -> Result<TokenStream> {
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
Expand Down Expand Up @@ -375,9 +375,10 @@ fn codegen_route(route: Route) -> Result<TokenStream> {
method: #method,
uri: #uri,
handler: monomorphized_function,
format: #format,
// format: #format,
rank: #rank,
sentinels: #sentinels,
unique_properties: vec![#format]
}
}

Expand Down
3 changes: 2 additions & 1 deletion core/lib/fuzz/targets/collision-matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
42 changes: 32 additions & 10 deletions core/lib/src/route/route.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use std::any::Any;
use std::fmt;
use std::borrow::Cow;

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.
Expand All @@ -27,7 +29,7 @@ use crate::sentinel::Sentry;
/// assert_eq!(route.method, Method::Get);
/// assert_eq!(route.uri, "/route/<path..>?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
Expand Down Expand Up @@ -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<MediaType>,
// /// The media type this route matches against, if any.
// pub format: Option<MediaType>,
/// The discovered sentinels.
pub(crate) sentinels: Vec<Sentry>,
/// The list of unique properties
pub(crate) unique_properties: Vec<Box<dyn UniqueProperty>>,
}

impl Route {
Expand Down Expand Up @@ -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![],
}
}

Expand Down Expand Up @@ -297,6 +302,22 @@ impl Route {
self
}

pub fn add_unique_prop<T: UniqueProperty>(&mut self, prop: T) -> &mut Self {
// Panic if we already have a unique_property with this type
assert!(self.get_unique_prop::<T>().is_none());
self.unique_properties.push(Box::new(prop));
self
}

pub fn get_unique_prop<T: Any>(&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.
///
Expand Down Expand Up @@ -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(())
Expand All @@ -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()
}
}
Expand All @@ -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<MediaType>,
/// 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<isize>,
/// The list of unique properties to use when routing this route
pub unique_properties: Vec<Box<dyn UniqueProperty>>,
/// Route-derived sentinels, if any.
/// This isn't `&'static [SentryInfo]` because `type_name()` isn't `const`.
pub sentinels: Vec<Sentry>,
Expand All @@ -407,9 +429,9 @@ impl From<StaticInfo> 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,
}
}
}
65 changes: 30 additions & 35 deletions core/lib/src/router/collider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use crate::route::{Route, Segment, RouteUri};

use crate::http::MediaType;

use super::unique_property;

pub trait Collide<T = Self> {
fn collides_with(&self, other: &T) -> bool;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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::<MediaType>().unwrap());
route_a.add_unique_prop(mt_str.parse::<MediaType>().unwrap());
}

let mut route_b = Route::new(m, "/", dummy_handler);
if let Some(mt_str) = mt2.into() {
route_b.format = Some(mt_str.parse::<MediaType>().unwrap());
route_b.add_unique_prop(mt_str.parse::<MediaType>().unwrap());
}

route_a.collides_with(&route_b)
Expand Down Expand Up @@ -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::<MediaType>().unwrap();
// let b = "text/*".parse::<MediaType>().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, B>(a: A, ap: &str, b: B, bp: &str) -> bool
where A: Into<Option<u16>>, B: Into<Option<u16>>
{
Expand Down
42 changes: 21 additions & 21 deletions core/lib/src/router/matcher.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::{Route, Request, Catcher};
use crate::router::Collide;
use crate::http::Status;
use crate::route::Color;

Expand Down Expand Up @@ -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))
}
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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::<MediaType>().unwrap());
route.add_unique_prop(mt_str.parse::<MediaType>().unwrap());
}

route.matches(&req)
Expand Down
3 changes: 3 additions & 0 deletions core/lib/src/router/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Loading