wip

Author: ibraheemdev

src/error.rs

@@ -79,7 +79,7 @@ impl InsertError {
             route.append(&current.prefix);
         }
 
-        // Add the prefixes of any conflicting children.
+        // Add the prefixes of the first conflicting child.
         let mut child = current.children.first();
         while let Some(node) = child {
             route.append(&node.prefix);

src/tree.rs

@@ -34,7 +34,7 @@ pub struct Node<T> {
     // The value stored at this node.
     //
     // See `Node::at` for why an `UnsafeCell` is necessary.
-    value: Option<UnsafeCell<T>>,
+    pub(crate) value: Option<UnsafeCell<T>>,
 
     // A parameter name remapping, stored at nodes that hold values.
     pub(crate) remapping: ParamRemapping,
@@ -77,20 +77,34 @@ struct InsertState<'node, T> {
 }
 
 impl<'node, T> InsertState<'node, T> {
-    fn node(&mut self) -> &mut Node<T> {
+    fn node_mut(&mut self) -> &mut Node<T> {
         match self.child {
             None => self.parent,
             Some(i) => &mut self.parent.children[i],
         }
     }
 
-    fn parent(&mut self) -> Option<&mut Node<T>> {
+    fn parent_mut(&mut self) -> Option<&mut Node<T>> {
         match self.child {
             None => None,
             Some(_) => Some(self.parent),
         }
     }
 
+    fn parent(&self) -> Option<&Node<T>> {
+        match self.child {
+            None => None,
+            Some(_) => Some(self.parent),
+        }
+    }
+
+    fn node(&self) -> &Node<T> {
+        match self.child {
+            None => self.parent,
+            Some(i) => &self.parent.children[i],
+        }
+    }
+
     fn set_child(self, i: usize) -> InsertState<'node, T> {
         match self.child {
             None => InsertState {
@@ -129,19 +143,19 @@ impl<T> Node<T> {
 
         'walk: loop {
             // Find the common prefix between the route and the current node.
-            let len = min(remaining.len(), state.node().prefix.len());
+            let len = min(remaining.len(), state.node_mut().prefix.len());
             let common_prefix = (0..len)
                 .find(|&i| {
-                    remaining[i] != state.node().prefix[i]
+                    remaining[i] != state.node_mut().prefix[i]
                     // Make sure not confuse the start of a wildcard with an escaped `{`.
-                        || remaining.is_escaped(i) != state.node().prefix.is_escaped(i)
+                        || remaining.is_escaped(i) != state.node_mut().prefix.is_escaped(i)
                 })
                 .unwrap_or(len);
 
             // If this node has a longer prefix than we need, we have to fork and extract the
             // common prefix into a shared parent.
-            if state.node().prefix.len() > common_prefix {
-                let node = state.node();
+            if state.node_mut().prefix.len() > common_prefix {
+                let node = state.node_mut();
 
                 // Move the non-matching suffix into a child node.
                 let suffix = node.prefix.as_ref().slice_off(common_prefix).to_owned();
@@ -165,7 +179,7 @@ impl<T> Node<T> {
             }
 
             if remaining.len() == common_prefix {
-                let node = state.node();
+                let node = state.node_mut();
 
                 // This node must not already contain a value.
                 if node.value.is_some() {
@@ -178,50 +192,91 @@ impl<T> Node<T> {
                 return Ok(());
             }
 
+            let common_remaining = remaining;
+
             // Otherwise, the route has a remaining non-matching suffix.
             //
             // We have to search deeper.
             remaining = remaining.slice_off(common_prefix);
             let next = remaining[0];
 
-            // For parameters with a suffix, we have to find the matching suffix or
-            // create a new child node.
-            if matches!(state.node().node_type, NodeType::Param { .. }) {
+            {
                 let terminator = remaining
                     .iter()
                     .position(|&b| b == b'/')
                     // Include the '/' in the suffix.
                     .map(|b| b + 1)
                     .unwrap_or(remaining.len());
 
-                let suffix = remaining.slice_until(terminator);
+                if let Ok(Some(wildcard)) = find_wildcard(remaining.slice_until(terminator)) {
+                    let suffix = remaining.slice_off(wildcard.end);
 
-                if !matches!(*suffix, b"" | b"/") {
-                    if let Some(parent) = state.parent() {
-                        for child in &parent.children {
-                            // If there is a static prefix that also leads to this route
-                            // parameter, we have a prefix-suffix conflict.
-                            //
-                            // For example, `/prefix{a}` and `/{a}suffix` are conflicting, as there is no clear
-                            // choice to match against `/prefixsuffix`.
-                            if matches!(child.node_type, NodeType::Static) && child.wild_child {
+                    if !matches!(*suffix, b"" | b"/") {
+                        // If we are inserting a suffix and there is a static prefix that already leads to this
+                        // route parameter, we have a prefix-suffix conflict.
+                        for child in &state.node().children {
+                            if state.node().wild_child_in_segment() {
                                 return Err(InsertError::conflict(
                                     &route,
-                                    UnescapedRoute::default().as_ref(), // TODO
-                                    child,
+                                    common_remaining,
+                                    state.node(),
                                 ));
                             }
+
+                            if matches!(child.node_type, NodeType::Static)
+                                && child.wild_child_in_segment()
+                            {
+                                return Err(InsertError::conflict(&route, remaining, child));
+                            }
                         }
                     }
                 }
+            }
+
+            // For parameters with a suffix, we have to find the matching suffix or create a new child node.
+            if matches!(state.node_mut().node_type, NodeType::Param { .. }) {
+                let terminator = remaining
+                    .iter()
+                    .position(|&b| b == b'/')
+                    // Include the '/' in the suffix.
+                    .map(|b| b + 1)
+                    .unwrap_or(remaining.len());
+
+                let suffix = remaining.slice_until(terminator);
 
-                for (i, child) in state.node().children.iter().enumerate() {
+                let mut extra_trailing_slash = false;
+                for (i, child) in state.node_mut().children.iter().enumerate() {
                     // Find a matching suffix.
                     if *child.prefix == **suffix {
-                        state.node().priority += 1;
+                        state.node_mut().priority += 1;
                         state = state.set_child(i);
                         continue 'walk;
                     }
+
+                    // Matches except for an extra trailing slash.
+                    if let Some((common, remaining)) = suffix.split_at_checked(child.prefix.len()) {
+                        if *common == *child.prefix && remaining == *b"/" {
+                            extra_trailing_slash = true;
+                        }
+                    }
+                }
+
+                // If we are inserting a conflicting suffix, and there is a static prefix that already leads to
+                // this route parameter, we have a prefix-suffix conflict.
+                if !extra_trailing_slash && !matches!(*suffix, b"" | b"/") {
+                    if let Some(parent) = state.parent_mut() {
+                        if parent.wild_child_in_segment() {
+                            return Err(InsertError::conflict(&route, common_remaining, parent));
+                        }
+
+                        for child in &parent.children {
+                            if matches!(child.node_type, NodeType::Static)
+                                && child.wild_child_in_segment()
+                            {
+                                return Err(InsertError::conflict(&route, common_remaining, child));
+                            }
+                        }
+                    }
                 }
 
                 // Multiple parameters within the same segment, e.g. `/{foo}{bar}`.
@@ -230,19 +285,20 @@ impl<T> Node<T> {
                 }
 
                 // If there is no matching suffix, create a new suffix node.
-                let child = state.node().add_suffix_child(Node {
+                let child = state.node_mut().add_suffix_child(Node {
                     prefix: suffix.to_owned(),
                     node_type: NodeType::Static,
                     priority: 1,
                     ..Node::default()
                 });
-                state.node().node_type = NodeType::Param { suffix: true };
+                let has_suffix = !matches!(*suffix, b"" | b"/");
+                state.node_mut().node_type = NodeType::Param { suffix: has_suffix };
                 state = state.set_child(child);
 
                 // If this is the final route segment, insert the value.
                 if terminator == remaining.len() {
-                    state.node().value = Some(UnsafeCell::new(val));
-                    state.node().remapping = remapping;
+                    state.node_mut().value = Some(UnsafeCell::new(val));
+                    state.node_mut().remapping = remapping;
                     return Ok(());
                 }
 
@@ -252,31 +308,31 @@ impl<T> Node<T> {
 
                 // Create a static node unless we are inserting a parameter.
                 if remaining[0] != b'{' || remaining.is_escaped(0) {
-                    let child = state.node().add_child(Node {
+                    let child = state.node_mut().add_child(Node {
                         node_type: NodeType::Static,
                         priority: 1,
                         ..Node::default()
                     });
-                    state.node().indices.push(remaining[0]);
+                    state.node_mut().indices.push(remaining[0]);
                     state = state.set_child(child);
                 }
 
                 // Insert the remaining route.
-                let last = state.node().insert_route(remaining, val)?;
+                let last = state.node_mut().insert_route(remaining, val)?;
                 last.remapping = remapping;
                 return Ok(());
             }
 
             // Find a child node that matches the next character in the route.
-            for mut i in 0..state.node().indices.len() {
-                if next == state.node().indices[i] {
+            for mut i in 0..state.node_mut().indices.len() {
+                if next == state.node_mut().indices[i] {
                     // Make sure not confuse the start of a wildcard with an escaped `{` or `}`.
                     if matches!(next, b'{' | b'}') && !remaining.is_escaped(0) {
                         continue;
                     }
 
                     // Continue searching in the child.
-                    i = state.node().update_child_priority(i);
+                    i = state.node_mut().update_child_priority(i);
                     state = state.set_child(i);
                     continue 'walk;
                 }
@@ -286,17 +342,17 @@ impl<T> Node<T> {
             //
             // If we're not inserting a wildcard we have to create a static child.
             if (next != b'{' || remaining.is_escaped(0))
-                && state.node().node_type != NodeType::CatchAll
+                && state.node_mut().node_type != NodeType::CatchAll
             {
-                let node = state.node();
+                let node = state.node_mut();
 
                 let terminator = remaining
                     .iter()
                     .position(|&b| b == b'/')
                     .unwrap_or(remaining.len());
 
                 if let Ok(Some(wildcard)) = find_wildcard(remaining.slice_until(terminator)) {
-                    // If there is a parameter prefix and this node also has a parameter suffix, we
+                    // If we are inserting a parameter prefix and this node already has a parameter suffix, we
                     // have a prefix-suffix conflict.
                     if wildcard.start > 0 && node.wild_child {
                         let wild_child = node.children.last().unwrap();
@@ -320,35 +376,105 @@ impl<T> Node<T> {
             // We're trying to insert a wildcard.
             //
             // If this node already has a wildcard child, we have to make sure it matches.
-            if state.node().wild_child {
+            if state.node_mut().wild_child {
                 // Wildcards are always the last child.
-                let wild_child = state.node().children.len() - 1;
+                let wild_child = state.node_mut().children.len() - 1;
                 state = state.set_child(wild_child);
-                state.node().priority += 1;
+                state.node_mut().priority += 1;
 
                 // Make sure the route parameter matches.
-                if let Some(wildcard) = remaining.get(..state.node().prefix.len()) {
-                    if *wildcard != *state.node().prefix {
-                        return Err(InsertError::conflict(&route, remaining, state.node()));
+                if let Some(wildcard) = remaining.get(..state.node_mut().prefix.len()) {
+                    if *wildcard != *state.node_mut().prefix {
+                        return Err(InsertError::conflict(&route, remaining, state.node_mut()));
                     }
                 }
 
                 // Catch-all routes cannot have children.
-                if state.node().node_type == NodeType::CatchAll {
-                    return Err(InsertError::conflict(&route, remaining, state.node()));
+                if state.node_mut().node_type == NodeType::CatchAll {
+                    return Err(InsertError::conflict(&route, remaining, state.node_mut()));
+                }
+
+                if let Some(parent) = state.parent() {
+                    // If there is a route with both a prefix and a suffix, and we are inserting a route with a
+                    // matching prefix but _without_ a suffix, we have a prefix-suffix conflict.
+                    if !parent.prefix.ends_with(b"/")
+                        && !state.node().prefix.starts_with(b"/")
+                        && matches!(state.node().node_type, NodeType::Param { suffix: true })
+                    {
+                        let terminator = remaining
+                            .iter()
+                            .position(|&b| b == b'/')
+                            // Include the '/' in the suffix.
+                            .map(|b| b + 1)
+                            .unwrap_or(remaining.len());
+
+                        if let Ok(Some(wildcard)) = find_wildcard(remaining.slice_until(terminator))
+                        {
+                            let suffix = remaining.slice_off(wildcard.end);
+
+                            if matches!(*suffix, b"" | b"/") {
+                                return Err(InsertError::conflict(&route, remaining, parent));
+                            }
+                        }
+                    }
                 }
 
                 // Continue with the wildcard node.
                 continue 'walk;
             }
 
+            if let Ok(Some(wildcard)) = find_wildcard(remaining) {
+                let node = state.node_mut();
+                let suffix = remaining.slice_off(wildcard.end);
+
+                // If we are inserting a suffix and there is a static prefix that already leads to this
+                // route parameter, we have a prefix-suffix conflict.
+                if !matches!(*suffix, b"" | b"/") {
+                    for child in &node.children {
+                        if node.wild_child_in_segment() {
+                            return Err(InsertError::conflict(&route, common_remaining, node));
+                        }
+
+                        if matches!(child.node_type, NodeType::Static)
+                            && child.wild_child_in_segment()
+                        {
+                            return Err(InsertError::conflict(&route, remaining, child));
+                        }
+                    }
+                }
+            }
+
             // Otherwise, create a new node for the wildcard and insert the route.
-            let last = state.node().insert_route(remaining, val)?;
+            let last = state.node_mut().insert_route(remaining, val)?;
             last.remapping = remapping;
             return Ok(());
         }
     }
 
+    /// Returns `true` if there is a wildcard parameter node within the current route segment,
+    /// i.e. before a trailing slash.
+    fn wild_child_in_segment(&self) -> bool {
+        if self.prefix.last() == Some(&b'/') {
+            return false;
+        }
+
+        if self.wild_child {
+            return true;
+        }
+
+        for child in &self.children {
+            if child.prefix.contains(&b'/') {
+                return false;
+            }
+
+            if child.wild_child || child.wild_child_in_segment() {
+                return true;
+            }
+        }
+
+        false
+    }
+
     // Insert a route at this node.
     //
     // If the route starts with a wildcard, a child node will be created for the parameter