Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix S1144 FP: Ignore Serializable members #8936

Merged

Conversation

zsolt-kolbay-sonarsource
Copy link
Contributor

Fixes #8532

@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource force-pushed the Zsolt/fix-S1144-FP-Serializable-members branch 3 times, most recently from 0f39ac8 to 072a90a Compare March 21, 2024 10:30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a question.
Also, Tim suggests on the issue to update the RSPEC.
Please take care of it and feel free to ask me for review.

{
"Id": "S4487",
"Message": "Remove this unread private field \u0027name\u0027 or refactor the code to use its value.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/ManuallyAddedNoncompliantIssues.CS/NetFramework48/InsecureDeserialization.cs#L50",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't this a valid issue? I don't understand why it disappeared.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks correct to me. S4487 and S1144 share the same analyzer class.
The example shows a class that has a private member that can be initialized through the constructor. Then the class can be serialized, so the private member is actually used/read. So I consider this a former FP.

@@ -19,6 +19,7 @@
*/

using SonarAnalyzer.Common.Walkers;
using SonarAnalyzer.Json.Parsing;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource force-pushed the Zsolt/fix-S1144-FP-Serializable-members branch from 4e12349 to e4a3c0b Compare March 22, 2024 17:05
Copy link

sonarcloud bot commented Mar 22, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Mar 22, 2024

@mary-georgiou-sonarsource mary-georgiou-sonarsource merged commit ae1601a into master Mar 28, 2024
25 checks passed
@mary-georgiou-sonarsource mary-georgiou-sonarsource deleted the Zsolt/fix-S1144-FP-Serializable-members branch March 28, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix S1144 FP/FN: Do not raise on serializable members
4 participants