Unreachable code in RazorPagesRazorViewEngineOptionsSetup.cs
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
Research direction
Start in src/Mvc/Mvc.RazorPages/src/DependencyInjection/RazorPagesRazorViewEngineOptionsSetup.cs at the CombinePath method and inspect the conditions around lines 65-72. Reorder or refactor the checks as proposed, then verify that paths with both trailing and leading slashes are combined without duplicating the slash while the other cases retain their existing behavior.
Written by the indexing model from the issue text.
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.
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:
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.
- Dominant language
- C#
- Stars
- 38.5k
- Forks
- 11.6k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 243
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from dotnet/aspnetcore
-
area-blazor feature-accessibility
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
dotnet/aspnetcore#69387 · 3 comments · 1 assignee ·
-
area-signalr
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
dotnet/aspnetcore#69283 · 1 comment ·
-
api-proposal api-suggestion area-blazor
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
dotnet/aspnetcore#69266 · 1 comment ·
-
area-grpc
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
dotnet/aspnetcore#69169 · 1 comment ·
-
test-failure
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
dotnet/aspnetcore#69095 · 1 comment ·
All issues in dotnet/aspnetcore
Similar issues
-
type/automation type/tech-debt
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
t/bug
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
ci-failure-cause test-failure
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
area:auth FE mvp P3
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
klasolsson81/jobbliggaren#1788 ·