From 0183b0f8ccc63a19e263be536400c73912df4669 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 6 Aug 2021 18:48:49 +0100 Subject: [PATCH] soft-disallow prefix resources with tail segments (#379) --- actix-router/src/resource.rs | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index 2ed1229d..61ff587a 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -1070,13 +1070,32 @@ impl ResourceDef { dyn_segment_count += 1; } + if is_prefix && has_tail_segment { + // tail segments in prefixes have no defined semantics + + #[cfg(not(test))] + log::warn!( + "Prefix resources should not have tail segments. \ + Use `ResourceDef::new` constructor. \ + This may become a panic in the future." + ); + + // panic in tests to make this case detectable + #[cfg(test)] + panic!("prefix resource definitions should not have tail segments"); + } + if unprocessed.ends_with('*') { // unnamed tail segment #[cfg(not(test))] - log::warn!("tail segments must have names; consider `{{tail}}*`"); + log::warn!( + "Tail segments must have names. \ + Consider `.../{{tail}}*`. \ + This may become a panic in the future." + ); - // to this case detectable in tests + // panic in tests to make this case detectable #[cfg(test)] panic!("tail segments must have names"); } else if !has_tail_segment && !unprocessed.is_empty() { @@ -1197,7 +1216,6 @@ mod tests { assert_eq!(ResourceDef::new("/"), ResourceDef::new(["/"])); assert_eq!(ResourceDef::new("/"), ResourceDef::new(vec!["/"])); - assert_eq!(ResourceDef::new("/{id}*"), ResourceDef::prefix("/{id}*")); assert_ne!(ResourceDef::new(""), ResourceDef::prefix("")); assert_ne!(ResourceDef::new("/"), ResourceDef::prefix("/")); @@ -1774,11 +1792,11 @@ mod tests { #[test] #[should_panic] fn invalid_unnamed_tail_segment() { - ResourceDef::new(r"/*"); + ResourceDef::new("/*"); } #[test] - // #[should_panic] // TODO: consider if this should be allowed + #[should_panic] fn prefix_plus_tail_match_is_allowed() { ResourceDef::prefix("/user/{id}*"); }