From e36d171f1ee0830a4b4a9e3ad71c260cc9c34cb4 Mon Sep 17 00:00:00 2001
From: Rob Ede <robjtede@icloud.com>
Date: Mon, 30 Aug 2021 19:15:30 +0100
Subject: [PATCH] add scope test

---
 actix-router/Cargo.toml      |  4 +--
 actix-router/src/resource.rs | 20 ++++++-----
 src/scope.rs                 | 66 ++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/actix-router/Cargo.toml b/actix-router/Cargo.toml
index 2a2ce1cc1..45632b57e 100644
--- a/actix-router/Cargo.toml
+++ b/actix-router/Cargo.toml
@@ -21,7 +21,7 @@ default = ["http"]
 
 [dependencies]
 bytestring = ">=0.1.5, <2"
-firestorm = "0.4"
+firestorm = "0.5"
 http = { version = "0.2.3", optional = true }
 log = "0.4"
 regex = "1.5"
@@ -29,7 +29,7 @@ serde = "1"
 
 [dev-dependencies]
 criterion = { version = "0.3", features = ["html_reports"] }
-firestorm = { version = "0.4", features = ["enable_system_time"] }
+firestorm = { version = "0.5", features = ["enable_system_time"] }
 http = "0.2.3"
 serde = { version = "1", features = ["derive"] }
 
diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs
index 57bd67bd2..fbf29cc7a 100644
--- a/actix-router/src/resource.rs
+++ b/actix-router/src/resource.rs
@@ -104,11 +104,11 @@ const REGEX_FLAGS: &str = "(?s-m)";
 ///
 ///
 /// # Prefix Resources
-/// A prefix resource is defined as pattern that can match just the start of a path.
+/// A prefix resource is defined as pattern that can match just the start of a path, up to a
+/// segment boundary.
 ///
-/// Prefix patterns with a trailing slash may have a weird, though correct, behavior.
-/// They basically define and require an empty segment to match.
-/// Examples are given below.
+/// Prefix patterns with a trailing slash may have an unexpected, though correct, behavior.
+/// They define and therefore require an empty segment in order to match. Examples are given below.
 ///
 /// Empty pattern matches any path as a prefix.
 ///
@@ -140,7 +140,7 @@ const REGEX_FLAGS: &str = "(?s-m)";
 /// `{name:regex}`. For example, `/user/{id:\d+}` will only match paths where the user ID
 /// is numeric.
 ///
-/// The regex could pontentially match multiple segments. If this is not wanted, then care must be
+/// The regex could potentially match multiple segments. If this is not wanted, then care must be
 /// taken to avoid matching a slash `/`. It is guaranteed, however, that the match ends at a
 /// segment boundary; the pattern `r"(/|$)` is always appended to the regex.
 ///
@@ -1104,10 +1104,12 @@ impl ResourceDef {
         let mut re = format!("({})", re);
 
         // Ensure the match ends at a segment boundary
-        if !is_prefix && !has_tail_segment {
-            re.push('$');
-        } else if is_prefix && !has_tail_segment {
-            re.push_str(r"(/|$)");
+        if !has_tail_segment {
+            if is_prefix {
+                re.push_str(r"(/|$)");
+            } else {
+                re.push('$');
+            }
         }
 
         let re = match Regex::new(&re) {
diff --git a/src/scope.rs b/src/scope.rs
index 97db53eeb..09af2c3e8 100644
--- a/src/scope.rs
+++ b/src/scope.rs
@@ -1153,4 +1153,70 @@ mod tests {
             Bytes::from_static(b"http://localhost:8080/a/b/c/12345")
         );
     }
+
+    #[actix_rt::test]
+    async fn dynamic_scopes() {
+        let srv = init_service(
+            App::new().service(
+                web::scope("/{a}/").service(
+                    web::scope("/{b}/")
+                        .route("", web::get().to(|_: HttpRequest| HttpResponse::Created()))
+                        .route(
+                            "/",
+                            web::get().to(|_: HttpRequest| HttpResponse::Accepted()),
+                        )
+                        .route("/{c}", web::get().to(|_: HttpRequest| HttpResponse::Ok())),
+                ),
+            ),
+        )
+        .await;
+
+        // note the unintuitive behavior with trailing slashes on scopes with dynamic segments
+        let req = TestRequest::with_uri("/a//b//c").to_request();
+        let resp = call_service(&srv, req).await;
+        assert_eq!(resp.status(), StatusCode::OK);
+
+        let req = TestRequest::with_uri("/a//b/").to_request();
+        let resp = call_service(&srv, req).await;
+        assert_eq!(resp.status(), StatusCode::CREATED);
+        
+        let req = TestRequest::with_uri("/a//b//").to_request();
+        let resp = call_service(&srv, req).await;
+        assert_eq!(resp.status(), StatusCode::ACCEPTED);
+
+        let req = TestRequest::with_uri("/a//b//c/d").to_request();
+        let resp = call_service(&srv, req).await;
+        assert_eq!(resp.status(), StatusCode::NOT_FOUND);
+
+        let srv = init_service(
+            App::new().service(
+                web::scope("/{a}").service(
+                    web::scope("/{b}")
+                        .route("", web::get().to(|_: HttpRequest| HttpResponse::Created()))
+                        .route(
+                            "/",
+                            web::get().to(|_: HttpRequest| HttpResponse::Accepted()),
+                        )
+                        .route("/{c}", web::get().to(|_: HttpRequest| HttpResponse::Ok())),
+                ),
+            ),
+        )
+        .await;
+
+        let req = TestRequest::with_uri("/a/b/c").to_request();
+        let resp = call_service(&srv, req).await;
+        assert_eq!(resp.status(), StatusCode::OK);
+        
+        let req = TestRequest::with_uri("/a/b").to_request();
+        let resp = call_service(&srv, req).await;
+        assert_eq!(resp.status(), StatusCode::CREATED);
+        
+        let req = TestRequest::with_uri("/a/b/").to_request();
+        let resp = call_service(&srv, req).await;
+        assert_eq!(resp.status(), StatusCode::ACCEPTED);
+
+        let req = TestRequest::with_uri("/a/b/c/d").to_request();
+        let resp = call_service(&srv, req).await;
+        assert_eq!(resp.status(), StatusCode::NOT_FOUND);
+    }
 }