Browse Source

feat: analyzer and codeFix for kestrel setup `ListenOptions.Listen(IPAddress.Any)` usage (#58872)

Korolev Dmitry 1 year ago
parent
commit
5c6839e3a0

+ 4 - 0
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`)
 

+ 9 - 0
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");
 }

+ 143 - 0
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<DiagnosticDescriptor> 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;
+    }
+}

+ 6 - 0
src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx

@@ -327,4 +327,10 @@
   <data name="Analyzer_PublicPartialProgramClass_Title" xml:space="preserve">
     <value>Unnecessary public Program class declaration</value>
   </data>
+  <data name="Analyzer_KestrelShouldListenOnIPv6AnyInsteadOfIpAny_Title" xml:space="preserve">
+    <value>Consider using ListenAnyIP() instead of Listen(IPAddress.Any)</value>
+  </data>
+  <data name="Analyzer_KestrelShouldListenOnIPv6AnyInsteadOfIpAny_Message" xml:space="preserve">
+    <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>
+  </data>
 </root>

+ 79 - 0
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<string> 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<ArgumentSyntax>();
+                        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 `<variable>.Listen(IPAddress.Any, ...)` to `<variable>.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;
+    }
+}

+ 90 - 0
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();
+    """;
+}

+ 3 - 0
src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpAnalyzerVerifier.cs

@@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.Diagnostics;
 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 @@ public static partial class CSharpAnalyzerVerifier<TAnalyzer>
 
         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),