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

Change path macro to assume end by default #359

Merged
merged 1 commit into from Dec 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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