Skip to content

Commit 370699c

Browse files
DeagleGrosscaptainsafia
DeagleGross
authored and
captainsafia
committed
feat: analyzer and codeFix for kestrel setup ListenOptions.Listen(IPAddress.Any) usage (#58872)
1 parent f91261e commit 370699c

File tree

7 files changed

+334
-0
lines changed

7 files changed

+334
-0
lines changed

‎docs/list-of-diagnostics.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
| __`ASP0022`__ | Route conflict detected between route handlers |
3131
| __`ASP0023`__ | Route conflict detected between controller actions |
3232
| __`ASP0024`__ | Route handler has multiple parameters with the [FromBody] attribute |
33+
| __`ASP0025`__ | Use AddAuthorizationBuilder |
34+
| __`ASP0026`__ | [Authorize] overridden by [AllowAnonymous] from farther away |
35+
| __`ASP0027`__ | Unnecessary public Program class declaration |
36+
| __`ASP0028`__ | Consider using ListenAnyIP() instead of Listen(IPAddress.Any) |
3337

3438
### API (`API1000-API1003`)
3539

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,4 +233,13 @@ internal static class DiagnosticDescriptors
233233
isEnabledByDefault: true,
234234
helpLinkUri: "https://aka.ms/aspnet/analyzers",
235235
customTags: WellKnownDiagnosticTags.Unnecessary);
236+
237+
internal static readonly DiagnosticDescriptor KestrelShouldListenOnIPv6AnyInsteadOfIpAny = new(
238+
"ASP0028",
239+
new LocalizableResourceString(nameof(Resources.Analyzer_KestrelShouldListenOnIPv6AnyInsteadOfIpAny_Title), Resources.ResourceManager, typeof(Resources)),
240+
new LocalizableResourceString(nameof(Resources.Analyzer_KestrelShouldListenOnIPv6AnyInsteadOfIpAny_Message), Resources.ResourceManager, typeof(Resources)),
241+
"Usage",
242+
DiagnosticSeverity.Info,
243+
isEnabledByDefault: true,
244+
helpLinkUri: "https://aka.ms/aspnet/analyzers");
236245
}

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/Kestrel/ListenOnIPv6AnyAnalyzer.cs‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Microsoft.CodeAnalysis.Diagnostics;
5+
using Microsoft.CodeAnalysis;
6+
using System.Collections.Immutable;
7+
using Microsoft.CodeAnalysis.CSharp;
8+
using Microsoft.CodeAnalysis.Operations;
9+
using Microsoft.CodeAnalysis.CSharp.Syntax;
10+
using System.Linq;
11+
12+
namespace Microsoft.AspNetCore.Analyzers.Kestrel;
13+
14+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
15+
public class ListenOnIPv6AnyAnalyzer : DiagnosticAnalyzer
16+
{
17+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [ DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny ];
18+
19+
public override void Initialize(AnalysisContext context)
20+
{
21+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
22+
context.EnableConcurrentExecution();
23+
24+
context.RegisterSyntaxNodeAction(KestrelServerOptionsListenInvocation, SyntaxKind.InvocationExpression);
25+
}
26+
27+
private void KestrelServerOptionsListenInvocation(SyntaxNodeAnalysisContext context)
28+
{
29+
// fail fast before accessing SemanticModel
30+
if (context.Node is not InvocationExpressionSyntax
31+
{
32+
Expression: MemberAccessExpressionSyntax
33+
{
34+
Name: IdentifierNameSyntax { Identifier.ValueText: "Listen" }
35+
}
36+
} kestrelOptionsListenExpressionSyntax)
37+
{
38+
return;
39+
}
40+
41+
var nodeOperation = context.SemanticModel.GetOperation(context.Node, context.CancellationToken);
42+
if (!IsKestrelServerOptionsType(nodeOperation, out var kestrelOptionsListenInvocation))
43+
{
44+
return;
45+
}
46+
47+
var addressArgument = kestrelOptionsListenInvocation?.Arguments.FirstOrDefault();
48+
if (!IsIPAddressType(addressArgument?.Parameter))
49+
{
50+
return;
51+
}
52+
53+
var args = kestrelOptionsListenExpressionSyntax.ArgumentList;
54+
var ipAddressArgumentSyntax = args.Arguments.FirstOrDefault();
55+
if (ipAddressArgumentSyntax is null)
56+
{
57+
return;
58+
}
59+
60+
// explicit usage like `options.Listen(IPAddress.Any, ...)`
61+
if (ipAddressArgumentSyntax is ArgumentSyntax
62+
{
63+
Expression: MemberAccessExpressionSyntax
64+
{
65+
Name: IdentifierNameSyntax { Identifier.ValueText: "Any" }
66+
}
67+
})
68+
{
69+
context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny, ipAddressArgumentSyntax.GetLocation()));
70+
}
71+
72+
// usage via local variable like
73+
// ```
74+
// var myIp = IPAddress.Any;
75+
// options.Listen(myIp, ...);
76+
// ```
77+
if (addressArgument!.Value is ILocalReferenceOperation localReferenceOperation)
78+
{
79+
var localVariableDeclaration = localReferenceOperation.Local.DeclaringSyntaxReferences.FirstOrDefault();
80+
if (localVariableDeclaration is null)
81+
{
82+
return;
83+
}
84+
85+
var localVarSyntax = localVariableDeclaration.GetSyntax(context.CancellationToken);
86+
if (localVarSyntax is VariableDeclaratorSyntax
87+
{
88+
Initializer.Value: MemberAccessExpressionSyntax
89+
{
90+
Name.Identifier.ValueText: "Any"
91+
}
92+
})
93+
{
94+
context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny, ipAddressArgumentSyntax.GetLocation()));
95+
}
96+
}
97+
}
98+
99+
private static bool IsIPAddressType(IParameterSymbol? parameter) => parameter is
100+
{
101+
Type: // searching type `System.Net.IPAddress`
102+
{
103+
Name: "IPAddress",
104+
ContainingNamespace: { Name: "Net", ContainingNamespace: { Name: "System", ContainingNamespace.IsGlobalNamespace: true } }
105+
}
106+
};
107+
108+
private static bool IsKestrelServerOptionsType(IOperation? operation, out IInvocationOperation? kestrelOptionsListenInvocation)
109+
{
110+
var result = operation is IInvocationOperation // searching type `Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions`
111+
{
112+
TargetMethod: { Name: "Listen" },
113+
Instance.Type:
114+
{
115+
Name: "KestrelServerOptions",
116+
ContainingNamespace:
117+
{
118+
Name: "Core",
119+
ContainingNamespace:
120+
{
121+
Name: "Kestrel",
122+
ContainingNamespace:
123+
{
124+
Name: "Server",
125+
ContainingNamespace:
126+
{
127+
Name: "AspNetCore",
128+
ContainingNamespace:
129+
{
130+
Name: "Microsoft",
131+
ContainingNamespace.IsGlobalNamespace: true
132+
}
133+
}
134+
}
135+
}
136+
}
137+
}
138+
};
139+
140+
kestrelOptionsListenInvocation = result ? (IInvocationOperation)operation! : null;
141+
return result;
142+
}
143+
}

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,4 +327,10 @@
327327
<data name="Analyzer_PublicPartialProgramClass_Title" xml:space="preserve">
328328
<value>Unnecessary public Program class declaration</value>
329329
</data>
330+
<data name="Analyzer_KestrelShouldListenOnIPv6AnyInsteadOfIpAny_Title" xml:space="preserve">
331+
<value>Consider using ListenAnyIP() instead of Listen(IPAddress.Any)</value>
332+
</data>
333+
<data name="Analyzer_KestrelShouldListenOnIPv6AnyInsteadOfIpAny_Message" xml:space="preserve">
334+
<value>If the server does not specifically reject IPv6, IPAddress.IPv6Any is preferred over IPAddress.Any usage for safety and performance reasons. See https://aka.ms/aspnetcore-warnings/ASP0028 for more details.</value>
335+
</data>
330336
</root>

‎src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Kestrel/ListenOnIPv6AnyFixer.cs‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Composition;
5+
using Microsoft.CodeAnalysis.CodeFixes;
6+
using Microsoft.CodeAnalysis;
7+
using System.Collections.Immutable;
8+
using System.Threading.Tasks;
9+
using Microsoft.AspNetCore.Analyzers;
10+
using Microsoft.CodeAnalysis.CodeActions;
11+
using Microsoft.CodeAnalysis.Editing;
12+
using Microsoft.CodeAnalysis.CSharp.Syntax;
13+
using Microsoft.CodeAnalysis.CSharp;
14+
15+
namespace Microsoft.AspNetCore.Fixers.Kestrel;
16+
17+
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
18+
public class ListenOnIPv6AnyFixer : CodeFixProvider
19+
{
20+
public override ImmutableArray<string> FixableDiagnosticIds => [ DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny.Id ];
21+
22+
public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
23+
24+
public override Task RegisterCodeFixesAsync(CodeFixContext context)
25+
{
26+
foreach (var diagnostic in context.Diagnostics)
27+
{
28+
context.RegisterCodeFix(
29+
CodeAction.Create(
30+
"Consider using IPAddress.IPv6Any instead of IPAddress.Any",
31+
async cancellationToken =>
32+
{
33+
var editor = await DocumentEditor.CreateAsync(context.Document, cancellationToken).ConfigureAwait(false);
34+
var root = await context.Document.GetSyntaxRootAsync(cancellationToken);
35+
if (root is null)
36+
{
37+
return context.Document;
38+
}
39+
40+
var argumentSyntax = root.FindNode(diagnostic.Location.SourceSpan).FirstAncestorOrSelf<ArgumentSyntax>();
41+
if (argumentSyntax is null)
42+
{
43+
return context.Document;
44+
}
45+
46+
// get to the `Listen(IPAddress.Any, ...)` invocation
47+
if (argumentSyntax.Parent?.Parent is not InvocationExpressionSyntax { ArgumentList.Arguments.Count: > 1 } invocationExpressionSyntax)
48+
{
49+
return context.Document;
50+
}
51+
if (invocationExpressionSyntax.Expression is not MemberAccessExpressionSyntax memberAccessExpressionSyntax)
52+
{
53+
return context.Document;
54+
}
55+
56+
var instanceVariableInvoked = memberAccessExpressionSyntax.Expression;
57+
var adjustedArgumentList = invocationExpressionSyntax.ArgumentList.RemoveNode(invocationExpressionSyntax.ArgumentList.Arguments.First(), SyntaxRemoveOptions.KeepLeadingTrivia);
58+
if (adjustedArgumentList is null || adjustedArgumentList.Arguments.Count == 0)
59+
{
60+
return context.Document;
61+
}
62+
63+
// changing invocation from `<variable>.Listen(IPAddress.Any, ...)` to `<variable>.ListenAnyIP(...)`
64+
editor.ReplaceNode(
65+
invocationExpressionSyntax,
66+
invocationExpressionSyntax
67+
.WithExpression(SyntaxFactory.ParseExpression($"{instanceVariableInvoked.ToString()}.ListenAnyIP"))
68+
.WithArgumentList(adjustedArgumentList!)
69+
.WithLeadingTrivia(invocationExpressionSyntax.GetLeadingTrivia())
70+
);
71+
return editor.GetChangedDocument();
72+
},
73+
equivalenceKey: DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny.Id),
74+
diagnostic);
75+
}
76+
77+
return Task.CompletedTask;
78+
}
79+
}

‎src/Framework/AspNetCoreAnalyzers/test/Kestrel/ListenOnIPv6AnyAnalyzerAndFixerTests.cs‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Text;
7+
using Microsoft.CodeAnalysis.Testing;
8+
using VerifyCS = Microsoft.AspNetCore.Analyzers.Verifiers.CSharpCodeFixVerifier<
9+
Microsoft.AspNetCore.Analyzers.Kestrel.ListenOnIPv6AnyAnalyzer,
10+
Microsoft.AspNetCore.Fixers.Kestrel.ListenOnIPv6AnyFixer>;
11+
12+
namespace Microsoft.AspNetCore.Analyzers.Kestrel;
13+
14+
public class ListenOnIPv6AnyAnalyzerAndFixerTests
15+
{
16+
[Fact]
17+
public async Task ReportsDiagnostic_IPAddressAsLocalVariable_OuterScope()
18+
{
19+
var source = GetKestrelSetupSource("myIp", extraOuterCode: "var myIp = IPAddress.Any;");
20+
await VerifyCS.VerifyAnalyzerAsync(source, codeSampleDiagnosticResult);
21+
}
22+
23+
[Fact]
24+
public async Task ReportsDiagnostic_IPAddressAsLocalVariable()
25+
{
26+
var source = GetKestrelSetupSource("myIp", extraInlineCode: "var myIp = IPAddress.Any;");
27+
await VerifyCS.VerifyAnalyzerAsync(source, codeSampleDiagnosticResult);
28+
}
29+
30+
[Fact]
31+
public async Task ReportsDiagnostic_ExplicitUsage()
32+
{
33+
var source = GetKestrelSetupSource("IPAddress.Any");
34+
await VerifyCS.VerifyAnalyzerAsync(source, codeSampleDiagnosticResult);
35+
}
36+
37+
[Fact]
38+
public async Task CodeFix_ExplicitUsage()
39+
{
40+
var source = GetKestrelSetupSource("IPAddress.Any");
41+
var fixedSource = GetCorrectedKestrelSetup();
42+
await VerifyCS.VerifyCodeFixAsync(source, codeSampleDiagnosticResult, fixedSource);
43+
}
44+
45+
[Fact]
46+
public async Task CodeFix_IPAddressAsLocalVariable()
47+
{
48+
var source = GetKestrelSetupSource("IPAddress.Any", extraInlineCode: "var myIp = IPAddress.Any;");
49+
var fixedSource = GetCorrectedKestrelSetup(extraInlineCode: "var myIp = IPAddress.Any;");
50+
await VerifyCS.VerifyCodeFixAsync(source, codeSampleDiagnosticResult, fixedSource);
51+
}
52+
53+
private static DiagnosticResult codeSampleDiagnosticResult
54+
= new DiagnosticResult(DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny).WithLocation(0);
55+
56+
static string GetKestrelSetupSource(string ipAddressArgument, string extraInlineCode = null, string extraOuterCode = null)
57+
=> GetCodeSample($$"""Listen({|#0:{{ipAddressArgument}}|}, """, extraInlineCode, extraOuterCode);
58+
59+
static string GetCorrectedKestrelSetup(string extraInlineCode = null, string extraOuterCode = null)
60+
=> GetCodeSample("ListenAnyIP(", extraInlineCode, extraOuterCode);
61+
62+
static string GetCodeSample(string invocation, string extraInlineCode = null, string extraOuterCode = null) => $$"""
63+
using Microsoft.Extensions.Hosting;
64+
using Microsoft.AspNetCore.Hosting;
65+
using Microsoft.AspNetCore.Server.Kestrel.Core;
66+
using System.Net;
67+
68+
{{extraOuterCode}}
69+
70+
var hostBuilder = new HostBuilder()
71+
.ConfigureWebHost(webHost =>
72+
{
73+
webHost.UseKestrel().ConfigureKestrel(options =>
74+
{
75+
{{extraInlineCode}}
76+
77+
options.ListenLocalhost(5000);
78+
options.ListenAnyIP(5000);
79+
options.{{invocation}}5000, listenOptions =>
80+
{
81+
listenOptions.UseHttps();
82+
listenOptions.Protocols = HttpProtocols.Http1AndHttp2AndHttp3;
83+
});
84+
});
85+
});
86+
87+
var host = hostBuilder.Build();
88+
host.Run();
89+
""";
90+
}

‎src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpAnalyzerVerifier.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Microsoft.CodeAnalysis.Testing;
1212
using Microsoft.CodeAnalysis.Testing.Verifiers;
1313
using Microsoft.Extensions.DependencyInjection;
14+
using Microsoft.AspNetCore.Hosting;
1415

1516
namespace Microsoft.AspNetCore.Analyzers.Verifiers;
1617

@@ -63,10 +64,12 @@ internal static ReferenceAssemblies GetReferenceAssemblies()
6364

6465
return net10Ref.AddAssemblies(ImmutableArray.Create(
6566
TrimAssemblyExtension(typeof(System.IO.Pipelines.PipeReader).Assembly.Location),
67+
TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer).Assembly.Location),
6668
TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Authorization.IAuthorizeData).Assembly.Location),
6769
TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Mvc.ModelBinding.IBinderTypeProviderMetadata).Assembly.Location),
6870
TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Mvc.BindAttribute).Assembly.Location),
6971
TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Hosting.WebHostBuilderExtensions).Assembly.Location),
72+
TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Hosting.WebHostBuilderKestrelExtensions).Assembly.Location),
7073
TrimAssemblyExtension(typeof(Microsoft.Extensions.Hosting.IHostBuilder).Assembly.Location),
7174
TrimAssemblyExtension(typeof(Microsoft.Extensions.Hosting.HostingHostBuilderExtensions).Assembly.Location),
7275
TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Builder.ConfigureHostBuilder).Assembly.Location),

0 commit comments

Comments
 (0)