Improve and fix panic in 'AdHoc::normalizer()'.

The normalizer now handles more cases.
rwf2 · May 5, 2023 · f1f533c · f1f533c
1 parent 311a82e
commit f1f533c
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 53 deletions.
diff --git 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<Orbit>) -> &[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
@@ -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 response = client.get("/foo/").dispatch();
-    assert_eq!(response.into_string().unwrap(), "foo");
-
-    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");
+    let client = match Client::debug(rocket) {
+        Ok(client) => client,
+        Err(e) => { e.pretty_print(); panic!("failed to build client"); }
+    };
+
+    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");
+
+    assert_response!(client: "/base/cat" => "rest");
+    assert_response!(client: "/base/cat/" => "rest");
+    assert_response!(client: "/base/cat/dog" => "rest");
 }