diff --git a/src/error.rs b/src/error.rs index ab879f3..f3a0b08 100644 --- a/src/error.rs +++ b/src/error.rs @@ -14,8 +14,8 @@ pub enum InsertError { }, /// Only one parameter per route segment is allowed. /// - /// Static segments are also allowed before a parameter, but not after it. For example, - /// `/foo-{bar}` is a valid route, but `/{bar}-foo` is not. + /// For example, `/foo-{bar}` and `/{bar}-foo` are valid routes, but `/{foo}-{bar}` + /// is not. InvalidParamSegment, /// Parameters must be registered with a valid name and matching braces. /// diff --git a/src/tree.rs b/src/tree.rs index 3965ac8..cbc6f61 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -38,6 +38,9 @@ pub(crate) enum NodeType { Root, /// A route parameter, e.g. `/{id}`. Param, + /// A route parameter that is followed by a static suffix + /// before a trailing slash, ex: `/{id}.png`. + ParamSuffix { suffix_start: usize }, /// A catch-all parameter, e.g. `/*file`. CatchAll, /// A static prefix, e.g. `/foo`. @@ -127,7 +130,11 @@ impl Node { // After matching against a wildcard the next character is always `/`. // // Continue searching in the child node if it already exists. - if current.node_type == NodeType::Param && current.children.len() == 1 { + if matches!( + current.node_type, + NodeType::Param | NodeType::ParamSuffix { .. } + ) && current.children.len() == 1 + { debug_assert_eq!(next, b'/'); current = &mut current.children[0]; current.priority += 1; @@ -173,11 +180,14 @@ impl Node { current = current.children.last_mut().unwrap(); current.priority += 1; + let segment = remaining + .iter() + .position(|b| *b == b'/') + .unwrap_or(remaining.len()); + // Make sure the route parameter matches. - if let Some(wildcard) = remaining.get(..current.prefix.len()) { - if *wildcard != *current.prefix { - return Err(InsertError::conflict(&route, remaining, current)); - } + if remaining[..segment] != *current.prefix { + return Err(InsertError::conflict(&route, remaining, current)); } // Catch-all routes cannot have children. @@ -402,10 +412,39 @@ impl Node { prefix = prefix.slice_off(wildcard.start); } + let (node_type, wildcard) = match prefix.get(wildcard.len()) { + // The entire route segment consists of the wildcard. + None | Some(&b'/') => { + let wildcard_prefix = prefix.slice_until(wildcard.len()); + prefix = prefix.slice_off(wildcard_prefix.len()); + (NodeType::Param, wildcard_prefix) + } + // The route parameter is followed a static suffix within the current segment. + _ => { + let end = prefix + .iter() + .position(|&b| b == b'/') + .unwrap_or(prefix.len()); + + let wildcard_prefix = prefix.slice_until(end); + let suffix = wildcard_prefix.slice_off(wildcard.len()); + + // Multiple parameters within the same segment, e.g. `/{foo}{bar}`. + if matches!(find_wildcard(suffix), Ok(Some(_))) { + return Err(InsertError::InvalidParamSegment); + } + + prefix = prefix.slice_off(end); + + let suffix_start = wildcard.len(); + (NodeType::ParamSuffix { suffix_start }, wildcard_prefix) + } + }; + // Add the parameter as a child node. let child = Self { - node_type: NodeType::Param, - prefix: prefix.slice_until(wildcard.len()).to_owned(), + node_type, + prefix: wildcard.to_owned(), ..Self::default() }; @@ -415,8 +454,7 @@ impl Node { current.priority += 1; // If the route doesn't end in the wildcard, we have to insert the suffix as a child. - if wildcard.len() < prefix.len() { - prefix = prefix.slice_off(wildcard.len()); + if !prefix.is_empty() { let child = Self { priority: 1, ..Self::default() @@ -616,6 +654,75 @@ impl Node { // Otherwise, there are no matching routes in the tree. return Err(MatchError::NotFound); } + NodeType::ParamSuffix { suffix_start } => { + let suffix = ¤t.prefix[suffix_start..]; + + // Check for more path segments. + let end = match path.iter().position(|&c| c == b'/') { + // Double `//` implying an empty parameter, no match. + Some(0) => { + try_backtrack!(); + return Err(MatchError::NotFound); + } + // Found another segment. + Some(i) => i, + // This is the last path segment. + None => path.len(), + }; + + // The path cannot contain a non-empty parameter and the suffix. + if suffix.len() >= end { + try_backtrack!(); + return Err(MatchError::NotFound); + } + + // Ensure the suffix matches. + for (a, b) in path[..end].iter().rev().zip(suffix.iter().rev()) { + if a != b { + try_backtrack!(); + return Err(MatchError::NotFound); + } + } + + let param = &path[..end - suffix.len()]; + let rest = &path[end..]; + + if rest.is_empty() { + let value = match current.value { + // Found the matching value. + Some(ref value) => value, + // Otherwise, this route does not match. + None => { + try_backtrack!(); + return Err(MatchError::NotFound); + } + }; + + // Store the parameter value. + params.push(b"", param); + + // Remap the keys of any route parameters we accumulated during the search. + params.for_each_key_mut(|(i, key)| *key = ¤t.remapping[i]); + + return Ok((value, params)); + } + + // If there is a static child, continue the search. + if let [child] = current.children.as_slice() { + // Store the parameter value. + params.push(b"", param); + + // Continue searching. + path = rest; + current = child; + backtracking = false; + continue 'walk; + } + + // Otherwise, this route does not match. + try_backtrack!(); + return Err(MatchError::NotFound); + } NodeType::CatchAll => { // Catch-all segments are only allowed at the end of the route, meaning // this node must contain the value. @@ -785,13 +892,6 @@ fn find_wildcard(path: UnescapedRef<'_>) -> Result>, InsertE return Err(InsertError::InvalidParam); } - if let Some(&c) = path.get(i + 1) { - // Prefixes after route parameters are not supported. - if c != b'/' { - return Err(InsertError::InvalidParamSegment); - } - } - return Ok(Some(start..i + 1)); } // `*` and `/` are invalid in parameter names. diff --git a/tests/insert.rs b/tests/insert.rs index 5513c1c..53b79e1 100644 --- a/tests/insert.rs +++ b/tests/insert.rs @@ -48,6 +48,19 @@ fn wildcard_conflict() { ("/user_{bar}", Err(conflict("/user_{name}"))), ("/id{id}", Ok(())), ("/id/{id}", Ok(())), + ("/x/{id}", Ok(())), + ("/x/{id}/", Ok(())), + ("/x/{id}y", Err(conflict("/x/{id}/"))), + ("/x/{id}y/", Err(conflict("/x/{id}/"))), + ("/x/x{id}", Ok(())), + ("/x/x{id}y", Err(conflict("/x/x{id}"))), + ("/qux/id", Ok(())), + ("/qux/{id}y", Ok(())), + ("/qux/{id}", Err(conflict("/qux/{id}y"))), + ("/qux/{id}/", Err(conflict("/qux/{id}y"))), + ("/qux/{id}x", Err(conflict("/qux/{id}y"))), + ("/qux/x{id}y", Ok(())), + ("/qux/x{id}", Err(conflict("/qux/x{id}y"))), ]) .run() } @@ -210,7 +223,6 @@ fn invalid_param() { ("}", Err(InsertError::InvalidParam)), ("x{y", Err(InsertError::InvalidParam)), ("x}", Err(InsertError::InvalidParam)), - ("/{foo}s", Err(InsertError::InvalidParamSegment)), ]) .run(); } diff --git a/tests/match.rs b/tests/match.rs index acb6123..e39cba9 100644 --- a/tests/match.rs +++ b/tests/match.rs @@ -581,11 +581,50 @@ fn escaped() { #[test] fn empty_param() { MatchTest { - routes: vec!["/y/{foo}", "/x/{foo}/z", "/z/{*xx}"], + routes: vec![ + "/y/{foo}", + "/x/{foo}/z", + "/z/{*foo}", + "/a/x{foo}", + "/b/{foo}x", + ], matches: vec![ ("/y/", "", Err(())), ("/x//z", "", Err(())), ("/z/", "", Err(())), + ("/a/x", "", Err(())), + ("/b/x", "", Err(())), + ], + } + .run(); +} + +#[test] +fn wildcard_suffix() { + MatchTest { + routes: vec![ + "/", + "/{foo}x", + "/foox", + "/{foo}x/bar", + "/{foo}x/bar/baz", + "/x{foo}", + "/x{foo}/bar", + ], + matches: vec![ + ("/", "/", p! {}), + ("/foox", "/foox", p! {}), + ("/barx", "/{foo}x", p! { "foo" => "bar" }), + ("/mx", "/{foo}x", p! { "foo" => "m" }), + ("/mx/", "", Err(())), + ("/mxm", "", Err(())), + ("/mx/bar", "/{foo}x/bar", p! { "foo" => "m" }), + ("/mxm/bar", "", Err(())), + ("/x", "", Err(())), + ("/xfoo", "/x{foo}", p! { "foo" => "foo" }), + ("/xfoox", "/x{foo}", p! { "foo" => "foox" }), + ("/xfoox/bar", "/x{foo}/bar", p! { "foo" => "foox" }), + ("/xfoox/bar/baz", "/{foo}x/bar/baz", p! { "foo" => "xfoo" }), ], } .run();