Skip to content

Commit

Permalink
Change path macro to assume end by default (#359)
Browse files Browse the repository at this point in the history
  • Loading branch information
seanmonstar committed Dec 30, 2019
1 parent da74792 commit 71aedea
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 25 deletions.
95 changes: 70 additions & 25 deletions src/filters/path.rs
Expand Up @@ -140,14 +140,15 @@ use crate::route::Route;
///
/// This will try to match exactly to the current request path segment.
///
/// Warning:
/// - [`end()`](./fn.end.html) should be used to match the end of a path to avoid having
/// filters for shorter paths like `/math` unintentionally match a longer
/// path such as `/math/sum`
/// - path-related filters should generally come **before** other types of filters, such
/// as those checking headers or body types. Including those other filters before
/// the path checks may result in strange errors being returned because a given request
/// does not match the parameters for a completely separate route.
/// # Note
///
/// - [`end()`](./fn.end.html) should be used to match the end of a path to avoid having
/// filters for shorter paths like `/math` unintentionally match a longer
/// path such as `/math/sum`
/// - Path-related filters should generally come **before** other types of filters, such
/// as those checking headers or body types. Including those other filters before
/// the path checks may result in strange errors being returned because a given request
/// does not match the parameters for a completely separate route.
///
/// # Panics
///
Expand Down Expand Up @@ -447,33 +448,34 @@ fn path_and_query(route: &Route) -> PathAndQuery {
/// let route = warp::path("sum")
/// .and(warp::path::param::<u32>())
/// .and(warp::path::param::<u32>())
/// .and(warp::path::end())
/// .map(|a, b| {
/// format!("{} + {} = {}", a, b, a + b)
/// });
/// ```
///
/// In fact, this is exactly what the macro expands to.
/// # Path Prefixes
///
/// The `path!` macro automatically assumes the path should include an `end()`
/// filter. To build up a path filter *prefix*, such that the `end()` isn't
/// included, use the `/ ..` syntax.
///
/// Note that path! does not automatically include an `end()` filter
/// so you should be careful to avoid letting shorter paths accidentally
/// match longer ones:
///
/// ```
/// use warp::Filter;
///
/// let sum = warp::path!("math" / "sum" / u32 / u32)
/// let prefix = warp::path!("math" / "sum" / ..);
///
/// let sum = warp::path!(u32 / u32)
/// .map(|a, b| {
/// format!("{} + {} = {}", a, b, a + b)
/// });
/// let help = warp::path!("math" / "sum")
///
/// let help = warp::path::end()
/// .map(|| "This API returns the sum of two u32's");
/// let api = help.or(sum);
/// ```
///
/// In the above example, the `sum` path won't actually be hit because
/// the `help` filter will match all requests. We would need to change
/// help to be `path!("math" / "sum").and(warp::path::end())` to ensure
/// that the shorter path filter does not match the longer paths.
/// let api = prefix.and(sum.or(help));
/// ```
#[macro_export]
macro_rules! path {
($($pieces:tt)*) => ({
Expand All @@ -485,17 +487,60 @@ macro_rules! path {
#[macro_export]
// not public API
macro_rules! __internal_path {
(@start ..) => ({
compile_error!("'..' cannot be the only segment")
});
(@start $first:tt $(/ $tail:tt)*) => ({
let __p = $crate::__internal_path!(@segment $first);
$(
let __p = $crate::Filter::and(__p, $crate::__internal_path!(@segment $tail));
)*
__p
$crate::__internal_path!(@munch $crate::any(); [$first] [$(/ $tail)*])
});

(@munch $sum:expr; [$cur:tt] [/ $next:tt $(/ $tail:tt)*]) => ({
$crate::__internal_path!(@munch $crate::Filter::and($sum, $crate::__internal_path!(@segment $cur)); [$next] [$(/ $tail)*])
});
(@munch $sum:expr; [$cur:tt] []) => ({
$crate::__internal_path!(@last $sum; $cur)
});

(@last $sum:expr; ..) => (
$sum
);
(@last $sum:expr; $end:tt) => (
$crate::Filter::and(
$crate::Filter::and($sum, $crate::__internal_path!(@segment $end)),
$crate::path::end()
)
);

(@segment ..) => (
compile_error!("'..' must be the last segment")
);
(@segment $param:ty) => (
$crate::path::param::<$param>()
);
(@segment $s:expr) => (
$crate::path($s)
);
}

// path! compile fail tests

/// ```compile_fail
/// warp::path!("foo" / .. / "bar");
/// ```
///
/// ```compile_fail
/// warp::path!(.. / "bar");
/// ```
///
/// ```compile_fail
/// warp::path!("foo" ..);
/// ```
///
/// ```compile_fail
/// warp::path!("foo" / .. /);
/// ```
///
/// ```compile_fail
/// warp::path!(..);
/// ```
fn _path_macro_compile_fail() {}
20 changes: 20 additions & 0 deletions tests/path.rs
Expand Up @@ -219,6 +219,26 @@ async fn path_macro() {
let req = warp::test::request().path("/foo/bar");
let p = path!("foo" / String);
assert_eq!(req.filter(&p).await.unwrap(), "bar");

// Requires path end

let req = warp::test::request().path("/foo/bar/baz");
let p = path!("foo" / "bar");
assert!(!req.matches(&p).await);

let req = warp::test::request().path("/foo/bar/baz");
let p = path!("foo" / "bar").and(warp::path("baz"));
assert!(!req.matches(&p).await);

// Prefix syntax

let req = warp::test::request().path("/foo/bar/baz");
let p = path!("foo" / "bar" / ..);
assert!(req.matches(&p).await);

let req = warp::test::request().path("/foo/bar/baz");
let p = path!("foo" / "bar" / ..).and(warp::path!("baz"));
assert!(req.matches(&p).await);
}

#[tokio::test]
Expand Down

0 comments on commit 71aedea

Please sign in to comment.