From 975b6efe89fa745aac8d05092374f58b8d9bc137 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 26 Feb 2026 20:06:47 +0900 Subject: [PATCH] fix(web): fix `match_*` incorrectness (#3946) --- actix-web/CHANGES.md | 2 + actix-web/src/app_service.rs | 12 +++++- actix-web/src/request.rs | 81 ++++++++++++++++++++++++++++++++++++ actix-web/src/rmap.rs | 30 +++++++++++++ actix-web/src/scope.rs | 10 ++++- actix-web/src/service.rs | 15 +++++++ 6 files changed, 148 insertions(+), 2 deletions(-) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index b0bc84fe0..ba53e912a 100644 --- a/actix-web/CHANGES.md +++ b/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 diff --git a/actix-web/src/app_service.rs b/actix-web/src/app_service.rs index 7622bd68a..fadcf825b 100644 --- a/actix-web/src/app_service.rs +++ b/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 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) diff --git a/actix-web/src/request.rs b/actix-web/src/request.rs index 90a437928..852a5a80f 100644 --- a/actix-web/src/request.rs +++ b/actix-web/src/request.rs @@ -42,6 +42,8 @@ pub struct HttpRequest { pub(crate) struct HttpRequestInner { pub(crate) head: Message, pub(crate) path: Path, + pub(crate) resource_path: SmallVec<[u16; 4]>, + pub(crate) resource_path_matched: bool, pub(crate) app_data: SmallVec<[Rc; 4]>, pub(crate) conn_data: Option>, pub(crate) extensions: Rc>, @@ -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 { + 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( diff --git a/actix-web/src/rmap.rs b/actix-web/src/rmap.rs index ee86d271b..d3e7c3917 100644 --- a/actix-web/src/rmap.rs +++ b/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 { + 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. diff --git a/actix-web/src/scope.rs b/actix-web/src/scope.rs index c00c51bed..560a66b8e 100644 --- a/actix-web/src/scope.rs +++ b/actix-web/src/scope.rs @@ -533,7 +533,15 @@ impl Service 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) diff --git a/actix-web/src/service.rs b/actix-web/src/service.rs index 6c7f6f5c8..1e819fcbc 100644 --- a/actix-web/src/service.rs +++ b/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<'_> {