diff --git a/docs/list-of-diagnostics.md b/docs/list-of-diagnostics.md index 64bbc9f53655..58874a8de585 100644 --- a/docs/list-of-diagnostics.md +++ b/docs/list-of-diagnostics.md @@ -30,6 +30,10 @@ | __`ASP0022`__ | Route conflict detected between route handlers | | __`ASP0023`__ | Route conflict detected between controller actions | | __`ASP0024`__ | Route handler has multiple parameters with the [FromBody] attribute | +| __`ASP0025`__ | Use AddAuthorizationBuilder | +| __`ASP0026`__ | [Authorize] overridden by [AllowAnonymous] from farther away | +| __`ASP0027`__ | Unnecessary public Program class declaration | +| __`ASP0028`__ | Consider using ListenAnyIP() instead of Listen(IPAddress.Any) | ### API (`API1000-API1003`) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs index 82fa924407df..a9ec1e603486 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs @@ -233,4 +233,13 @@ internal static class DiagnosticDescriptors isEnabledByDefault: true, helpLinkUri: "https://aka.ms/aspnet/analyzers", customTags: WellKnownDiagnosticTags.Unnecessary); + + internal static readonly DiagnosticDescriptor KestrelShouldListenOnIPv6AnyInsteadOfIpAny = new( + "ASP0028", + new LocalizableResourceString(nameof(Resources.Analyzer_KestrelShouldListenOnIPv6AnyInsteadOfIpAny_Title), Resources.ResourceManager, typeof(Resources)), + new LocalizableResourceString(nameof(Resources.Analyzer_KestrelShouldListenOnIPv6AnyInsteadOfIpAny_Message), Resources.ResourceManager, typeof(Resources)), + "Usage", + DiagnosticSeverity.Info, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/aspnet/analyzers"); } diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Kestrel/ListenOnIPv6AnyAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Kestrel/ListenOnIPv6AnyAnalyzer.cs new file mode 100644 index 000000000000..63fe4134e9b4 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Kestrel/ListenOnIPv6AnyAnalyzer.cs @@ -0,0 +1,143 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using System.Linq; + +namespace Microsoft.AspNetCore.Analyzers.Kestrel; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class ListenOnIPv6AnyAnalyzer : DiagnosticAnalyzer +{ + public override ImmutableArray SupportedDiagnostics => [ DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny ]; + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterSyntaxNodeAction(KestrelServerOptionsListenInvocation, SyntaxKind.InvocationExpression); + } + + private void KestrelServerOptionsListenInvocation(SyntaxNodeAnalysisContext context) + { + // fail fast before accessing SemanticModel + if (context.Node is not InvocationExpressionSyntax + { + Expression: MemberAccessExpressionSyntax + { + Name: IdentifierNameSyntax { Identifier.ValueText: "Listen" } + } + } kestrelOptionsListenExpressionSyntax) + { + return; + } + + var nodeOperation = context.SemanticModel.GetOperation(context.Node, context.CancellationToken); + if (!IsKestrelServerOptionsType(nodeOperation, out var kestrelOptionsListenInvocation)) + { + return; + } + + var addressArgument = kestrelOptionsListenInvocation?.Arguments.FirstOrDefault(); + if (!IsIPAddressType(addressArgument?.Parameter)) + { + return; + } + + var args = kestrelOptionsListenExpressionSyntax.ArgumentList; + var ipAddressArgumentSyntax = args.Arguments.FirstOrDefault(); + if (ipAddressArgumentSyntax is null) + { + return; + } + + // explicit usage like `options.Listen(IPAddress.Any, ...)` + if (ipAddressArgumentSyntax is ArgumentSyntax + { + Expression: MemberAccessExpressionSyntax + { + Name: IdentifierNameSyntax { Identifier.ValueText: "Any" } + } + }) + { + context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny, ipAddressArgumentSyntax.GetLocation())); + } + + // usage via local variable like + // ``` + // var myIp = IPAddress.Any; + // options.Listen(myIp, ...); + // ``` + if (addressArgument!.Value is ILocalReferenceOperation localReferenceOperation) + { + var localVariableDeclaration = localReferenceOperation.Local.DeclaringSyntaxReferences.FirstOrDefault(); + if (localVariableDeclaration is null) + { + return; + } + + var localVarSyntax = localVariableDeclaration.GetSyntax(context.CancellationToken); + if (localVarSyntax is VariableDeclaratorSyntax + { + Initializer.Value: MemberAccessExpressionSyntax + { + Name.Identifier.ValueText: "Any" + } + }) + { + context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny, ipAddressArgumentSyntax.GetLocation())); + } + } + } + + private static bool IsIPAddressType(IParameterSymbol? parameter) => parameter is + { + Type: // searching type `System.Net.IPAddress` + { + Name: "IPAddress", + ContainingNamespace: { Name: "Net", ContainingNamespace: { Name: "System", ContainingNamespace.IsGlobalNamespace: true } } + } + }; + + private static bool IsKestrelServerOptionsType(IOperation? operation, out IInvocationOperation? kestrelOptionsListenInvocation) + { + var result = operation is IInvocationOperation // searching type `Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions` + { + TargetMethod: { Name: "Listen" }, + Instance.Type: + { + Name: "KestrelServerOptions", + ContainingNamespace: + { + Name: "Core", + ContainingNamespace: + { + Name: "Kestrel", + ContainingNamespace: + { + Name: "Server", + ContainingNamespace: + { + Name: "AspNetCore", + ContainingNamespace: + { + Name: "Microsoft", + ContainingNamespace.IsGlobalNamespace: true + } + } + } + } + } + } + }; + + kestrelOptionsListenInvocation = result ? (IInvocationOperation)operation! : null; + return result; + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx index 075bcb700452..8c9397f5be64 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx @@ -327,4 +327,10 @@ Unnecessary public Program class declaration + + Consider using ListenAnyIP() instead of Listen(IPAddress.Any) + + + 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. + diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Kestrel/ListenOnIPv6AnyFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Kestrel/ListenOnIPv6AnyFixer.cs new file mode 100644 index 000000000000..207d80e9c6e0 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Kestrel/ListenOnIPv6AnyFixer.cs @@ -0,0 +1,79 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Composition; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis; +using System.Collections.Immutable; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Analyzers; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.CSharp; + +namespace Microsoft.AspNetCore.Fixers.Kestrel; + +[ExportCodeFixProvider(LanguageNames.CSharp), Shared] +public class ListenOnIPv6AnyFixer : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds => [ DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny.Id ]; + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override Task RegisterCodeFixesAsync(CodeFixContext context) + { + foreach (var diagnostic in context.Diagnostics) + { + context.RegisterCodeFix( + CodeAction.Create( + "Consider using IPAddress.IPv6Any instead of IPAddress.Any", + async cancellationToken => + { + var editor = await DocumentEditor.CreateAsync(context.Document, cancellationToken).ConfigureAwait(false); + var root = await context.Document.GetSyntaxRootAsync(cancellationToken); + if (root is null) + { + return context.Document; + } + + var argumentSyntax = root.FindNode(diagnostic.Location.SourceSpan).FirstAncestorOrSelf(); + if (argumentSyntax is null) + { + return context.Document; + } + + // get to the `Listen(IPAddress.Any, ...)` invocation + if (argumentSyntax.Parent?.Parent is not InvocationExpressionSyntax { ArgumentList.Arguments.Count: > 1 } invocationExpressionSyntax) + { + return context.Document; + } + if (invocationExpressionSyntax.Expression is not MemberAccessExpressionSyntax memberAccessExpressionSyntax) + { + return context.Document; + } + + var instanceVariableInvoked = memberAccessExpressionSyntax.Expression; + var adjustedArgumentList = invocationExpressionSyntax.ArgumentList.RemoveNode(invocationExpressionSyntax.ArgumentList.Arguments.First(), SyntaxRemoveOptions.KeepLeadingTrivia); + if (adjustedArgumentList is null || adjustedArgumentList.Arguments.Count == 0) + { + return context.Document; + } + + // changing invocation from `.Listen(IPAddress.Any, ...)` to `.ListenAnyIP(...)` + editor.ReplaceNode( + invocationExpressionSyntax, + invocationExpressionSyntax + .WithExpression(SyntaxFactory.ParseExpression($"{instanceVariableInvoked.ToString()}.ListenAnyIP")) + .WithArgumentList(adjustedArgumentList!) + .WithLeadingTrivia(invocationExpressionSyntax.GetLeadingTrivia()) + ); + return editor.GetChangedDocument(); + }, + equivalenceKey: DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny.Id), + diagnostic); + } + + return Task.CompletedTask; + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/test/Kestrel/ListenOnIPv6AnyAnalyzerAndFixerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Kestrel/ListenOnIPv6AnyAnalyzerAndFixerTests.cs new file mode 100644 index 000000000000..af0cbd4c307e --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Kestrel/ListenOnIPv6AnyAnalyzerAndFixerTests.cs @@ -0,0 +1,90 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Text; +using Microsoft.CodeAnalysis.Testing; +using VerifyCS = Microsoft.AspNetCore.Analyzers.Verifiers.CSharpCodeFixVerifier< + Microsoft.AspNetCore.Analyzers.Kestrel.ListenOnIPv6AnyAnalyzer, + Microsoft.AspNetCore.Fixers.Kestrel.ListenOnIPv6AnyFixer>; + +namespace Microsoft.AspNetCore.Analyzers.Kestrel; + +public class ListenOnIPv6AnyAnalyzerAndFixerTests +{ + [Fact] + public async Task ReportsDiagnostic_IPAddressAsLocalVariable_OuterScope() + { + var source = GetKestrelSetupSource("myIp", extraOuterCode: "var myIp = IPAddress.Any;"); + await VerifyCS.VerifyAnalyzerAsync(source, codeSampleDiagnosticResult); + } + + [Fact] + public async Task ReportsDiagnostic_IPAddressAsLocalVariable() + { + var source = GetKestrelSetupSource("myIp", extraInlineCode: "var myIp = IPAddress.Any;"); + await VerifyCS.VerifyAnalyzerAsync(source, codeSampleDiagnosticResult); + } + + [Fact] + public async Task ReportsDiagnostic_ExplicitUsage() + { + var source = GetKestrelSetupSource("IPAddress.Any"); + await VerifyCS.VerifyAnalyzerAsync(source, codeSampleDiagnosticResult); + } + + [Fact] + public async Task CodeFix_ExplicitUsage() + { + var source = GetKestrelSetupSource("IPAddress.Any"); + var fixedSource = GetCorrectedKestrelSetup(); + await VerifyCS.VerifyCodeFixAsync(source, codeSampleDiagnosticResult, fixedSource); + } + + [Fact] + public async Task CodeFix_IPAddressAsLocalVariable() + { + var source = GetKestrelSetupSource("IPAddress.Any", extraInlineCode: "var myIp = IPAddress.Any;"); + var fixedSource = GetCorrectedKestrelSetup(extraInlineCode: "var myIp = IPAddress.Any;"); + await VerifyCS.VerifyCodeFixAsync(source, codeSampleDiagnosticResult, fixedSource); + } + + private static DiagnosticResult codeSampleDiagnosticResult + = new DiagnosticResult(DiagnosticDescriptors.KestrelShouldListenOnIPv6AnyInsteadOfIpAny).WithLocation(0); + + static string GetKestrelSetupSource(string ipAddressArgument, string extraInlineCode = null, string extraOuterCode = null) + => GetCodeSample($$"""Listen({|#0:{{ipAddressArgument}}|}, """, extraInlineCode, extraOuterCode); + + static string GetCorrectedKestrelSetup(string extraInlineCode = null, string extraOuterCode = null) + => GetCodeSample("ListenAnyIP(", extraInlineCode, extraOuterCode); + + static string GetCodeSample(string invocation, string extraInlineCode = null, string extraOuterCode = null) => $$""" + using Microsoft.Extensions.Hosting; + using Microsoft.AspNetCore.Hosting; + using Microsoft.AspNetCore.Server.Kestrel.Core; + using System.Net; + + {{extraOuterCode}} + + var hostBuilder = new HostBuilder() + .ConfigureWebHost(webHost => + { + webHost.UseKestrel().ConfigureKestrel(options => + { + {{extraInlineCode}} + + options.ListenLocalhost(5000); + options.ListenAnyIP(5000); + options.{{invocation}}5000, listenOptions => + { + listenOptions.UseHttps(); + listenOptions.Protocols = HttpProtocols.Http1AndHttp2AndHttp3; + }); + }); + }); + + var host = hostBuilder.Build(); + host.Run(); + """; +} diff --git a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpAnalyzerVerifier.cs b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpAnalyzerVerifier.cs index 325b0c230955..aa3ee600db1e 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpAnalyzerVerifier.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpAnalyzerVerifier.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Testing.Verifiers; using Microsoft.Extensions.DependencyInjection; +using Microsoft.AspNetCore.Hosting; namespace Microsoft.AspNetCore.Analyzers.Verifiers; @@ -63,10 +64,12 @@ internal static ReferenceAssemblies GetReferenceAssemblies() return net10Ref.AddAssemblies(ImmutableArray.Create( TrimAssemblyExtension(typeof(System.IO.Pipelines.PipeReader).Assembly.Location), + TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Authorization.IAuthorizeData).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Mvc.ModelBinding.IBinderTypeProviderMetadata).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Mvc.BindAttribute).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Hosting.WebHostBuilderExtensions).Assembly.Location), + TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Hosting.WebHostBuilderKestrelExtensions).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.Extensions.Hosting.IHostBuilder).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.Extensions.Hosting.HostingHostBuilderExtensions).Assembly.Location), TrimAssemblyExtension(typeof(Microsoft.AspNetCore.Builder.ConfigureHostBuilder).Assembly.Location),