From a13fc779e8c2859d7edc1bdc764a28aadeb6333a Mon Sep 17 00:00:00 2001 From: Costin Zaharia Date: Thu, 21 Mar 2024 14:23:19 +0100 Subject: [PATCH] Update rspec --- analyzers/rspec/cs/S6934.html | 132 ++++++++++++++++++++++------------ analyzers/rspec/cs/S6934.json | 16 ++--- 2 files changed, 96 insertions(+), 52 deletions(-) diff --git a/analyzers/rspec/cs/S6934.html b/analyzers/rspec/cs/S6934.html index dcd0229f47d..6df28d9a091 100644 --- a/analyzers/rspec/cs/S6934.html +++ b/analyzers/rspec/cs/S6934.html @@ -1,54 +1,92 @@ -

The routing middleware in ASP.NET Core MVC uses a set of predefined rules and conventions to determine -which controller and action method to invoke for a given HTTP request. The routing configuration is typically defined with the MapControllerRoute -method during the application configuration. However, without some extra configuration on the developer’s part, sometimes the routing system cannot -correctly resolve a route and map it to a certain action, resulting in unexpected behavior or errors.

+

When a route template is defined through an attribute on an action method, conventional routing for that action is disabled. To maintain good +practice, it’s recommended not to combine conventional and attribute-based routing within a single controller to avoid unpredicted behavior. As such, +the controller should exclude itself from conventional routing by applying a [Route] attribute.

Why is this an issue?

-

In ASP.NET MVC, when a HttpMethodAttribute (such as -HttpGet, HttpPost, etc) is specified with a -given route template at the action level, it’s important that its controller also has a RouteAttribute defined. If not, then the -route pattern that has been defined in WebApplication.MapControllerRoute is applied, resulting in an unexpected route and potential -confusion. This applies also to when areas are -defined.

-

How to fix it

-

When any of the controller actions is annotated with a HttpMethodAttribute' with a route template, you should also annotate the controller -with the `RouteAttribute as well.

+

In ASP.NET Core MVC, the routing middleware utilizes a series of rules and conventions to +identify the appropriate controller and action method to handle a specific HTTP request. This process, known as conventional routing, is +generally established using the MapControllerRoute +method. This method is typically configured in one central location for all controllers during the application setup.

+

Conversely, attribute routing allows routes to be defined at the controller or action method level. It is possible to mix both +mechanisms. Although it’s permissible to employ diverse routing strategies across multiple controllers, combining both mechanisms within one +controller can result in confusion and increased complexity, as illustrated below.

+
+// Conventional mapping definition
+app.MapControllerRoute(
+    name: "default",
+    pattern: "{controller=Home}/{action=Index}/{id?}");
+
+public class PersonController
+{
+        // Conventional routing:
+        // Matches e.g. /Person/Index/123
+        public IActionResult Index(int? id) => View();
+
+        // Attribute routing:
+        // Matches e.g. /Age/Ascending (and model binds "Age" to sortBy and "Ascending" to direction)
+        // but does not match /Person/List/Age/Ascending
+        [HttpGet(template: "{sortBy}/{direction}")]
+        public IActionResult List(string sortBy, SortOrder direction) => View();
+}
+
+

How to fix it in ASP.NET Core

+

When any of the controller actions are annotated with a HttpMethodAttribute with a +route template defined, you should specify a route template on the controller with the RouteAttribute as well.

Code examples

Noncompliant code example

-
-    public class PersonController : Controller
-    {
-        [HttpGet("GetPerson")]
-        public ActionResult Index() // Noncompliant, this action will be reachable by "/root/GetPerson" instead of "/root/Person/GetPerson"
-        {
-            return View();
-        }
-    }
+
+public class PersonController: Controller
+{
+    // Matches /Person/Index/123
+    public IActionResult Index(int? id) => View();
+
+    // Matches /Age/Ascending
+    [HttpGet(template: "{sortBy}/{direction}")] // Noncompliant: The "Index" and the "List" actions are
+                                                // reachable via different routing mechanisms and routes
+    public IActionResult List(string sortBy, SortOrder direction) => View();
+}
 

 
Compliant solution

+
+[Route("[controller]/{action=Index}")]
+public class PersonController: Controller
+{
+    // Matches /Person/Index/123
+    [Route("{id?}")]
+    public IActionResult Index(int? id) => View();
+
+    // Matches Person/List/Age/Ascending
+    [HttpGet("{sortBy}/{direction}")] // Compliant: The route is relative to the controller
+    public IActionResult List(string sortBy, SortOrder direction) => View();
+}
+

+
There are also alternative options to prevent the mixing of conventional and attribute-based routing:

 
-    public class PersonController: Controller
-    {
-        [HttpGet]
-        public ActionResult Index() // Compliant, no route template is given to the attribute
-        {
-            return View();
-        }
-    }
+// Option 1. Replace the attribute-based routing with a conventional route
+app.MapControllerRoute(
+    name: "Lists",
+    pattern: "{controller}/List/{sortBy}/{direction}",
+    defaults: new { action = "List" } ); // Matches Person/List/Age/Ascending
+
+// Option 2. Use a binding, that does not depend on route templates
+public class PersonController: Controller
+{
+    // Matches Person/List?sortBy=Age&direction=Ascending
+    [HttpGet] // Compliant: Parameters are bound from the query string
+    public IActionResult List(string sortBy, SortOrder direction) => View();
+}
 
-    [Route("Person")]
-    public class PersonController: Controller
-    {
-        [HttpGet("GetPerson")]
-        public ActionResult Index() // Compliant
-        {
-            return View();
-        }
-    }
+// Option 3. Use an absolute route
+public class PersonController: Controller
+{
+    // Matches Person/List/Age/Ascending
+    [HttpGet("/[controller]/[action]/{sortBy}/{direction}")] // Illustrate the expected route by starting with "/"
+    public IActionResult List(string sortBy, SortOrder direction) => View();
+}
 

 
Resources

 
Documentation

@@ -56,6 +94,12 @@ 
Documentation

   
  •  Microsoft Learn - Overview of ASP.NET Core MVC 
    • 
   
  •  Microsoft Learn - Routing to controller actions in ASP.NET
   Core 
    • 
+  
  •  Microsoft Learn - Mixed routing:
+  Attribute routing vs conventional routing 
    • 
+  
  •  Microsoft Learn - HttpMethodAttribute Class 
    • 
+  
  •  Microsoft Learn - RouteAttribute Class 
    • 
 
 
    Articles & blog posts