bitwarden
Potential errors in the project
Steps To Reproduce
Hello!
I’d like to show you several issues found in your project.
Potential error in BillingInfo.cs
The property is assigned values twice.
Below is a code fragment with this issue:
public BillingInvoice(Invoice inv)
{
Amount = inv.AmountDue / 100M; // <=
Date = inv.Created;
Url = inv.HostedInvoiceUrl;
PdfUrl = inv.InvoicePdf;
Number = inv.Number;
Paid = inv.Paid;
Amount = inv.Total / 100M; // <=
}
The result of the inv.AmountDue / 100 expression is assigned to Amout. However, there’s a similar operation five lines below but this time inv.Total / 100M is assigned.
Potential error in AppleIapService.cs.
Perhaps the '??' operator doesn’t work as expected.
Here’s this code fragment:
throw new Exception("Failed verifying Apple IAP after too many attempts. Last attempt status: " +
lastReceiptStatus?.Status ?? "null");
The '??' operator’s priority is lower than the '+' operator’s priority. Therefore, the value of the Status property is added to the string first, and after that the null-coalescing operator snaps into action.
Link to the line with error.
Potential error in FreshdeskController.cs.
Erroneous postfix notation.
The fragment with a potential error:
private async Task<HttpResponseMessage> CallFreshdeskApiAsync(...., int retriedCount = 0)
{
....
return await CallFreshdeskApiAsync(request, retriedCount++); // <=
}
During the postfix incrementation the current value of the variable is returned first and only after that this value is increased.
Link to the line with error.
All the described issues were found when we checked your project with the PVS-Studio analyzer. You can find more issues in the article.
Expected Result
No errors in the code.
Actual Result
The presence of errors in the code.
Screenshots or Videos
No response
Additional Context
No response
Build Version
1.48.1
Environment
Cloud (bitwarden.com)
Environment Details
No response
Hi @NikiPn this is a really great write-up and accompanying article. I'm going to assign this to me and slowly start tackling them but I also don't want to steal your thunder. You did a lot of awesome ground work here and I would be very open to reviewing some pull requests if you want to make them.
At the beginning of the year we started enforcing linting rules in our CI pipeline and I have a goal of extending that by both being stricter with the built in analyzers and eventually enabling nullable annotations. Once we've got that foundation set I am all for building on top of it and looking at outside analyzers so that we can keep our code quality top notch. Once again, thanks a ton!
