soft-disallow prefix resources with tail segments

actix-router/src/resource.rs

@@ -572,6 +572,20 @@ impl ResourceDef {
             PatternType::Prefix(ref prefix) if prefix == path => true,
             PatternType::Prefix(ref prefix) => is_strict_prefix(prefix, path),
 
+            // dynamic prefix
+            PatternType::Dynamic(ref re, _) if !re.as_str().ends_with('$') => {
+                match re.find(path) {
+                    // prefix matches exactly
+                    Some(m) if m.end() == path.len() => true,
+
+                    // prefix matches part
+                    Some(m) => is_strict_prefix(m.as_str(), path),
+
+                    // prefix does not match
+                    None => false,
+                }
+            }
+
             PatternType::Dynamic(ref re, _) => re.is_match(path),
             PatternType::DynamicSet(ref re, _) => re.is_match(path),
         }
@@ -616,18 +630,27 @@ impl ResourceDef {
         profile_method!(find_match);
 
         match &self.pat_type {
-            PatternType::Static(segment) => {
+            PatternType::Static(segment) if path == segment => Some(segment.len()),
-                if segment == path {
+            PatternType::Static(_) => None,
-                    Some(segment.len())
-                } else {
-                    None
-                }
-            }
 
             PatternType::Prefix(prefix) if path == prefix => Some(prefix.len()),
             PatternType::Prefix(prefix) if is_strict_prefix(prefix, path) => Some(prefix.len()),
             PatternType::Prefix(_) => None,
 
+            // dynamic prefix
+            PatternType::Dynamic(ref re, _) if !re.as_str().ends_with('$') => {
+                match re.find(path) {
+                    // prefix matches exactly
+                    Some(m) if m.end() == path.len() => Some(m.end()),
+
+                    // prefix matches part
+                    Some(m) if is_strict_prefix(m.as_str(), path) => Some(m.end()),
+
+                    // prefix does not match
+                    _ => None,
+                }
+            }
+
             PatternType::Dynamic(re, _) => re.find(path).map(|m| m.end()),
 
             PatternType::DynamicSet(re, params) => {
@@ -659,8 +682,8 @@ impl ResourceDef {
     /// assert_eq!(path.unprocessed(), "");
     /// ```
     pub fn capture_match_info<T: ResourcePath>(&self, path: &mut Path<T>) -> bool {
-        profile_method!(is_path_match);
+        profile_method!(capture_match_info);
-        self.capture_match_info_fn(path, &|_, _| true, &None::<()>)
+        self.capture_match_info_fn(path, |_, _| true, ())
     }
 
     /// Collects dynamic segment values into `resource` after matching paths and executing
@@ -683,7 +706,7 @@ impl ResourceDef {
     ///     resource.capture_match_info_fn(
     ///         path,
     ///         // when env var is not set, reject when path contains "admin"
-    ///         &|res, admin_allowed| !res.path().contains("admin"),
+    ///         |res, admin_allowed| !res.path().contains("admin"),
     ///         &admin_allowed
     ///     )
     /// }
@@ -704,15 +727,15 @@ impl ResourceDef {
     pub fn capture_match_info_fn<R, T, F, U>(
         &self,
         resource: &mut R,
-        check_fn: &F,
+        check_fn: F,
-        user_data: &Option<U>,
+        user_data: U,
     ) -> bool
     where
         R: Resource<T>,
         T: ResourcePath,
-        F: Fn(&R, &Option<U>) -> bool,
+        F: FnOnce(&R, U) -> bool,
     {
-        profile_method!(is_path_match_fn);
+        profile_method!(capture_match_info_fn);
 
         let mut segments = <[PathItem; MAX_DYNAMIC_SEGMENTS]>::default();
         let path = resource.resource_path();
@@ -932,7 +955,9 @@ impl ResourceDef {
                 }
                 _ => false,
             })
-            .expect("malformed dynamic segment");
+            .unwrap_or_else(|| {
+                panic!(r#"path "{}" contains malformed dynamic segment"#, pattern)
+            });
 
         let (mut param, mut unprocessed) = pattern.split_at(close_idx + 1);
 
@@ -1022,13 +1047,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() {
@@ -1149,7 +1193,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("/"));
@@ -1478,10 +1521,6 @@ mod tests {
         assert_eq!(&path[0], "test2");
         assert_eq!(path.unprocessed(), "subpath1/subpath2/index.html");
 
-        let resource = ResourceDef::prefix(r"/id/{id:\d{3}}");
-        assert!(resource.is_match("/id/1234"));
-        assert_eq!(resource.find_match("/id/1234"), Some(7));
-
         let resource = ResourceDef::prefix("/user");
         // input string shorter than prefix
         assert!(resource.find_match("/foo").is_none());
@@ -1554,6 +1593,21 @@ mod tests {
         assert!(path.get("uid").is_some());
     }
 
+    #[test]
+    fn dynamic_prefix_proper_segmentation() {
+        let resource = ResourceDef::prefix(r"/id/{id:\d{3}}");
+
+        assert!(resource.is_match("/id/123"));
+        assert!(resource.is_match("/id/123/foo"));
+        assert!(!resource.is_match("/id/1234"));
+        assert!(!resource.is_match("/id/123a"));
+
+        assert_eq!(resource.find_match("/id/123"), Some(7));
+        assert_eq!(resource.find_match("/id/123/foo"), Some(7));
+        assert_eq!(resource.find_match("/id/1234"), None);
+        assert_eq!(resource.find_match("/id/123a"), None);
+    }
+
     #[test]
     fn build_path_map() {
         let resource = ResourceDef::new("/user/{item1}/{item2}/");
@@ -1670,11 +1724,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}*");
     }