From f1f533c1e5b0df5b44877d7cca39fb0f596a21b6 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 5 May 2023 11:41:44 -0700 Subject: [PATCH] Improve and fix panic in 'AdHoc::normalizer()'. The normalizer now handles more cases. --- core/lib/src/fairing/ad_hoc.rs | 7 ++- core/lib/tests/adhoc-uri-normalizer.rs | 83 +++++++++++--------------- 2 files changed, 39 insertions(+), 51 deletions(-) diff --git a/core/lib/src/fairing/ad_hoc.rs b/core/lib/src/fairing/ad_hoc.rs index 4da164b3..232e2f2a 100644 --- a/core/lib/src/fairing/ad_hoc.rs +++ b/core/lib/src/fairing/ad_hoc.rs @@ -321,7 +321,7 @@ impl AdHoc { fn routes(&self, rocket: &Rocket) -> &[crate::Route] { self.routes.get_or_set(|| { rocket.routes() - .filter(|r| r.uri.has_trailing_slash() || r.uri.metadata.dynamic_trail) + .filter(|r| r.uri.has_trailing_slash()) .cloned() .collect() }) @@ -353,7 +353,8 @@ impl AdHoc { let new_path = path.as_str() .rsplit_once('/') .map(|(prefix, _)| prefix) - .unwrap_or(path.as_str()); + .filter(|path| !path.is_empty()) + .unwrap_or("/"); let base = route.uri.base().as_str(); let uri = match route.uri.unmounted().query() { @@ -362,7 +363,7 @@ impl AdHoc { }; let mut route = route.clone(); - route.uri = RouteUri::try_new(base, &uri).expect("valid => valid"); + route.uri = RouteUri::try_new(base, &uri).ok()?; route.name = route.name.map(|r| format!("{} [normalized]", r).into()); Some(route) }) diff --git a/core/lib/tests/adhoc-uri-normalizer.rs b/core/lib/tests/adhoc-uri-normalizer.rs index a3774f04..ecd45793 100644 --- a/core/lib/tests/adhoc-uri-normalizer.rs +++ b/core/lib/tests/adhoc-uri-normalizer.rs @@ -20,60 +20,47 @@ fn baz(_baz: PathBuf) -> &'static str { "baz" } #[get("/doggy/<_>/<_baz..>?doggy")] fn doggy(_baz: PathBuf) -> &'static str { "doggy" } +#[get("/<_..>")] +fn rest() -> &'static str { "rest" } + +macro_rules! assert_response { + ($client:ident : $path:expr => $response:expr) => { + let response = $client.get($path).dispatch().into_string().unwrap(); + assert_eq!(response, $response, "\nGET {}: got {} but expected {}", + $path, response, $response); + }; +} + #[test] fn test_adhoc_normalizer_works_as_expected () { let rocket = rocket::build() .mount("/", routes![foo, bar, not_bar, baz, doggy]) - .mount("/base", routes![foo, bar, not_bar, baz, doggy]) + .mount("/base", routes![foo, bar, not_bar, baz, doggy, rest]) .attach(AdHoc::uri_normalizer()); - let client = Client::debug(rocket).unwrap(); + let client = match Client::debug(rocket) { + Ok(client) => client, + Err(e) => { e.pretty_print(); panic!("failed to build client"); } + }; - let response = client.get("/foo/").dispatch(); - assert_eq!(response.into_string().unwrap(), "foo"); + assert_response!(client: "/foo" => "foo"); + assert_response!(client: "/foo/" => "foo"); + assert_response!(client: "/bar/" => "bar"); + assert_response!(client: "/bar" => "not_bar"); + assert_response!(client: "/foo/bar" => "baz"); + assert_response!(client: "/doggy/bar?doggy" => "doggy"); + assert_response!(client: "/foo/bar/" => "baz"); + assert_response!(client: "/foo/bar/baz" => "baz"); + assert_response!(client: "/base/foo/" => "foo"); + assert_response!(client: "/base/foo" => "foo"); + assert_response!(client: "/base/bar/" => "bar"); + assert_response!(client: "/base/bar" => "not_bar"); + assert_response!(client: "/base/foo/bar" => "baz"); + assert_response!(client: "/doggy/foo/bar?doggy" => "doggy"); + assert_response!(client: "/base/foo/bar/" => "baz"); + assert_response!(client: "/base/foo/bar/baz" => "baz"); - let response = client.get("/foo").dispatch(); - assert_eq!(response.into_string().unwrap(), "foo"); - - let response = client.get("/bar/").dispatch(); - assert_eq!(response.into_string().unwrap(), "bar"); - - let response = client.get("/bar").dispatch(); - assert_eq!(response.into_string().unwrap(), "not_bar"); - - let response = client.get("/foo/bar").dispatch(); - assert_eq!(response.into_string().unwrap(), "baz"); - - let response = client.get("/doggy/bar?doggy").dispatch(); - assert_eq!(response.into_string().unwrap(), "doggy"); - - let response = client.get("/foo/bar/").dispatch(); - assert_eq!(response.into_string().unwrap(), "baz"); - - let response = client.get("/foo/bar/baz").dispatch(); - assert_eq!(response.into_string().unwrap(), "baz"); - - let response = client.get("/base/foo/").dispatch(); - assert_eq!(response.into_string().unwrap(), "foo"); - - let response = client.get("/base/foo").dispatch(); - assert_eq!(response.into_string().unwrap(), "foo"); - - let response = client.get("/base/bar/").dispatch(); - assert_eq!(response.into_string().unwrap(), "bar"); - - let response = client.get("/base/bar").dispatch(); - assert_eq!(response.into_string().unwrap(), "not_bar"); - - let response = client.get("/base/foo/bar").dispatch(); - assert_eq!(response.into_string().unwrap(), "baz"); - - let response = client.get("/doggy/foo/bar?doggy").dispatch(); - assert_eq!(response.into_string().unwrap(), "doggy"); - - let response = client.get("/base/foo/bar/").dispatch(); - assert_eq!(response.into_string().unwrap(), "baz"); - - let response = client.get("/base/foo/bar/baz").dispatch(); - assert_eq!(response.into_string().unwrap(), "baz"); + assert_response!(client: "/base/cat" => "rest"); + assert_response!(client: "/base/cat/" => "rest"); + assert_response!(client: "/base/cat/dog" => "rest"); }