diff --git a/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6964-IntentionalFindings-net8.0.json b/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6964-IntentionalFindings-net8.0.json
index 05a624fc5d2..feb91c9503b 100644
--- a/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6964-IntentionalFindings-net8.0.json
+++ b/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6964-IntentionalFindings-net8.0.json
@@ -2,9 +2,15 @@
"Issues": [
{
"Id": "S6964",
- "Message": "Property used as input in a controller action should be nullable or annotated with the Required attribute to avoid under-posting.",
+ "Message": "Value type property used as input in a controller action should be nullable, required or annotated with the JsonRequiredAttribute to avoid under-posting.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6964.cs#L9",
"Location": "Line 9 Position 24-37"
+ },
+ {
+ "Id": "S6964",
+ "Message": "Value type property used as input in a controller action should be nullable, required or annotated with the JsonRequiredAttribute to avoid under-posting.",
+ "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S6967.cs#L12",
+ "Location": "Line 12 Position 35-37"
}
]
}
\ No newline at end of file
diff --git a/analyzers/rspec/cs/S6964.html b/analyzers/rspec/cs/S6964.html
index e98a4cc4844..026d6cbcdc2 100644
--- a/analyzers/rspec/cs/S6964.html
+++ b/analyzers/rspec/cs/S6964.html
@@ -20,41 +20,45 @@
Why is this an issue?
}
Exceptions
+This rule does not raise an issue when:
- - This rule does not raise an issue for properties decorated with the properties are decorated with the ValidateNever
attribute.
+ - properties are in model classes which are not in the same project as the Controller class that references them. This is due to a limitation of
+ Roslyn (see here).
How to fix it
You should mark any model value-type property as nullable or annotate it with the Required attribute. Thus, when a
-client underposts, you ensure that the missing properties can be detected on the server side rather than being auto-filled, and therefore, incoming
-data meets the application’s expectations.
+href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/nullable-value-types">nullable, required or JsonRequired. Thus when a client
+underposts, you ensure that the missing properties can be detected on the server side rather than being auto-filled, and therefore, incoming data
+meets the application’s expectations.
Code examples
Noncompliant code example
public class Product
{
- public int Id { get; set; }
+ public int Id { get; set; } // Noncompliant
public string Name { get; set; }
public int NumberOfItems { get; set; } // Noncompliant
public decimal Price { get; set; } // Noncompliant
}
If the client sends a request without setting the NumberOfItems or Price properties, they will default to 0.
-In the request handler method there’s no way to determine whether they were intentionally set to 0 or omitted by mistake.
+In the request handler method, there’s no way to determine whether they were intentionally set to 0 or omitted by mistake.
Compliant solution
public class Product
{
- public int Id { get; set; }
+ public required int Id { get; set; }
public string Name { get; set; }
- public int? NumberOfItems { get; set; } // Compliant - property is optional
- [Required] public decimal Price { get; set; } // Compliant - property must have a value
+ public int? NumberOfItems { get; set; } // Compliant - property is optional
+ [JsonRequired] public decimal Price { get; set; } // Compliant - property must have a value
}
-In this example the request handler method can
+In this example, the request handler method can
- manually check whether
NumberOfItems was filled out through the HasValue property
- rely on Model Validation to make sure
Price is not missing
diff --git a/analyzers/rspec/cs/S6964.json b/analyzers/rspec/cs/S6964.json
index 8356b79c1e1..a7d98b5cdc5 100644
--- a/analyzers/rspec/cs/S6964.json
+++ b/analyzers/rspec/cs/S6964.json
@@ -1,5 +1,5 @@
{
- "title": "Property used as input in a controller action should be nullable or annotated with the Required attribute to avoid under-posting.",
+ "title": "Value type property used as input in a controller action should be nullable, required or annotated with the JsonRequiredAttribute to avoid under-posting.",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs
index 6f71be7024a..e3ded471da3 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AspNet/AvoidUnderPosting.cs
@@ -26,16 +26,13 @@ namespace SonarAnalyzer.Rules.CSharp;
public sealed class AvoidUnderPosting : SonarDiagnosticAnalyzer
{
private const string DiagnosticId = "S6964";
- private const string MessageFormat = "Property used as input in a controller action should be nullable or annotated with the Required attribute to avoid under-posting.";
+ private const string MessageFormat = "Value type property used as input in a controller action should be nullable, required or annotated with the JsonRequiredAttribute to avoid under-posting.";
private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
private static readonly ImmutableArray IgnoredTypes = ImmutableArray.Create(
KnownType.Microsoft_AspNetCore_Http_IFormCollection,
KnownType.Microsoft_AspNetCore_Http_IFormFile,
KnownType.Microsoft_AspNetCore_Http_IFormFileCollection);
- private static readonly ImmutableArray ValidationAttributes = ImmutableArray.Create(
- KnownType.System_ComponentModel_DataAnnotations_RequiredAttribute,
- KnownType.System_Text_Json_Serialization_JsonRequiredAttribute);
public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule);
@@ -79,7 +76,7 @@ private static void CheckInvalidProperties(INamedTypeSymbol parameterType, Sonar
var declaredProperties = new List();
GetAllDeclaredProperties(parameterType, examinedTypes, declaredProperties);
var invalidProperties = declaredProperties
- .Where(x => !CanBeNull(x.Type) && !x.HasAnyAttribute(ValidationAttributes) && !x.IsRequired());
+ .Where(x => !CanBeNull(x.Type) && !x.HasAttribute(KnownType.System_Text_Json_Serialization_JsonRequiredAttribute) && !x.IsRequired());
foreach (var property in invalidProperties)
{
context.ReportIssue(Rule, property.GetFirstSyntaxRef().GetIdentifier()?.GetLocation());
diff --git a/analyzers/src/SonarAnalyzer.Common/Extensions/ISymbolExtensions.cs b/analyzers/src/SonarAnalyzer.Common/Extensions/ISymbolExtensions.cs
index 77e45512704..b0dbb0c6130 100644
--- a/analyzers/src/SonarAnalyzer.Common/Extensions/ISymbolExtensions.cs
+++ b/analyzers/src/SonarAnalyzer.Common/Extensions/ISymbolExtensions.cs
@@ -27,9 +27,6 @@ public static class ISymbolExtensions
public static bool HasAttribute(this ISymbol symbol, KnownType type) =>
symbol.GetAttributes(type).Any();
- public static bool HasAnyAttribute(this ISymbol symbol, ImmutableArray types) =>
- symbol.GetAttributes(types).Any();
-
public static SyntaxNode GetFirstSyntaxRef(this ISymbol symbol) =>
symbol?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/AvoidUnderPostingTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/AvoidUnderPostingTest.cs
index 480828c7904..71c3200ff68 100644
--- a/analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/AvoidUnderPostingTest.cs
+++ b/analyzers/tests/SonarAnalyzer.Test/Rules/AspNet/AvoidUnderPostingTest.cs
@@ -35,7 +35,7 @@ public class AvoidUnderPostingTest
private readonly VerifierBuilder builder = new VerifierBuilder()
.WithBasePath("AspNet")
- .AddReferences(AspNetReferences);
+ .AddReferences([..AspNetReferences, .. NuGetMetadataReference.SystemTextJson("7.0.4")]);
[TestMethod]
public void AvoidUnderPosting_CSharp() =>
@@ -56,7 +56,6 @@ public void AvoidUnderPosting_CSharp9() =>
[TestMethod]
public void AvoidUnderPosting_CSharp12() =>
builder.AddPaths("AvoidUnderPosting.CSharp12.cs")
- .AddReferences(NuGetMetadataReference.SystemTextJson("7.0.4"))
.WithOptions(ParseOptionsHelper.FromCSharp12)
.Verify();
@@ -74,7 +73,7 @@ public void AvoidUnderPosting_EnclosingTypes_CSharp(string enclosingType) =>
{
public int ValueProperty { get; set; } // Noncompliant
public int? NullableValueProperty { get; set; } // Compliant
- [Required] public int RequiredValueProperty { get; set; } // Compliant
+ public required int RequiredValueProperty { get; set; } // Compliant
}
public class ControllerClass : Controller
@@ -82,7 +81,7 @@ public class ControllerClass : Controller
[HttpPost] public IActionResult Create(Model model) => View(model);
}
""")
- .WithOptions(ParseOptionsHelper.FromCSharp10)
+ .WithOptions(ParseOptionsHelper.FromCSharp11)
.Verify();
[DataTestMethod]
diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp8.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp8.cs
index acdab62faa6..3d510d027f4 100644
--- a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp8.cs
+++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp8.cs
@@ -1,13 +1,14 @@
using Microsoft.AspNetCore.Mvc;
using System;
using System.ComponentModel.DataAnnotations;
+using System.Text.Json.Serialization;
namespace NullableReferences
{
public class ModelUsedInController
{
#nullable enable
- public string NonNullableReferenceProperty { get; set; } // Compliant - ASP.NET Core treats non-nullable reference types as if they were decorated with the [Required] attribute
+ public string NonNullableReferenceProperty { get; set; } // Compliant - the JSON serializer will throw an exception if the value is missing from the request
public object AnotherNonNullableReferenceProperty { get; set; }
[Required] public string RequiredNonNullableReferenceProperty { get; set; }
public string? NullableReferenceProperty { get; set; }
@@ -39,7 +40,7 @@ public class GenericType
public TNoContstraint NoConstraintProperty { get; set; }
public TClass ClassProperty { get; set; }
public TStruct StructProperty { get; set; } // Noncompliant
- [Required] public TStruct RequiredStructProperty { get; set; }
+ [JsonRequired] public TStruct RequiredStructProperty { get; set; }
public TStruct? NullableStructProperty { get; set; }
public TNotNull NotNullProperty { get; set; } // Noncompliant
}
diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp9.cs
index e0ba0e8ceff..25d984a5cbb 100644
--- a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp9.cs
+++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.CSharp9.cs
@@ -1,12 +1,13 @@
using Microsoft.AspNetCore.Mvc;
using System.ComponentModel.DataAnnotations;
+using System.Text.Json.Serialization;
namespace CSharp9
{
// https://learn.microsoft.com/en-us/aspnet/core/mvc/models/model-binding#constructor-binding-and-record-types
public record RecordModel(
int ValueProperty, // Noncompliant
- [property: Required] int RequiredValueProperty, // without the property prefix the attribute would have been applied to the constructor parameter instead
+ [property: JsonRequired] int RequiredValueProperty, // without the property prefix the attribute would have been applied to the constructor parameter instead
int? NullableValueProperty);
public class ModelUsedInController
diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.cs
index 237aed24ceb..6947fe1feea 100644
--- a/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.cs
+++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/AspNet/AvoidUnderPosting.cs
@@ -13,10 +13,10 @@ public class ClassNotUsedInRequests
public class ModelUsedInController
{
- public int ValueProperty { get; set; } // Noncompliant {{Property used as input in a controller action should be nullable or annotated with the Required attribute to avoid under-posting.}}
+ public int ValueProperty { get; set; } // Noncompliant {{Value type property used as input in a controller action should be nullable, required or annotated with the JsonRequiredAttribute to avoid under-posting.}}
// ^^^^^^^^^^^^^
public int? NullableValueProperty { get; set; }
- [Required] public int RequiredValueProperty { get; set; }
+ [Required] public int RequiredValueProperty { get; set; } // Noncompliant, RequiredAttribute has no effect on value types
[Range(0, 10)] public int ValuePropertyWithRangeValidation { get; set; } // Noncompliant
[Required] public int? RequiredNullableValueProperty { get; set; }
public int PropertyWithPrivateSetter { get; private set; }