From 5379a46a99e1415d68c73c0048e5f9dbb628dea3 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Fri, 6 Aug 2021 19:45:10 +0300 Subject: [PATCH 1/3] ResourceDef: relax unnecessary bounds (#381) --- actix-router/src/resource.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index 58016bb3..eb0483a5 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -616,13 +616,8 @@ impl ResourceDef { profile_method!(find_match); match &self.pat_type { - PatternType::Static(segment) => { - if segment == path { - Some(segment.len()) - } else { - None - } - } + PatternType::Static(segment) if path == segment => Some(segment.len()), + PatternType::Static(_) => None, PatternType::Prefix(prefix) if path == prefix => Some(prefix.len()), PatternType::Prefix(prefix) if is_strict_prefix(prefix, path) => Some(prefix.len()), @@ -660,7 +655,7 @@ impl ResourceDef { /// ``` pub fn capture_match_info(&self, path: &mut Path) -> bool { profile_method!(is_path_match); - 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 +678,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,13 +699,13 @@ impl ResourceDef { pub fn capture_match_info_fn( &self, resource: &mut R, - check_fn: &F, - user_data: &Option, + check_fn: F, + user_data: U, ) -> bool where R: Resource, T: ResourcePath, - F: Fn(&R, &Option) -> bool, + F: FnOnce(&R, U) -> bool, { profile_method!(is_path_match_fn); From 48b2e115099b5bfbe378f1427d8bf2897f1dc75f Mon Sep 17 00:00:00 2001 From: Aravinth Manivannan Date: Fri, 6 Aug 2021 22:36:29 +0530 Subject: [PATCH 2/3] improve malformed path error message (#384) --- actix-router/src/resource.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index eb0483a5..a3ea8a1c 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -927,7 +927,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); From 430305824371047405441eeaf43db0bf840e0cc2 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 6 Aug 2021 18:25:21 +0100 Subject: [PATCH 3/3] enforce path / separators on dynamic prefixes (#378) --- actix-router/src/resource.rs | 51 +++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index a3ea8a1c..5c98493d 100644 --- a/actix-router/src/resource.rs +++ b/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), } @@ -623,6 +637,20 @@ impl ResourceDef { 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) => { @@ -654,7 +682,7 @@ impl ResourceDef { /// assert_eq!(path.unprocessed(), ""); /// ``` pub fn capture_match_info(&self, path: &mut Path) -> bool { - profile_method!(is_path_match); + profile_method!(capture_match_info); self.capture_match_info_fn(path, |_, _| true, ()) } @@ -707,7 +735,7 @@ impl ResourceDef { T: ResourcePath, 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(); @@ -1475,10 +1503,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()); @@ -1551,6 +1575,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}/");