Skip to content

Commit

Permalink
Add CA2022: Avoid inexact read with 'Stream.Read' (#7208)
Browse files Browse the repository at this point in the history
* Add CA2022: Avoid unreliable call to 'Stream.Read'

This analyzer detects calls to 'Stream.Read' or 'Stream.ReadAsync' that
do not check the return value. These methods may return fewer bytes than
requested, resulting in unreliable code.

The fixer will recommend replacing the call with 'Stream.ReadExactly' or
'Stream.ReadExactlyAsync', if available.

* Simplify ReadExactly availability check

* Improve analyzer title and message
  • Loading branch information
mpidash authored Mar 26, 2024
1 parent 524e3b7 commit 9022e53
Show file tree
Hide file tree
Showing 22 changed files with 1,409 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ CA1514 | Maintainability | Info | AvoidLengthCheckWhenSlicingToEndAnalyzer, [Doc
CA1515 | Maintainability | Disabled | MakeTypesInternal, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1515)
CA1871 | Performance | Info | DoNotPassNonNullableValueToArgumentNullExceptionThrowIfNull, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1871)
CA1872 | Performance | Info | PreferConvertToHexStringOverBitConverterAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1872)
CA2022 | Reliability | Warning | AvoidUnreliableStreamReadAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2022)
CA2262 | Usage | Info | ProvideHttpClientHandlerMaxResponseHeaderLengthValueCorrectly, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2262)
CA2263 | Usage | Info | PreferGenericOverloadsAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2263)
CA2264 | Usage | Warning | DoNotPassNonNullableValueToArgumentNullExceptionThrowIfNull, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2264)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,18 @@
<value>Prefer 'static readonly' fields over constant array arguments if the called method is called repeatedly and is not mutating the passed array</value>
<comment>{Locked="static readonly"}</comment>
</data>
<data name="AvoidUnreliableStreamReadTitle" xml:space="preserve">
<value>Avoid inexact read with 'Stream.Read'</value>
</data>
<data name="AvoidUnreliableStreamReadCodeFixTitle" xml:space="preserve">
<value>Use 'Stream.ReadExactly'</value>
</data>
<data name="AvoidUnreliableStreamReadDescription" xml:space="preserve">
<value>A call to 'Stream.Read' may return fewer bytes than requested, resulting in unreliable code if the return value is not checked.</value>
</data>
<data name="AvoidUnreliableStreamReadMessage" xml:space="preserve">
<value>Avoid inexact read with '{0}'</value>
</data>
<data name="TestForEmptyStringsUsingStringLengthTitle" xml:space="preserve">
<value>Test for empty strings using string length</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.NetCore.Analyzers.Runtime
{
using static MicrosoftNetCoreAnalyzersResources;

/// <summary>
/// CA2022: <inheritdoc cref="AvoidUnreliableStreamReadTitle"/>
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared]
public sealed class AvoidUnreliableStreamReadFixer : CodeFixProvider
{
private const string Async = nameof(Async);
private const string ReadExactly = nameof(ReadExactly);
private const string ReadExactlyAsync = nameof(ReadExactlyAsync);

public sealed override ImmutableArray<string> FixableDiagnosticIds { get; } =
ImmutableArray.Create(AvoidUnreliableStreamReadAnalyzer.RuleId);

public sealed override FixAllProvider GetFixAllProvider()
{
return WellKnownFixAllProviders.BatchFixer;
}

public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
var node = root.FindNode(context.Span, getInnermostNodeForTie: true);

if (node is null)
{
return;
}

var semanticModel = await context.Document.GetRequiredSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
var operation = semanticModel.GetOperation(node, context.CancellationToken);

if (operation is not IInvocationOperation invocation ||
invocation.Instance is null)
{
return;
}

var compilation = semanticModel.Compilation;
var streamType = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemIOStream);

if (streamType is null)
{
return;
}

var readExactlyMethods = streamType.GetMembers(ReadExactly)
.OfType<IMethodSymbol>()
.ToImmutableArray();

if (readExactlyMethods.IsEmpty)
{
return;
}

var codeAction = CodeAction.Create(
AvoidUnreliableStreamReadCodeFixTitle,
ct => ReplaceWithReadExactlyCall(context.Document, ct),
nameof(AvoidUnreliableStreamReadCodeFixTitle));

context.RegisterCodeFix(codeAction, context.Diagnostics);

async Task<Document> ReplaceWithReadExactlyCall(Document document, CancellationToken cancellationToken)
{
var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
var generator = editor.Generator;
var arguments = invocation.Arguments.GetArgumentsInParameterOrder();

var isAsyncInvocation = invocation.TargetMethod.Name.EndsWith(Async, StringComparison.Ordinal);
var methodExpression = generator.MemberAccessExpression(
invocation.Instance.Syntax,
isAsyncInvocation ? ReadExactlyAsync : ReadExactly);
var methodInvocation = CanUseSpanOverload()
? generator.InvocationExpression(
methodExpression,
isAsyncInvocation && arguments.Length == 4
// Stream.ReadExactlyAsync(buffer, ct)
?[arguments[0].Syntax, arguments[3].Syntax]
// Stream.ReadExactly(buffer) and Stream.ReadExactlyAsync(buffer)
:[arguments[0].Syntax])
: generator.InvocationExpression(
methodExpression,
invocation.Arguments.Where(a => !a.IsImplicit).Select(a => a.Syntax));

editor.ReplaceNode(invocation.Syntax, methodInvocation.WithTriviaFrom(invocation.Syntax));

return document.WithSyntaxRoot(editor.GetChangedRoot());

bool CanUseSpanOverload()
{
return arguments.Length >= 3 &&
arguments[2].Value is IPropertyReferenceOperation propertyRef &&
propertyRef.Property.Name.Equals(WellKnownMemberNames.LengthPropertyName, StringComparison.Ordinal) &&
AreSameInstance(arguments[0].Value, propertyRef.Instance);
}

static bool AreSameInstance(IOperation? operation1, IOperation? operation2)
{
return (operation1, operation2) switch
{
(IFieldReferenceOperation fieldRef1, IFieldReferenceOperation fieldRef2) => fieldRef1.Member == fieldRef2.Member,
(IPropertyReferenceOperation propRef1, IPropertyReferenceOperation propRef2) => propRef1.Member == propRef2.Member,
(IParameterReferenceOperation paramRef1, IParameterReferenceOperation paramRef2) => paramRef1.Parameter == paramRef2.Parameter,
(ILocalReferenceOperation localRef1, ILocalReferenceOperation localRef2) => localRef1.Local == localRef2.Local,
_ => false,
};
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.NetCore.Analyzers.Runtime
{
using static MicrosoftNetCoreAnalyzersResources;

/// <summary>
/// CA2022: <inheritdoc cref="AvoidUnreliableStreamReadTitle"/>
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class AvoidUnreliableStreamReadAnalyzer : DiagnosticAnalyzer
{
internal const string RuleId = "CA2022";

private const string Read = nameof(Read);
private const string ReadAsync = nameof(ReadAsync);

internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(AvoidUnreliableStreamReadTitle)),
CreateLocalizableResourceString(nameof(AvoidUnreliableStreamReadMessage)),
DiagnosticCategory.Reliability,
RuleLevel.BuildWarning,
CreateLocalizableResourceString(nameof(AvoidUnreliableStreamReadDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

public sealed override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

public sealed override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(OnCompilationStart);
}

private void OnCompilationStart(CompilationStartAnalysisContext context)
{
if (!RequiredSymbols.TryGetSymbols(context.Compilation, out var symbols))
{
return;
}

context.RegisterOperationAction(AnalyzeInvocation, OperationKind.Invocation);

void AnalyzeInvocation(OperationAnalysisContext context)
{
var invocation = (IInvocationOperation)context.Operation;

if (symbols.IsAnyStreamReadMethod(invocation.TargetMethod))
{
if (invocation.Parent is not IExpressionStatementOperation)
{
return;
}
}
else if (symbols.IsAnyStreamReadAsyncMethod(invocation.TargetMethod))
{
if (invocation.Parent is not IAwaitOperation awaitOperation ||
awaitOperation.Parent is not IExpressionStatementOperation)
{
return;
}
}
else
{
return;
}

context.ReportDiagnostic(invocation.CreateDiagnostic(Rule, invocation.TargetMethod.ToDisplayString()));
}
}

internal sealed class RequiredSymbols
{
private RequiredSymbols(
ImmutableArray<IMethodSymbol> streamReadMethods,
ImmutableArray<IMethodSymbol> streamReadAsyncMethods)
{
_streamReadMethods = streamReadMethods;
_streamReadAsyncMethods = streamReadAsyncMethods;
}

public static bool TryGetSymbols(Compilation compilation, [NotNullWhen(true)] out RequiredSymbols? symbols)
{
symbols = default;

var streamType = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemIOStream);

if (streamType is null)
{
return false;
}

var streamReadMethods = streamType.GetMembers(Read)
.OfType<IMethodSymbol>()
.ToImmutableArray();
var streamReadAsyncMethods = streamType.GetMembers(ReadAsync)
.OfType<IMethodSymbol>()
.ToImmutableArray();

if (streamReadMethods.IsEmpty && streamReadAsyncMethods.IsEmpty)
{
return false;
}

symbols = new RequiredSymbols(streamReadMethods, streamReadAsyncMethods);

return true;
}

public bool IsAnyStreamReadMethod(IMethodSymbol method)
{
return _streamReadMethods.Any(m =>
SymbolEqualityComparer.Default.Equals(method, m) || IsOverrideOf(method, m));
}

public bool IsAnyStreamReadAsyncMethod(IMethodSymbol method)
{
return _streamReadAsyncMethods.Any(m =>
SymbolEqualityComparer.Default.Equals(method, m) || IsOverrideOf(method, m));
}

private static bool IsOverrideOf(IMethodSymbol method, IMethodSymbol baseMethod)
{
var overriddenMethod = method.OverriddenMethod;
while (overriddenMethod is not null)
{
if (SymbolEqualityComparer.Default.Equals(overriddenMethod, baseMethod))
{
return true;
}

overriddenMethod = overriddenMethod.OverriddenMethod;
}

return false;
}

private readonly ImmutableArray<IMethodSymbol> _streamReadMethods;
private readonly ImmutableArray<IMethodSymbol> _streamReadAsyncMethods;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,26 @@
<target state="translated">Nepoužívejte parametry StringBuilder pro volání nespravovaného kódu</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadCodeFixTitle">
<source>Use 'Stream.ReadExactly'</source>
<target state="new">Use 'Stream.ReadExactly'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadDescription">
<source>A call to 'Stream.Read' may return fewer bytes than requested, resulting in unreliable code if the return value is not checked.</source>
<target state="new">A call to 'Stream.Read' may return fewer bytes than requested, resulting in unreliable code if the return value is not checked.</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadMessage">
<source>Avoid inexact read with '{0}'</source>
<target state="new">Avoid inexact read with '{0}'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadTitle">
<source>Avoid inexact read with 'Stream.Read'</source>
<target state="new">Avoid inexact read with 'Stream.Read'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnsealedAttributesDescription">
<source>The .NET Framework class library provides methods for retrieving custom attributes. By default, these methods search the attribute inheritance hierarchy. Sealing the attribute eliminates the search through the inheritance hierarchy and can improve performance.</source>
<target state="translated">Knihovna tříd .NET Framework poskytuje metody pro načítání vlastních atributů. Ve výchozím nastavení tyto metody prohledávají hierarchii dědičnosti atributů. Zapečetění atributu eliminuje prohledávání hierarchie dědičnosti a může zvýšit výkon.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,26 @@
<target state="translated">StringBuilder-Parameter für "P/Invokes" vermeiden</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadCodeFixTitle">
<source>Use 'Stream.ReadExactly'</source>
<target state="new">Use 'Stream.ReadExactly'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadDescription">
<source>A call to 'Stream.Read' may return fewer bytes than requested, resulting in unreliable code if the return value is not checked.</source>
<target state="new">A call to 'Stream.Read' may return fewer bytes than requested, resulting in unreliable code if the return value is not checked.</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadMessage">
<source>Avoid inexact read with '{0}'</source>
<target state="new">Avoid inexact read with '{0}'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadTitle">
<source>Avoid inexact read with 'Stream.Read'</source>
<target state="new">Avoid inexact read with 'Stream.Read'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnsealedAttributesDescription">
<source>The .NET Framework class library provides methods for retrieving custom attributes. By default, these methods search the attribute inheritance hierarchy. Sealing the attribute eliminates the search through the inheritance hierarchy and can improve performance.</source>
<target state="translated">Die .NET Framework-Klassenbibliothek stellt Methoden zum Abrufen benutzerdefinierter Attribute bereit. Standardmäßig durchsuchen diese Methoden die Attributvererbungshierarchie. Durch das Versiegeln des Attributs entfällt das Durchsuchen der Vererbungshierarchie, und die Leistung kann gesteigert werden.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,26 @@
<target state="translated">Evitar los parámetros "StringBuilder" para los elementos P/Invoke</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadCodeFixTitle">
<source>Use 'Stream.ReadExactly'</source>
<target state="new">Use 'Stream.ReadExactly'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadDescription">
<source>A call to 'Stream.Read' may return fewer bytes than requested, resulting in unreliable code if the return value is not checked.</source>
<target state="new">A call to 'Stream.Read' may return fewer bytes than requested, resulting in unreliable code if the return value is not checked.</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadMessage">
<source>Avoid inexact read with '{0}'</source>
<target state="new">Avoid inexact read with '{0}'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnreliableStreamReadTitle">
<source>Avoid inexact read with 'Stream.Read'</source>
<target state="new">Avoid inexact read with 'Stream.Read'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUnsealedAttributesDescription">
<source>The .NET Framework class library provides methods for retrieving custom attributes. By default, these methods search the attribute inheritance hierarchy. Sealing the attribute eliminates the search through the inheritance hierarchy and can improve performance.</source>
<target state="translated">La biblioteca de clases de .NET Framework proporciona los métodos para recuperar los atributos personalizados. De forma predeterminada, estos métodos buscan la jerarquía de herencia del atributo. Al sellar el atributo, se elimina la búsqueda a través de la jerarquía de herencia y puede mejorarse el rendimiento.</target>
Expand Down
Loading

0 comments on commit 9022e53

Please sign in to comment.