fix(web): fix `match_*` incorrectness (#3946)

actix-web/CHANGES.md

@@ -3,8 +3,10 @@
 ## Unreleased
 
 - Panic when calling `Route::to()` or `Route::service()` after `Route::wrap()` to prevent silently dropping route middleware. [#3944]
+- Fix `HttpRequest::{match_pattern,match_name}` reporting path-only matches when route guards disambiguate overlapping resources. [#3346]
 
 [#3944]: https://github.com/actix/actix-web/pull/3944
+[#3346]: https://github.com/actix/actix-web/issues/3346
 
 ## 4.13.0
 

actix-web/src/app_service.rs

@@ -228,6 +228,8 @@ where
                 let inner = Rc::get_mut(&mut req.inner).unwrap();
                 inner.path.get_mut().update(&head.uri);
                 inner.path.reset();
+                inner.resource_path.clear();
+                inner.resource_path_matched = false;
                 inner.head = head;
                 inner.conn_data = conn_data;
                 inner.extensions = extensions;
@@ -332,7 +334,15 @@ impl Service<ServiceRequest> for AppRouting {
             guards.iter().all(|guard| guard.check(&guard_ctx))
         });
 
-        if let Some((srv, _info)) = res {
+        if let Some((srv, info)) = res {
+            req.push_resource_id(info.0);
+
+            let matched = req
+                .resource_map()
+                .is_resource_path_match(req.resource_id_path());
+
+            req.mark_resource_path(matched);
+
             srv.call(req)
         } else {
             self.default.call(req)

actix-web/src/request.rs

@@ -42,6 +42,8 @@ pub struct HttpRequest {
 pub(crate) struct HttpRequestInner {
     pub(crate) head: Message<RequestHead>,
     pub(crate) path: Path<Url>,
+    pub(crate) resource_path: SmallVec<[u16; 4]>,
+    pub(crate) resource_path_matched: bool,
     pub(crate) app_data: SmallVec<[Rc<Extensions>; 4]>,
     pub(crate) conn_data: Option<Rc<Extensions>>,
     pub(crate) extensions: Rc<RefCell<Extensions>>,
@@ -65,6 +67,8 @@ impl HttpRequest {
             inner: Rc::new(HttpRequestInner {
                 head,
                 path,
+                resource_path: SmallVec::new(),
+                resource_path_matched: false,
                 app_state,
                 app_data: data,
                 conn_data,
@@ -180,6 +184,26 @@ impl HttpRequest {
         &mut Rc::get_mut(&mut self.inner).unwrap().path
     }
 
+    #[inline]
+    pub(crate) fn push_resource_id(&mut self, id: u16) {
+        Rc::get_mut(&mut self.inner).unwrap().resource_path.push(id);
+    }
+
+    #[inline]
+    pub(crate) fn mark_resource_path(&mut self, is_matched: bool) {
+        Rc::get_mut(&mut self.inner).unwrap().resource_path_matched = is_matched;
+    }
+
+    #[inline]
+    pub(crate) fn resource_path(&self) -> &[u16] {
+        &self.inner.resource_path
+    }
+
+    #[inline]
+    pub(crate) fn is_resource_path_matched(&self) -> bool {
+        self.inner.resource_path_matched
+    }
+
     /// The resource definition pattern that matched the path. Useful for logging and metrics.
     ///
     /// For example, when a resource with pattern `/user/{id}/profile` is defined and a call is made
@@ -188,6 +212,15 @@ impl HttpRequest {
     /// Returns a None when no resource is fully matched, including default services.
     #[inline]
     pub fn match_pattern(&self) -> Option<String> {
+        if self.is_resource_path_matched() {
+            if let Some(pattern) = self
+                .resource_map()
+                .match_pattern_by_resource_path(self.resource_path())
+            {
+                return Some(pattern);
+            }
+        }
+
         self.resource_map().match_pattern(self.path())
     }
 
@@ -196,6 +229,15 @@ impl HttpRequest {
     /// Returns a None when no resource is fully matched, including default services.
     #[inline]
     pub fn match_name(&self) -> Option<&str> {
+        if self.is_resource_path_matched() {
+            if let Some(name) = self
+                .resource_map()
+                .match_name_by_resource_path(self.resource_path())
+            {
+                return Some(name);
+            }
+        }
+
         self.resource_map().match_name(self.path())
     }
 
@@ -633,6 +675,7 @@ mod tests {
     use super::*;
     use crate::{
         dev::{ResourceDef, Service},
+        guard,
         http::{header, StatusCode},
         test::{self, call_service, init_service, read_body, TestRequest},
         web, App, HttpResponse,
@@ -1019,6 +1062,44 @@ mod tests {
         assert_eq!(res.status(), StatusCode::OK);
     }
 
+    #[actix_rt::test]
+    async fn extract_path_pattern_with_guards() {
+        let srv = init_service(
+            App::new().service(
+                web::scope("/widgets")
+                    .service(
+                        web::resource("/{id}")
+                            .name("get_widget")
+                            .guard(guard::Get())
+                            .to(|req: HttpRequest| {
+                                assert_eq!(req.match_pattern(), Some("/widgets/{id}".to_owned()));
+                                assert_eq!(req.match_name(), Some("get_widget"));
+                                HttpResponse::Ok().finish()
+                            }),
+                    )
+                    .service(
+                        web::resource("/action")
+                            .name("widget_action")
+                            .guard(guard::Post())
+                            .to(|req: HttpRequest| {
+                                assert_eq!(req.match_pattern(), Some("/widgets/action".to_owned()));
+                                assert_eq!(req.match_name(), Some("widget_action"));
+                                HttpResponse::Ok().finish()
+                            }),
+                    ),
+            ),
+        )
+        .await;
+
+        let req = TestRequest::get().uri("/widgets/42").to_request();
+        let res = call_service(&srv, req).await;
+        assert_eq!(res.status(), StatusCode::OK);
+
+        let req = TestRequest::post().uri("/widgets/action").to_request();
+        let res = call_service(&srv, req).await;
+        assert_eq!(res.status(), StatusCode::OK);
+    }
+
     #[actix_rt::test]
     async fn extract_path_pattern_complex() {
         let srv = init_service(

actix-web/src/rmap.rs

@@ -240,10 +240,40 @@ impl ResourceMap {
         )
     }
 
+    pub(crate) fn is_resource_path_match(&self, resource_path: &[u16]) -> bool {
+        self.find_node_by_resource_path(resource_path)
+            .is_some_and(|node| node.nodes.is_none())
+    }
+
+    pub(crate) fn match_name_by_resource_path(&self, resource_path: &[u16]) -> Option<&str> {
+        self.find_node_by_resource_path(resource_path)?
+            .pattern
+            .name()
+    }
+
+    pub(crate) fn match_pattern_by_resource_path(&self, resource_path: &[u16]) -> Option<String> {
+        self.find_node_by_resource_path(resource_path)?
+            .root_rmap_fn(String::with_capacity(AVG_PATH_LEN), |mut acc, node| {
+                let pattern = node.pattern.pattern()?;
+                acc.push_str(pattern);
+                Some(acc)
+            })
+    }
+
     fn find_matching_node(&self, path: &str) -> Option<&ResourceMap> {
         self._find_matching_node(path).flatten()
     }
 
+    fn find_node_by_resource_path(&self, resource_path: &[u16]) -> Option<&ResourceMap> {
+        let mut node = self;
+
+        for id in resource_path {
+            node = node.nodes.as_ref()?.get(*id as usize)?;
+        }
+
+        Some(node)
+    }
+
     /// Returns `None` if root pattern doesn't match;
     /// `Some(None)` if root pattern matches but there is no matching child pattern.
     /// Don't search sideways when `Some(none)` is returned.

actix-web/src/scope.rs

@@ -533,7 +533,15 @@ impl Service<ServiceRequest> for ScopeService {
             guards.iter().all(|guard| guard.check(&guard_ctx))
         });
 
-        if let Some((srv, _info)) = res {
+        if let Some((srv, info)) = res {
+            req.push_resource_id(info.0);
+
+            let matched = req
+                .resource_map()
+                .is_resource_path_match(req.resource_id_path());
+
+            req.mark_resource_path(matched);
+
             srv.call(req)
         } else {
             self.default.call(req)

actix-web/src/service.rs

@@ -321,6 +321,21 @@ impl ServiceRequest {
             .push(extensions);
     }
 
+    #[inline]
+    pub(crate) fn push_resource_id(&mut self, id: u16) {
+        self.req.push_resource_id(id);
+    }
+
+    #[inline]
+    pub(crate) fn mark_resource_path(&mut self, is_matched: bool) {
+        self.req.mark_resource_path(is_matched);
+    }
+
+    #[inline]
+    pub(crate) fn resource_id_path(&self) -> &[u16] {
+        self.req.resource_path()
+    }
+
     /// Creates a context object for use with a routing [guard](crate::guard).
     #[inline]
     pub fn guard_ctx(&self) -> GuardContext<'_> {