Allow dynamic parameters to match empty segments.

The net effect of this commit is three-fold:

  * A request to `/` now matches `/<a>`. `/foo/` matches `/<a>/<b>`.
  * A segment matched to a dynamic parameter may be empty.
  * A request to `/foo/` no longer matches `/foo` or `/<a>`. Instead,
    such a request would match `/foo/<a>` or `/foo/`.

The `&str` and `String` parameter guards were updated to reflect this
change: they now error, with a newly introduced error type `Empty` in
the `rocket::error` module, when the parameter is empty. As this was the
only built-in parameter guard that would be effected by this change (all
other guards already required nonempty parameters to succeed), the
majority of applications will see no effect as a result.

For applications wanting the previous functionality, a new
`AdHoc::uri_normalizer()` fairing was introduced.
This commit is contained in:
Sergio Benitez 2023-04-07 10:46:53 -07:00
parent 0a56312607
commit ac0a77bae2
12 changed files with 55 additions and 35 deletions

View File

@ -2,13 +2,13 @@ error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str`
--> tests/ui-fail-nightly/typed-uri-bad-type.rs:22:37 --> tests/ui-fail-nightly/typed-uri-bad-type.rs:22:37
| |
22 | fn optionals(id: Option<i32>, name: Result<String, &str>) { } 22 | fn optionals(id: Option<i32>, name: Result<String, &str>) { }
| ^^^^^^^^^^^^^^^^^^^^ expected `Infallible`, found `&str` | ^^^^^^^^^^^^^^^^^^^^ expected `Empty`, found `&str`
error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str` error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str`
--> tests/ui-fail-nightly/typed-uri-bad-type.rs:22:37 --> tests/ui-fail-nightly/typed-uri-bad-type.rs:22:37
| |
22 | fn optionals(id: Option<i32>, name: Result<String, &str>) { } 22 | fn optionals(id: Option<i32>, name: Result<String, &str>) { }
| ^^^^^^^^^^^^^^^^^^^^ expected `&str`, found `Infallible` | ^^^^^^^^^^^^^^^^^^^^ expected `&str`, found `Empty`
error[E0277]: the trait bound `usize: FromUriParam<rocket::http::uri::fmt::Path, &str>` is not satisfied error[E0277]: the trait bound `usize: FromUriParam<rocket::http::uri::fmt::Path, &str>` is not satisfied
--> tests/ui-fail-nightly/typed-uri-bad-type.rs:45:22 --> tests/ui-fail-nightly/typed-uri-bad-type.rs:45:22

View File

@ -285,10 +285,10 @@ error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str`
--> tests/ui-fail-nightly/typed-uris-bad-params.rs:15:37 --> tests/ui-fail-nightly/typed-uris-bad-params.rs:15:37
| |
15 | fn optionals(id: Option<i32>, name: Result<String, &str>) { } 15 | fn optionals(id: Option<i32>, name: Result<String, &str>) { }
| ^^^^^^^^^^^^^^^^^^^^ expected `Infallible`, found `&str` | ^^^^^^^^^^^^^^^^^^^^ expected `Empty`, found `&str`
error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str` error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str`
--> tests/ui-fail-nightly/typed-uris-bad-params.rs:15:37 --> tests/ui-fail-nightly/typed-uris-bad-params.rs:15:37
| |
15 | fn optionals(id: Option<i32>, name: Result<String, &str>) { } 15 | fn optionals(id: Option<i32>, name: Result<String, &str>) { }
| ^^^^^^^^^^^^^^^^^^^^ expected `&str`, found `Infallible` | ^^^^^^^^^^^^^^^^^^^^ expected `&str`, found `Empty`

View File

@ -2,13 +2,13 @@ error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str`
--> tests/ui-fail-stable/typed-uri-bad-type.rs:22:37 --> tests/ui-fail-stable/typed-uri-bad-type.rs:22:37
| |
22 | fn optionals(id: Option<i32>, name: Result<String, &str>) { } 22 | fn optionals(id: Option<i32>, name: Result<String, &str>) { }
| ^^^^^^ expected enum `Infallible`, found `&str` | ^^^^^^ expected struct `Empty`, found `&str`
error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str` error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str`
--> tests/ui-fail-stable/typed-uri-bad-type.rs:22:37 --> tests/ui-fail-stable/typed-uri-bad-type.rs:22:37
| |
22 | fn optionals(id: Option<i32>, name: Result<String, &str>) { } 22 | fn optionals(id: Option<i32>, name: Result<String, &str>) { }
| ^^^^^^ expected `&str`, found enum `Infallible` | ^^^^^^ expected `&str`, found struct `Empty`
error[E0277]: the trait bound `usize: FromUriParam<rocket::http::uri::fmt::Path, &str>` is not satisfied error[E0277]: the trait bound `usize: FromUriParam<rocket::http::uri::fmt::Path, &str>` is not satisfied
--> tests/ui-fail-stable/typed-uri-bad-type.rs:45:22 --> tests/ui-fail-stable/typed-uri-bad-type.rs:45:22

View File

@ -276,10 +276,10 @@ error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str`
--> tests/ui-fail-stable/typed-uris-bad-params.rs:15:37 --> tests/ui-fail-stable/typed-uris-bad-params.rs:15:37
| |
15 | fn optionals(id: Option<i32>, name: Result<String, &str>) { } 15 | fn optionals(id: Option<i32>, name: Result<String, &str>) { }
| ^^^^^^ expected enum `Infallible`, found `&str` | ^^^^^^ expected struct `Empty`, found `&str`
error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str` error[E0271]: type mismatch resolving `<String as FromParam<'_>>::Error == &str`
--> tests/ui-fail-stable/typed-uris-bad-params.rs:15:37 --> tests/ui-fail-stable/typed-uris-bad-params.rs:15:37
| |
15 | fn optionals(id: Option<i32>, name: Result<String, &str>) { } 15 | fn optionals(id: Option<i32>, name: Result<String, &str>) { }
| ^^^^^^ expected `&str`, found enum `Infallible` | ^^^^^^ expected `&str`, found struct `Empty`

View File

@ -98,6 +98,10 @@ pub enum ErrorKind {
), ),
} }
/// An error that occurs when a value was unexpectedly empty.
#[derive(Clone, Copy, Default, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct Empty;
impl From<ErrorKind> for Error { impl From<ErrorKind> for Error {
fn from(kind: ErrorKind) -> Self { fn from(kind: ErrorKind) -> Self {
Error::new(kind) Error::new(kind)
@ -259,3 +263,17 @@ impl Drop for Error {
} }
} }
} }
impl fmt::Debug for Empty {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("empty parameter")
}
}
impl fmt::Display for Empty {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("empty parameter")
}
}
impl StdError for Empty { }

View File

@ -1,6 +1,7 @@
use std::str::FromStr; use std::str::FromStr;
use std::path::PathBuf; use std::path::PathBuf;
use crate::error::Empty;
use crate::http::uri::{Segments, error::PathError, fmt::Path}; use crate::http::uri::{Segments, error::PathError, fmt::Path};
/// Trait to convert a dynamic path segment string to a concrete value. /// Trait to convert a dynamic path segment string to a concrete value.
@ -184,20 +185,27 @@ pub trait FromParam<'a>: Sized {
} }
impl<'a> FromParam<'a> for &'a str { impl<'a> FromParam<'a> for &'a str {
type Error = std::convert::Infallible; type Error = Empty;
#[inline(always)] #[inline(always)]
fn from_param(param: &'a str) -> Result<&'a str, Self::Error> { fn from_param(param: &'a str) -> Result<&'a str, Self::Error> {
if param.is_empty() {
return Err(Empty);
}
Ok(param) Ok(param)
} }
} }
impl<'a> FromParam<'a> for String { impl<'a> FromParam<'a> for String {
type Error = std::convert::Infallible; type Error = Empty;
#[inline(always)] #[inline(always)]
fn from_param(param: &'a str) -> Result<String, Self::Error> { fn from_param(param: &'a str) -> Result<String, Self::Error> {
// TODO: Tell the user they're being inefficient? if param.is_empty() {
return Err(Empty);
}
Ok(param.to_string()) Ok(param.to_string())
} }
} }

View File

@ -802,6 +802,8 @@ impl<'r> Request<'r> {
/// ```rust /// ```rust
/// # let c = rocket::local::blocking::Client::debug_with(vec![]).unwrap(); /// # let c = rocket::local::blocking::Client::debug_with(vec![]).unwrap();
/// # let get = |uri| c.get(uri); /// # let get = |uri| c.get(uri);
/// use rocket::error::Empty;
///
/// assert_eq!(get("/a/b/c").param(0), Some(Ok("a"))); /// assert_eq!(get("/a/b/c").param(0), Some(Ok("a")));
/// assert_eq!(get("/a/b/c").param(1), Some(Ok("b"))); /// assert_eq!(get("/a/b/c").param(1), Some(Ok("b")));
/// assert_eq!(get("/a/b/c").param(2), Some(Ok("c"))); /// assert_eq!(get("/a/b/c").param(2), Some(Ok("c")));
@ -811,7 +813,7 @@ impl<'r> Request<'r> {
/// assert!(get("/1/b/3").param::<usize>(1).unwrap().is_err()); /// assert!(get("/1/b/3").param::<usize>(1).unwrap().is_err());
/// assert_eq!(get("/1/b/3").param(2), Some(Ok(3))); /// assert_eq!(get("/1/b/3").param(2), Some(Ok(3)));
/// ///
/// assert_eq!(get("/").param::<&str>(0), None); /// assert_eq!(get("/").param::<&str>(0), Some(Err(Empty)));
/// ``` /// ```
#[inline] #[inline]
pub fn param<'a, T>(&'a self, n: usize) -> Option<Result<T, T::Error>> pub fn param<'a, T>(&'a self, n: usize) -> Option<Result<T, T::Error>>
@ -940,7 +942,7 @@ impl<'r> Request<'r> {
/// codegen. /// codegen.
#[inline] #[inline]
pub fn routed_segment(&self, n: usize) -> Option<&str> { pub fn routed_segment(&self, n: usize) -> Option<&str> {
self.routed_segments(0..).get(n).filter(|p| !p.is_empty()) self.routed_segments(0..).get(n)
} }
/// Get the segments beginning at the `n`th, 0-indexed, after the mount /// Get the segments beginning at the `n`th, 0-indexed, after the mount

View File

@ -127,8 +127,6 @@ impl<'a> RouteUri<'a> {
.into_normalized() .into_normalized()
.into_owned(); .into_owned();
dbg!(&base, &origin, &compiled_uri, &uri);
let source = uri.to_string().into(); let source = uri.to_string().into();
let metadata = Metadata::from(&base, &uri); let metadata = Metadata::from(&base, &uri);

View File

@ -19,7 +19,9 @@ impl Collide for Route {
/// * If route doesn't specify a format, it gets requests for any format. /// * If route doesn't specify a format, it gets requests for any format.
/// ///
/// Because query parsing is lenient, and dynamic query parameters can be /// Because query parsing is lenient, and dynamic query parameters can be
/// missing, queries do not impact whether two routes collide. /// missing, the particularities of a query string do not impact whether two
/// routes collide. The query effects the route's color, however, which
/// effects its rank.
fn collides_with(&self, other: &Route) -> bool { fn collides_with(&self, other: &Route) -> bool {
self.method == other.method self.method == other.method
&& self.rank == other.rank && self.rank == other.rank
@ -64,9 +66,7 @@ impl Collide for RouteUri<'_> {
impl Collide for Segment { impl Collide for Segment {
fn collides_with(&self, other: &Self) -> bool { fn collides_with(&self, other: &Self) -> bool {
self.dynamic && !other.value.is_empty() self.dynamic || other.dynamic || self.value == other.value
|| other.dynamic && !self.value.is_empty()
|| self.value == other.value
} }
} }
@ -136,7 +136,6 @@ mod tests {
#[test] #[test]
fn non_collisions() { fn non_collisions() {
assert_no_collision!("/<a>", "/");
assert_no_collision!("/a", "/b"); assert_no_collision!("/a", "/b");
assert_no_collision!("/a/b", "/a"); assert_no_collision!("/a/b", "/a");
assert_no_collision!("/a/b", "/a/c"); assert_no_collision!("/a/b", "/a/c");
@ -152,7 +151,6 @@ mod tests {
assert_no_collision!("/hello", "/hello/"); assert_no_collision!("/hello", "/hello/");
assert_no_collision!("/hello/there", "/hello/there/"); assert_no_collision!("/hello/there", "/hello/there/");
assert_no_collision!("/hello/<a>", "/hello/");
assert_no_collision!("/a?<b>", "/b"); assert_no_collision!("/a?<b>", "/b");
assert_no_collision!("/a/b", "/a?<b>"); assert_no_collision!("/a/b", "/a?<b>");
@ -194,6 +192,8 @@ mod tests {
assert_no_collision!("/a", "/aaa"); assert_no_collision!("/a", "/aaa");
assert_no_collision!("/", "/a"); assert_no_collision!("/", "/a");
assert_no_collision!(ranked "/<a>", "/");
assert_no_collision!(ranked "/hello/<a>", "/hello/");
assert_no_collision!(ranked "/", "/?a"); assert_no_collision!(ranked "/", "/?a");
assert_no_collision!(ranked "/", "/?<a>"); assert_no_collision!(ranked "/", "/?<a>");
assert_no_collision!(ranked "/a/<b>", "/a/<b>?d"); assert_no_collision!(ranked "/a/<b>", "/a/<b>?d");
@ -201,9 +201,11 @@ mod tests {
#[test] #[test]
fn collisions() { fn collisions() {
assert_collision!("/<a>", "/");
assert_collision!("/a", "/a"); assert_collision!("/a", "/a");
assert_collision!("/hello", "/hello"); assert_collision!("/hello", "/hello");
assert_collision!("/hello/there/how/ar", "/hello/there/how/ar"); assert_collision!("/hello/there/how/ar", "/hello/there/how/ar");
assert_collision!("/hello/<a>", "/hello/");
assert_collision!("/<a>", "/<b>"); assert_collision!("/<a>", "/<b>");
assert_collision!("/<a>", "/b"); assert_collision!("/<a>", "/b");

View File

@ -32,9 +32,6 @@ impl Catcher {
/// * It is a default catcher _or_ has a code of `status`. /// * It is a default catcher _or_ has a code of `status`.
/// * Its base is a prefix of the normalized/decoded `req.path()`. /// * Its base is a prefix of the normalized/decoded `req.path()`.
pub(crate) fn matches(&self, status: Status, req: &Request<'_>) -> bool { pub(crate) fn matches(&self, status: Status, req: &Request<'_>) -> bool {
dbg!(self.base.path().segments());
dbg!(req.uri().path().segments());
self.code.map_or(true, |code| code == status.code) self.code.map_or(true, |code| code == status.code)
&& self.base.path().segments().prefix_of(req.uri().path().segments()) && self.base.path().segments().prefix_of(req.uri().path().segments())
} }
@ -70,10 +67,6 @@ fn paths_match(route: &Route, req: &Request<'_>) -> bool {
return true; return true;
} }
if route_seg.dynamic && req_seg.is_empty() {
return false;
}
if !route_seg.dynamic && route_seg.value != req_seg { if !route_seg.dynamic && route_seg.value != req_seg {
return false; return false;
} }
@ -161,6 +154,8 @@ mod tests {
assert!(req_matches_route("/a/b?c=foo&d=z", "/a/b?c=foo&<c..>")); assert!(req_matches_route("/a/b?c=foo&d=z", "/a/b?c=foo&<c..>"));
assert!(req_matches_route("/a/b?c=foo&d=z", "/a/b?d=z&<c..>")); assert!(req_matches_route("/a/b?c=foo&d=z", "/a/b?d=z&<c..>"));
assert!(req_matches_route("/", "/<foo>"));
assert!(req_matches_route("/a", "/<foo>"));
assert!(req_matches_route("/a", "/a")); assert!(req_matches_route("/a", "/a"));
assert!(req_matches_route("/a/", "/a/")); assert!(req_matches_route("/a/", "/a/"));

View File

@ -185,6 +185,7 @@ mod test {
assert!(rankless_route_collisions(&["/<_..>", "/<_>"])); assert!(rankless_route_collisions(&["/<_..>", "/<_>"]));
assert!(rankless_route_collisions(&["/<_>/b", "/a/b"])); assert!(rankless_route_collisions(&["/<_>/b", "/a/b"]));
assert!(rankless_route_collisions(&["/", "/<foo..>"])); assert!(rankless_route_collisions(&["/", "/<foo..>"]));
assert!(rankless_route_collisions(&["/<_>", "/"]));
} }
#[test] #[test]
@ -232,13 +233,13 @@ mod test {
assert!(!rankless_route_collisions(&["/a/b", "/a/b/c"])); assert!(!rankless_route_collisions(&["/a/b", "/a/b/c"]));
assert!(!rankless_route_collisions(&["/a/b/c/d", "/a/b/c/<d>/e"])); assert!(!rankless_route_collisions(&["/a/b/c/d", "/a/b/c/<d>/e"]));
assert!(!rankless_route_collisions(&["/a/d/<b..>", "/a/b/c"])); assert!(!rankless_route_collisions(&["/a/d/<b..>", "/a/b/c"]));
assert!(!rankless_route_collisions(&["/<_>", "/"]));
assert!(!rankless_route_collisions(&["/a/<_>", "/a"])); assert!(!rankless_route_collisions(&["/a/<_>", "/a"]));
assert!(!rankless_route_collisions(&["/a/<_>", "/<_>"])); assert!(!rankless_route_collisions(&["/a/<_>", "/<_>"]));
} }
#[test] #[test]
fn test_no_collision_when_ranked() { fn test_no_collision_when_ranked() {
assert!(!default_rank_route_collisions(&["/<_>", "/"]));
assert!(!default_rank_route_collisions(&["/<a>", "/hello"])); assert!(!default_rank_route_collisions(&["/<a>", "/hello"]));
assert!(!default_rank_route_collisions(&["/hello/bob", "/hello/<b>"])); assert!(!default_rank_route_collisions(&["/hello/bob", "/hello/<b>"]));
assert!(!default_rank_route_collisions(&["/a/b/c/d", "/<a>/<b>/c/d"])); assert!(!default_rank_route_collisions(&["/a/b/c/d", "/<a>/<b>/c/d"]));
@ -298,6 +299,7 @@ mod test {
assert!(route(&router, Get, "/hello").is_some()); assert!(route(&router, Get, "/hello").is_some());
let router = router_with_routes(&["/<a>"]); let router = router_with_routes(&["/<a>"]);
assert!(route(&router, Get, "/").is_some());
assert!(route(&router, Get, "/hello").is_some()); assert!(route(&router, Get, "/hello").is_some());
assert!(route(&router, Get, "/hi").is_some()); assert!(route(&router, Get, "/hi").is_some());
assert!(route(&router, Get, "/bobbbbbbbbbby").is_some()); assert!(route(&router, Get, "/bobbbbbbbbbby").is_some());
@ -307,6 +309,7 @@ mod test {
assert!(route(&router, Get, "/hello/hi").is_some()); assert!(route(&router, Get, "/hello/hi").is_some());
assert!(route(&router, Get, "/i/a").is_some()); assert!(route(&router, Get, "/i/a").is_some());
assert!(route(&router, Get, "/jdlk/asdij").is_some()); assert!(route(&router, Get, "/jdlk/asdij").is_some());
assert!(route(&router, Get, "/a/").is_some());
let mut router = Router::new(); let mut router = Router::new();
router.add_route(Route::new(Put, "/hello", dummy_handler)); router.add_route(Route::new(Put, "/hello", dummy_handler));
@ -347,7 +350,6 @@ mod test {
assert!(route(&router, Put, "/hello").is_none()); assert!(route(&router, Put, "/hello").is_none());
assert!(route(&router, Post, "/hello").is_none()); assert!(route(&router, Post, "/hello").is_none());
assert!(route(&router, Options, "/hello").is_none()); assert!(route(&router, Options, "/hello").is_none());
assert!(route(&router, Get, "/").is_none());
assert!(route(&router, Get, "/hello/").is_none()); assert!(route(&router, Get, "/hello/").is_none());
assert!(route(&router, Get, "/hello/there/").is_none()); assert!(route(&router, Get, "/hello/there/").is_none());
assert!(route(&router, Get, "/hello/there/").is_none()); assert!(route(&router, Get, "/hello/there/").is_none());
@ -355,7 +357,6 @@ mod test {
let router = router_with_routes(&["/<a>/<b>"]); let router = router_with_routes(&["/<a>/<b>"]);
assert!(route(&router, Get, "/a/b/c").is_none()); assert!(route(&router, Get, "/a/b/c").is_none());
assert!(route(&router, Get, "/a").is_none()); assert!(route(&router, Get, "/a").is_none());
assert!(route(&router, Get, "/a/").is_none());
assert!(route(&router, Get, "/a/b/c/d").is_none()); assert!(route(&router, Get, "/a/b/c/d").is_none());
assert!(route(&router, Get, "/a/b/").is_none()); assert!(route(&router, Get, "/a/b/").is_none());
assert!(route(&router, Put, "/hello/hi").is_none()); assert!(route(&router, Put, "/hello/hi").is_none());

View File

@ -74,10 +74,6 @@ fn hello(lang: Option<Lang>, opt: Options<'_>) -> String {
#[launch] #[launch]
fn rocket() -> _ { fn rocket() -> _ {
// FIXME: Check docs corresponding to normalization/matching/colliding.
// FUZZ: If rand_req1.matches(foo) && rand_req2.matches(bar) =>
// rand_req1.collides_with(rand_req2)
rocket::build() rocket::build()
.mount("/", routes![hello]) .mount("/", routes![hello])
.mount("/hello", routes![world, mir]) .mount("/hello", routes![world, mir])