Suggest using parameter binding instead of manual parsing of HttpContext
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 290
Description
## Background and Motivation
Although we support scenarios where users can manual bind parameters from the HttpContext associated with their route handler, it is generally recommended to leverage the existing binding logic in ASP.NET to support this.
Note: we prototyped this analyzer as part of the 2022 hackathon.
## Proposed Analyzer
### Analyzer Behavior and Message
When we determine that the user has accessed the `HttpContext.HttpRequest` value to extract parameters, provide a warning with the following message.
> Recommend using built-in parameter binding instead of manually processing HTTP request.
### Category
- [ ] Design
- [ ] Documentation
- [ ] Globalization
- [ ] Interoperability
- [ ] Maintainability
- [ ] Naming
- [ ] Performance
- [ ] Reliability
- [ ] Security
- [ ] Style
- [X] Usage
### Severity Level
- [ ] Error
- [X] Warning
- [ ] Info
- [ ] Hidden
## Usage Scenarios
Some patterns we can detect:
**Untyped parsing**
```C#
app.MapGet("/hey/{name}", (HttpRequest req) => $"Hello {req.RouteValues["name"]}");
```
Suggested fix:
```C#
app.MapGet("/hey/{name}", (string name) => $"Hello {name}");
```
We should can remove the HttpRequest if it's not referenced by anything in the method.
**Typed parsing**
If we can detect the parsing of those members:
```C#
app.MapGet("/todos/{id}", (HttpRequest req) =>
{
if (int.TryParse((string?)req.RouteValues["id"], out var id))
{
return Results.BadRequest();
}
var todo = db.Find(id);
if (todo is null)
{
return Results.BadRequest();
}
return Results.Ok(todo);
});
````
To
```C#
app.MapGet("/todos/{id}", (int id) =>
{
var todo = db.Find(id);
if (todo is null)
{
return Results.BadRequest();
}
return Results.Ok(todo);
});
```
This should also work for the query string:
```C#
app.MapGet("/todos", async (HttpContext context) =>
{
if (!int.TryParse(context.Request.Query["pageIndex"], out var pageIndex))
{
context.Response.StatusCode = 400;
return;
}
if (!int.TryParse(context.Request.Query["pageSize"], out var pageSize))
{
context.Response.StatusCode = 400;
return;
}
await context.Response.WriteAsJsonAsync(db.GetTodos().Take(pageIndex).Skip(pageSize));
});
```
Suggested change:
```C#
app.MapGet("/todos", (int pageIndex, int pageSize, HttpContext context) =>
{
await context.Response.WriteAsJsonAsync(db.GetTodos().Take(pageIndex).Skip(pageSize));
});
```
## Risks
There are some scenarios where users may prefer to manually parse parameters from `HttpContext.HttpRequest` but the warning level of this analyzer is sufficient.
Contributor guide
Assessment
This issue has not been assessed yet.