dotnet / dotnet/aspnetcore

Unreachable code in RazorPagesRazorViewEngineOptionsSetup.cs

Open
#67,151 0 comments 0 reactions 0 assignees View on GitHub
area-mvc
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 10h
Merged PRs (30d)
281

Description

Hello!
### Description
There is a potential logical bug in the internal `RazorPagesRazorViewEngineOptionsSetup.CombinePath` method. The `else if` condition check is unreachable because the preceding `if` condition completely overlaps it.

https://github.com/dotnet/aspnetcore/blob/fc4c0464f6a5b02d8763d901fe885d1dd12996d9/src/Mvc/Mvc.RazorPages/src/DependencyInjection/RazorPagesRazorViewEngineOptionsSetup.cs#L65-L72

If both `path1.EndsWith('/')` and `path2.StartsWith('/')` are true, the first `if` block triggers because it uses the logical OR (`||`) operator. As a result, the `else if` block with the logical AND (`&&`) operator is dead code and never executes.

### Proposed Fix
The conditions should be reordered so the stricter strict check (`&&`) comes first:

```csharp
if (path1.EndsWith('/') && path2.StartsWith('/'))
{
return string.Concat(path1, path2.AsSpan(1));
}
else if (path1.EndsWith('/') || path2.StartsWith('/'))
{
return path1 + path2;
}
```
Or refactored into a switch expression.

Please review this when you have a moment. Thank you!

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.