Skip to content

Commit

Permalink
Support routes that match any method.
Browse files Browse the repository at this point in the history
This commit introduces support for method-less routes and route
attributes, which match _any_ valid method: `#[route("/")]`. The `Route`
structure's `method` parameter is now accordingly `Option<Route>`.

The syntax for the `route` attribute has changed in a breaking manner.
To set a method, a key/value of `method = NAME` must be introduced:

```rust
```

If the method's name is a valid identifier, it can be used without
quotes. Otherwise it must be quoted:

```rust
// `PATCH` is a valid identifier

// `VERSION-CONTROL` is not
```
  • Loading branch information
SergioBenitez committed Aug 24, 2024
1 parent 9496b70 commit aa12827
Show file tree
Hide file tree
Showing 19 changed files with 296 additions and 204 deletions.
4 changes: 2 additions & 2 deletions benchmarks/src/routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ fn generate_matching_requests<'c>(client: &'c Client, routes: &[Route]) -> Vec<L
.join("&");

let uri = format!("/{}?{}", path, query);
let mut req = client.req(route.method, uri);
let mut req = client.req(route.method.unwrap(), uri);
if let Some(ref format) = route.format {
if let Some(true) = route.method.allows_request_body() {
if let Some(true) = route.method.and_then(|m| m.allows_request_body()) {
req.add_header(ContentType::from(format.clone()));
} else {
req.add_header(Accept::from(format.clone()));
Expand Down
13 changes: 8 additions & 5 deletions core/codegen/src/attribute/route/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ fn internal_uri_macro_decl(route: &Route) -> TokenStream {
// Generate a unique macro name based on the route's metadata.
let macro_name = route.handler.sig.ident.prepend(crate::URI_MACRO_PREFIX);
let inner_macro_name = macro_name.uniqueify_with(|mut hasher| {
route.attr.method.0.hash(&mut hasher);
route.attr.method.as_ref().map(|m| m.0.hash(&mut hasher));
route.attr.uri.path().hash(&mut hasher);
route.attr.uri.query().hash(&mut hasher);
route.attr.data.as_ref().map(|d| d.value.hash(&mut hasher));
Expand Down Expand Up @@ -395,7 +395,7 @@ fn codegen_route(route: Route) -> Result<TokenStream> {
let internal_uri_macro = internal_uri_macro_decl(&route);
let responder_outcome = responder_outcome_expr(&route);

let method = &route.attr.method;
let method = Optional(route.attr.method.clone());
let uri = route.attr.uri.to_string();
let rank = Optional(route.attr.rank);
let format = Optional(route.attr.format.as_ref());
Expand Down Expand Up @@ -480,9 +480,12 @@ fn incomplete_route(
let method_attribute = MethodAttribute::from_meta(&syn::parse2(full_attr)?)?;

let attribute = Attribute {
method: SpanWrapped {
full_span: method_span, key_span: None, span: method_span, value: Method(method)
},
method: Some(SpanWrapped {
full_span: method_span,
key_span: None,
span: method_span,
value: Method(method),
}),
uri: method_attribute.uri,
data: method_attribute.data,
format: method_attribute.format,
Expand Down
24 changes: 15 additions & 9 deletions core/codegen/src/attribute/route/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ pub struct Arguments {
#[derive(Debug, FromMeta)]
pub struct Attribute {
#[meta(naked)]
pub method: SpanWrapped<Method>,
pub uri: RouteUri,
pub method: Option<SpanWrapped<Method>>,
pub data: Option<SpanWrapped<Dynamic>>,
pub format: Option<MediaType>,
pub rank: Option<isize>,
Expand Down Expand Up @@ -129,17 +129,23 @@ impl Route {
// Emit a warning if a `data` param was supplied for non-payload methods.
if let Some(ref data) = attr.data {
let lint = Lint::DubiousPayload;
match attr.method.0.allows_request_body() {
None if lint.enabled(handler.span()) => {
match attr.method.as_ref() {
Some(m) if m.0.allows_request_body() == Some(false) => {
diags.push(data.full_span
.error("`data` cannot be used on this route")
.span_note(m.span, "method does not support request payloads"))
},
Some(m) if m.0.allows_request_body().is_none() && lint.enabled(handler.span()) => {
data.full_span.warning("`data` used with non-payload-supporting method")
.note(format!("'{}' does not typically support payloads", attr.method.0))
.span_note(m.span, format!("'{}' does not typically support payloads", m.0))
.note(lint.how_to_suppress())
.emit_as_item_tokens();
},
None if lint.enabled(handler.span()) => {
data.full_span.warning("`data` used on route with wildcard method")
.note("some methods may not support request payloads")
.note(lint.how_to_suppress())
.emit_as_item_tokens();
}
Some(false) => {
diags.push(data.full_span
.error("`data` cannot be used on this route")
.span_note(attr.method.span, "method does not support request payloads"))
}
_ => { /* okay */ },
}
Expand Down
25 changes: 16 additions & 9 deletions core/codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,20 @@ macro_rules! route_attribute {
/// * [`patch`] - `PATCH` specific route
///
/// Additionally, [`route`] allows the method and uri to be explicitly
/// specified:
/// specified, and for the method to be omitted entirely, to match any
/// method:
///
/// ```rust
/// # #[macro_use] extern crate rocket;
/// #
/// #[route(GET, uri = "/")]
/// fn index() -> &'static str {
/// "Hello, world!"
/// }
///
/// #[route("/", method = GET)]
/// fn get_index() { /* ... */ }
///
/// #[route("/", method = "VERSION-CONTROL")]
/// fn versioned_index() { /* ... */ }
///
/// #[route("/")]
/// fn index() { /* ... */ }
/// ```
///
/// [`get`]: attr.get.html
Expand Down Expand Up @@ -171,7 +176,9 @@ macro_rules! route_attribute {
/// The generic route attribute is defined as:
///
/// ```text
/// generic-route := METHOD ',' 'uri' '=' route
/// generic-route := route (',' method)?
///
/// method := 'method' '=' METHOD
/// ```
///
/// # Typing Requirements
Expand Down Expand Up @@ -1161,12 +1168,12 @@ pub fn derive_uri_display_path(input: TokenStream) -> TokenStream {
/// assert_eq!(my_routes.len(), 2);
///
/// let index_route = &my_routes[0];
/// assert_eq!(index_route.method, Method::Get);
/// assert_eq!(index_route.method, Some(Method::Get));
/// assert_eq!(index_route.name.as_ref().unwrap(), "index");
/// assert_eq!(index_route.uri.path(), "/");
///
/// let hello_route = &my_routes[1];
/// assert_eq!(hello_route.method, Method::Post);
/// assert_eq!(hello_route.method, Some(Method::Post));
/// assert_eq!(hello_route.name.as_ref().unwrap(), "hello");
/// assert_eq!(hello_route.uri.path(), "/hi/<person>");
/// ```
Expand Down
4 changes: 2 additions & 2 deletions core/codegen/tests/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ fn post1(
}

#[route(
POST,
uri = "/<a>/<name>/name/<path..>?sky=blue&<sky>&<query..>",
"/<a>/<name>/name/<path..>?sky=blue&<sky>&<query..>",
method = POST,
format = "json",
data = "<simple>",
rank = 138
Expand Down
4 changes: 4 additions & 0 deletions core/http/src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ macro_rules! define_methods {
#[doc(hidden)]
pub const ALL: &'static [&'static str] = &[$($name),*];

/// A slice containing every defined method variant.
#[doc(hidden)]
pub const ALL_VARIANTS: &'static [Method] = &[$(Self::$V),*];

/// Whether the method is considered "safe".
///
/// From [RFC9110 §9.2.1](https://www.rfc-editor.org/rfc/rfc9110#section-9.2.1):
Expand Down
15 changes: 6 additions & 9 deletions core/lib/fuzz/targets/collision-matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ struct ArbitraryRequestData<'a> {

#[derive(Arbitrary)]
struct ArbitraryRouteData<'a> {
method: ArbitraryMethod,
method: Option<ArbitraryMethod>,
uri: ArbitraryRouteUri<'a>,
format: Option<ArbitraryMediaType>,
}

impl std::fmt::Debug for ArbitraryRouteData<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("ArbitraryRouteData")
.field("method", &self.method.0)
.field("method", &self.method.map(|v| v.0))
.field("base", &self.uri.0.base())
.field("unmounted", &self.uri.0.unmounted().to_string())
.field("uri", &self.uri.0.to_string())
Expand Down Expand Up @@ -59,12 +59,14 @@ 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);
let method = self.method.map(|m| m.0);
let mut r = Route::ranked(0, method, &self.uri.0.to_string(), dummy_handler);
r.format = self.format.map(|f| f.0);
r
}
}

#[derive(Clone, Copy)]
struct ArbitraryMethod(Method);

struct ArbitraryOrigin<'a>(Origin<'a>);
Expand All @@ -79,12 +81,7 @@ struct ArbitraryRouteUri<'a>(RouteUri<'a>);

impl<'a> Arbitrary<'a> for ArbitraryMethod {
fn arbitrary(u: &mut Unstructured<'a>) -> Result<Self> {
let all_methods = &[
Method::Get, Method::Put, Method::Post, Method::Delete, Method::Options,
Method::Head, Method::Trace, Method::Connect, Method::Patch
];

Ok(ArbitraryMethod(*u.choose(all_methods)?))
Ok(ArbitraryMethod(*u.choose(Method::ALL_VARIANTS)?))
}

fn size_hint(_: usize) -> (usize, Option<usize>) {
Expand Down
6 changes: 3 additions & 3 deletions core/lib/src/phase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use figment::Figment;
use crate::listener::Endpoint;
use crate::shutdown::Stages;
use crate::{Catcher, Config, Rocket, Route};
use crate::router::Router;
use crate::router::{Router, Finalized};
use crate::fairing::Fairings;

mod private {
Expand Down Expand Up @@ -100,7 +100,7 @@ phases! {
/// represents a fully built and finalized application server ready for
/// launch into orbit. See [`Rocket#ignite`] for full details.
Ignite (#[derive(Debug)] Igniting) {
pub(crate) router: Router,
pub(crate) router: Router<Finalized>,
pub(crate) fairings: Fairings,
pub(crate) figment: Figment,
pub(crate) config: Config,
Expand All @@ -114,7 +114,7 @@ phases! {
/// An instance of `Rocket` in this phase is typed as [`Rocket<Orbit>`] and
/// represents a running application.
Orbit (#[derive(Debug)] Orbiting) {
pub(crate) router: Router,
pub(crate) router: Router<Finalized>,
pub(crate) fairings: Fairings,
pub(crate) figment: Figment,
pub(crate) config: Config,
Expand Down
15 changes: 8 additions & 7 deletions core/lib/src/rocket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,9 +557,10 @@ impl Rocket<Build> {

// Initialize the router; check for collisions.
let mut router = Router::new();
self.routes.clone().into_iter().for_each(|r| router.add_route(r));
self.catchers.clone().into_iter().for_each(|c| router.add_catcher(c));
router.finalize().map_err(|(r, c)| ErrorKind::Collisions { routes: r, catchers: c, })?;
self.routes.clone().into_iter().for_each(|r| router.routes.push(r));
self.catchers.clone().into_iter().for_each(|c| router.catchers.push(c));
let router = router.finalize()
.map_err(|(r, c)| ErrorKind::Collisions { routes: r, catchers: c, })?;

// Finally, freeze managed state for faster access later.
self.state.freeze();
Expand Down Expand Up @@ -840,8 +841,8 @@ impl<P: Phase> Rocket<P> {
pub fn routes(&self) -> impl Iterator<Item = &Route> {
match self.0.as_ref() {
StateRef::Build(p) => Either::Left(p.routes.iter()),
StateRef::Ignite(p) => Either::Right(p.router.routes()),
StateRef::Orbit(p) => Either::Right(p.router.routes()),
StateRef::Ignite(p) => Either::Right(p.router.routes.iter()),
StateRef::Orbit(p) => Either::Right(p.router.routes.iter()),
}
}

Expand Down Expand Up @@ -871,8 +872,8 @@ impl<P: Phase> Rocket<P> {
pub fn catchers(&self) -> impl Iterator<Item = &Catcher> {
match self.0.as_ref() {
StateRef::Build(p) => Either::Left(p.catchers.iter()),
StateRef::Ignite(p) => Either::Right(p.router.catchers()),
StateRef::Orbit(p) => Either::Right(p.router.catchers()),
StateRef::Ignite(p) => Either::Right(p.router.catchers.iter()),
StateRef::Orbit(p) => Either::Right(p.router.catchers.iter()),
}
}

Expand Down
28 changes: 16 additions & 12 deletions core/lib/src/route/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::sentinel::Sentry;
///
/// let route = routes![route_name].remove(0);
/// assert_eq!(route.name.unwrap(), "route_name");
/// assert_eq!(route.method, Method::Get);
/// assert_eq!(route.method, Some(Method::Get));
/// assert_eq!(route.uri, "/route/<path..>?query");
/// assert_eq!(route.rank, 2);
/// assert_eq!(route.format.unwrap(), MediaType::JSON);
Expand Down Expand Up @@ -164,8 +164,8 @@ use crate::sentinel::Sentry;
pub struct Route {
/// The name of this route, if one was given.
pub name: Option<Cow<'static, str>>,
/// The method this route matches against.
pub method: Method,
/// The method this route matches, or `None` to match any method.
pub method: Option<Method>,
/// The function that should be called when the route matches.
pub handler: Box<dyn Handler>,
/// The route URI.
Expand Down Expand Up @@ -203,12 +203,12 @@ impl Route {
/// // this is a route matching requests to `GET /`
/// let index = Route::new(Method::Get, "/", handler);
/// assert_eq!(index.rank, -9);
/// assert_eq!(index.method, Method::Get);
/// assert_eq!(index.method, Some(Method::Get));
/// assert_eq!(index.uri, "/");
/// ```
#[track_caller]
pub fn new<H: Handler>(method: Method, uri: &str, handler: H) -> Route {
Route::ranked(None, method, uri, handler)
pub fn new<M: Into<Option<Method>>, H: Handler>(method: M, uri: &str, handler: H) -> Route {
Route::ranked(None, method.into(), uri, handler)
}

/// Creates a new route with the given rank, method, path, and handler with
Expand All @@ -233,17 +233,19 @@ impl Route {
///
/// let foo = Route::ranked(1, Method::Post, "/foo?bar", handler);
/// assert_eq!(foo.rank, 1);
/// assert_eq!(foo.method, Method::Post);
/// assert_eq!(foo.method, Some(Method::Post));
/// assert_eq!(foo.uri, "/foo?bar");
///
/// let foo = Route::ranked(None, Method::Post, "/foo?bar", handler);
/// assert_eq!(foo.rank, -12);
/// assert_eq!(foo.method, Method::Post);
/// assert_eq!(foo.method, Some(Method::Post));
/// assert_eq!(foo.uri, "/foo?bar");
/// ```
#[track_caller]
pub fn ranked<H, R>(rank: R, method: Method, uri: &str, handler: H) -> Route
where H: Handler + 'static, R: Into<Option<isize>>,
pub fn ranked<M, H, R>(rank: R, method: M, uri: &str, handler: H) -> Route
where M: Into<Option<Method>>,
H: Handler + 'static,
R: Into<Option<isize>>,
{
let uri = RouteUri::new("/", uri);
let rank = rank.into().unwrap_or_else(|| uri.default_rank());
Expand All @@ -253,7 +255,9 @@ impl Route {
sentinels: Vec::new(),
handler: Box::new(handler),
location: None,
rank, uri, method,
method: method.into(),
rank,
uri,
}
}

Expand Down Expand Up @@ -362,7 +366,7 @@ pub struct StaticInfo {
/// The route's name, i.e, the name of the function.
pub name: &'static str,
/// The route's method.
pub method: Method,
pub method: Option<Method>,
/// The route's URi, without the base mount point.
pub uri: &'static str,
/// The route's format, if any.
Expand Down
14 changes: 11 additions & 3 deletions core/lib/src/router/collider.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::catcher::Catcher;
use crate::route::{Route, Segment, RouteUri};

use crate::http::MediaType;
use crate::http::{MediaType, Method};

pub trait Collide<T = Self> {
fn collides_with(&self, other: &T) -> bool;
Expand Down Expand Up @@ -87,7 +87,7 @@ impl Route {
/// assert!(a.collides_with(&b));
/// ```
pub fn collides_with(&self, other: &Route) -> bool {
self.method == other.method
methods_collide(self, other)
&& self.rank == other.rank
&& self.uri.collides_with(&other.uri)
&& formats_collide(self, other)
Expand Down Expand Up @@ -190,8 +190,16 @@ impl Collide for MediaType {
}
}

fn methods_collide(route: &Route, other: &Route) -> bool {
match (route.method, other.method) {
(Some(a), Some(b)) => a == b,
(None, _) | (_, None) => true,
}
}

fn formats_collide(route: &Route, other: &Route) -> bool {
match (route.method.allows_request_body(), other.method.allows_request_body()) {
let payload_support = |m: &Option<Method>| m.and_then(|m| m.allows_request_body());
match (payload_support(&route.method), payload_support(&other.method)) {
// 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.
Expand Down
Loading

0 comments on commit aa12827

Please sign in to comment.