From ec0fe457de3c591e0e38fd6ac5549201d6273bf1 Mon Sep 17 00:00:00 2001
From: Rob Ede <robjtede@icloud.com>
Date: Fri, 7 Jun 2024 22:54:43 +0100
Subject: [PATCH] fix: prevent trailing slashes in scope prefixes

---
 actix-web-codegen/src/scope.rs                | 22 ++++++++++++++-----
 actix-web-codegen/tests/trybuild.rs           |  1 +
 .../tests/trybuild/scope-trailing-slash.rs    |  6 +++++
 .../trybuild/scope-trailing-slash.stderr      |  5 +++++
 4 files changed, 28 insertions(+), 6 deletions(-)
 create mode 100644 actix-web-codegen/tests/trybuild/scope-trailing-slash.rs
 create mode 100644 actix-web-codegen/tests/trybuild/scope-trailing-slash.stderr

diff --git a/actix-web-codegen/src/scope.rs b/actix-web-codegen/src/scope.rs
index 1748671b7..c64bcf8bf 100644
--- a/actix-web-codegen/src/scope.rs
+++ b/actix-web-codegen/src/scope.rs
@@ -18,19 +18,29 @@ fn with_scope_inner(args: TokenStream, input: TokenStream) -> syn::Result<TokenS
     if args.is_empty() {
         return Err(syn::Error::new(
             Span::call_site(),
-            "missing arguments for scope macro, \
-                expected: #[scope(\"/prefix\")]",
+            "missing arguments for scope macro, expected: #[scope(\"/prefix\")]",
         ));
     }
 
     let scope_prefix = syn::parse::<syn::LitStr>(args.clone()).map_err(|err| {
         syn::Error::new(
             err.span(),
-            "argument to scope macro is not a string literal, \
-            expected: #[scope(\"/prefix\")]",
+            "argument to scope macro is not a string literal, expected: #[scope(\"/prefix\")]",
         )
     })?;
 
+    let scope_prefix_value = scope_prefix.value();
+
+    if scope_prefix_value.ends_with("/") {
+        // trailing slashes cause non-obvious problems
+        // it's better to point them out to developers rather than
+
+        return Err(syn::Error::new(
+            scope_prefix.span(),
+            "scopes should not have trailing slashes; see https://docs.rs/actix-web/4/actix_web/struct.Scope.html#avoid-trailing-slashes",
+        ));
+    }
+
     let mut module = syn::parse::<syn::ItemMod>(input).map_err(|err| {
         syn::Error::new(err.span(), "#[scope] macro must be attached to a module")
     })?;
@@ -43,7 +53,7 @@ fn with_scope_inner(args: TokenStream, input: TokenStream) -> syn::Result<TokenS
                 fun.attrs = fun
                     .attrs
                     .iter()
-                    .map(|attr| modify_attribute_with_scope(attr, &scope_prefix.value()))
+                    .map(|attr| modify_attribute_with_scope(attr, &scope_prefix_value))
                     .collect();
             }
         }
@@ -52,7 +62,7 @@ fn with_scope_inner(args: TokenStream, input: TokenStream) -> syn::Result<TokenS
     Ok(module.to_token_stream().into())
 }
 
-// Check if the attribute is a method type and has a route path, then modify it
+/// Checks if the attribute is a method type and has a route path, then modifies it.
 fn modify_attribute_with_scope(attr: &syn::Attribute, scope_path: &str) -> syn::Attribute {
     match (attr.parse_args::<RouteArgs>(), attr.clone().meta) {
         (Ok(route_args), syn::Meta::List(meta_list)) if has_allowed_methods_in_scope(attr) => {
diff --git a/actix-web-codegen/tests/trybuild.rs b/actix-web-codegen/tests/trybuild.rs
index 1b56be72a..91073cf3b 100644
--- a/actix-web-codegen/tests/trybuild.rs
+++ b/actix-web-codegen/tests/trybuild.rs
@@ -21,6 +21,7 @@ fn compile_macros() {
     t.compile_fail("tests/trybuild/scope-on-handler.rs");
     t.compile_fail("tests/trybuild/scope-missing-args.rs");
     t.compile_fail("tests/trybuild/scope-invalid-args.rs");
+    t.compile_fail("tests/trybuild/scope-trailing-slash.rs");
 
     t.pass("tests/trybuild/docstring-ok.rs");
 
diff --git a/actix-web-codegen/tests/trybuild/scope-trailing-slash.rs b/actix-web-codegen/tests/trybuild/scope-trailing-slash.rs
new file mode 100644
index 000000000..84632b59f
--- /dev/null
+++ b/actix-web-codegen/tests/trybuild/scope-trailing-slash.rs
@@ -0,0 +1,6 @@
+use actix_web_codegen::scope;
+
+#[scope("/api/")]
+mod api {}
+
+fn main() {}
diff --git a/actix-web-codegen/tests/trybuild/scope-trailing-slash.stderr b/actix-web-codegen/tests/trybuild/scope-trailing-slash.stderr
new file mode 100644
index 000000000..66933432e
--- /dev/null
+++ b/actix-web-codegen/tests/trybuild/scope-trailing-slash.stderr
@@ -0,0 +1,5 @@
+error: scopes should not have trailing slashes; see https://docs.rs/actix-web/4/actix_web/struct.Scope.html#avoid-trailing-slashes
+ --> tests/trybuild/scope-trailing-slash.rs:3:9
+  |
+3 | #[scope("/api/")]
+  |         ^^^^^^^