fix(router,web): fix panic when normalizing and rewriting paths (#3919)

actix-router/CHANGES.md

@@ -4,8 +4,10 @@
 
 - Minimum supported Rust version (MSRV) is now 1.88.
 - Support `deserialize_any` in `PathDeserializer` (enables derived `#[serde(untagged)]` enums in path segments). [#2881]
+- Fix stale path segment indices after path rewrites, preventing out-of-bounds access during extraction. [#3562]
 
 [#2881]: https://github.com/actix/actix-web/pull/2881
+[#3562]: https://github.com/actix/actix-web/issues/3562
 
 ## 0.5.3
 

actix-router/src/path.rs

@@ -93,6 +93,45 @@ impl<T: ResourcePath> Path<T> {
         self.segments.clear();
     }
 
+    /// Set new path while preserving and remapping existing captured segment indices.
+    ///
+    /// The `reindex` closure maps byte indices from the previous path to byte indices in the new
+    /// path.
+    #[doc(hidden)]
+    pub fn update_with_reindex<F>(&mut self, path: T, mut reindex: F)
+    where
+        F: FnMut(u16) -> u16,
+    {
+        self.skip = reindex(self.skip);
+
+        for (_, item) in &mut self.segments {
+            if let PathItem::Segment(start, end) = item {
+                *start = reindex(*start);
+                *end = reindex(*end);
+
+                if *start > *end {
+                    *start = *end;
+                }
+            }
+        }
+
+        self.path = path;
+        let path = self.path.path();
+
+        self.skip = clamp_to_char_boundary(path, self.skip);
+
+        for (_, item) in &mut self.segments {
+            if let PathItem::Segment(start, end) = item {
+                *start = clamp_to_char_boundary(path, *start);
+                *end = clamp_to_char_boundary(path, *end);
+
+                if *start > *end {
+                    *start = *end;
+                }
+            }
+        }
+    }
+
     /// Reset state.
     #[inline]
     pub fn reset(&mut self) {
@@ -179,6 +218,16 @@ impl<T: ResourcePath> Path<T> {
     }
 }
 
+fn clamp_to_char_boundary(path: &str, idx: u16) -> u16 {
+    let mut idx = usize::from(idx).min(path.len());
+
+    while idx > 0 && !path.is_char_boundary(idx) {
+        idx -= 1;
+    }
+
+    idx as u16
+}
+
 #[derive(Debug)]
 pub struct PathIter<'a, T> {
     idx: usize,

actix-web/CHANGES.md

@@ -7,10 +7,12 @@
 - Ignore unparsable cookies in `Cookie` request header.
 - Add `experimental-introspection` feature to report configured routes [#3594]
 - Add config/method for `TCP_NODELAY`. [#3918]
+- Fix panic when `NormalizePath` rewrites a scoped dynamic path before extraction (e.g., `scope("{tail:.*}")` + `Path<String>`). [#3562]
 
 [#3895]: https://github.com/actix/actix-web/pull/3895
 [#3594]: https://github.com/actix/actix-web/pull/3594
 [#3918]: https://github.com/actix/actix-web/pull/3918
+[#3562]: https://github.com/actix/actix-web/issues/3562
 
 ## 4.12.1
 

actix-web/src/middleware/normalize.rs

@@ -1,6 +1,7 @@
 //! For middleware documentation, see [`NormalizePath`].
 
 use actix_http::uri::{PathAndQuery, Uri};
+use actix_router::Url;
 use actix_service::{Service, Transform};
 use actix_utils::future::{ready, Ready};
 use bytes::Bytes;
@@ -14,6 +15,28 @@ use crate::{
     Error,
 };
 
+fn build_byte_index_map(old_path: &str, new_path: &str) -> Vec<u16> {
+    let old_path = old_path.as_bytes();
+    let new_path = new_path.as_bytes();
+
+    let mut map = Vec::with_capacity(old_path.len() + 1);
+    map.push(0);
+
+    let mut old_idx = 0usize;
+    let mut new_idx = 0usize;
+
+    while old_idx < old_path.len() {
+        if new_idx < new_path.len() && old_path[old_idx] == new_path[new_idx] {
+            new_idx += 1;
+        }
+
+        old_idx += 1;
+        map.push(new_idx.min(u16::MAX as usize) as u16);
+    }
+
+    map
+}
+
 /// Determines the behavior of the [`NormalizePath`] middleware.
 ///
 /// The default is `TrailingSlash::Trim`.
@@ -183,6 +206,7 @@ where
             // Both of the paths have the same length,
             // so the change can not be deduced from the length comparison
             if path != original_path {
+                let reindex = build_byte_index_map(original_path, path);
                 let mut parts = head.uri.clone().into_parts();
                 let query = parts.path_and_query.as_ref().and_then(|pq| pq.query());
 
@@ -193,7 +217,11 @@ where
                 parts.path_and_query = Some(PathAndQuery::from_maybe_shared(path).unwrap());
 
                 let uri = Uri::from_parts(parts).unwrap();
-                req.match_info_mut().get_mut().update(&uri);
+                req.match_info_mut()
+                    .update_with_reindex(Url::new(uri.clone()), |idx| {
+                        let idx = usize::from(idx).min(reindex.len() - 1);
+                        reindex[idx]
+                    });
                 req.head_mut().uri = uri;
             }
         }
@@ -209,7 +237,7 @@ mod tests {
     use super::*;
     use crate::{
         guard::fn_guard,
-        test::{call_service, init_service, TestRequest},
+        test::{call_service, init_service, read_body, TestRequest},
         web, App, HttpResponse,
     };
 
@@ -406,6 +434,45 @@ mod tests {
         }
     }
 
+    #[actix_rt::test]
+    async fn scope_dynamic_tail_path_is_reindexed() {
+        async fn handler(path: web::Path<String>) -> HttpResponse {
+            HttpResponse::Ok().body(path.into_inner())
+        }
+
+        let app = init_service(
+            App::new().service(
+                web::scope("{tail:.*}")
+                    .wrap(NormalizePath::trim())
+                    .default_service(web::to(handler)),
+            ),
+        )
+        .await;
+
+        let req = TestRequest::with_uri("/uaie//iuaei").to_request();
+        let res = call_service(&app, req).await;
+
+        assert_eq!(res.status(), StatusCode::OK);
+        assert_eq!(read_body(res).await, Bytes::from_static(b"uaie/iuaei"));
+    }
+
+    #[actix_rt::test]
+    async fn scope_static_prefix_skip_is_reindexed() {
+        let app = init_service(
+            App::new().service(
+                web::scope("/api")
+                    .wrap(NormalizePath::trim())
+                    .service(web::resource("/v1").to(HttpResponse::Ok)),
+            ),
+        )
+        .await;
+
+        let req = TestRequest::with_uri("/api//v1").to_request();
+        let res = call_service(&app, req).await;
+
+        assert_eq!(res.status(), StatusCode::OK);
+    }
+
     #[actix_rt::test]
     async fn no_path() {
         let app = init_service(