fix: resolve CodeQL integer-overflow and obsolete warnings#89
Conversation
- Cast SKRect/DrawBitmap multiplications to float before evaluation to avoid int overflow. - Guard nullable ImageAttributes in SvgQRCode.IsBlockedByLogo. - Update renderer tests to use non-obsolete options overloads. - Correct PayloadGeneratorTests pragma (CS0612 -> CS0618). Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| float iconDestWidth = iconSizePercent * (float)bmp.Width / 100f; | ||
| float iconDestHeight = iconDestWidth * (float)icon.Height / (float)icon.Width; | ||
| float iconX = ((float)bmp.Width - iconDestWidth) / 2; | ||
| float iconY = ((float)bmp.Height - iconDestHeight) / 2; | ||
| var centerDest = new SKRect(iconX - iconBorderWidth, iconY - iconBorderWidth, iconX - iconBorderWidth + iconDestWidth + iconBorderWidth * 2f, iconY - iconBorderWidth + iconDestHeight + iconBorderWidth * 2f); |
There was a problem hiding this comment.
📝 Info: Integer overflow risk fixed in icon size calculation
The old code at QRCoder.Core/Renderers/QRCode.cs:219 computed iconSizePercent * bmp.Width as int * int, which could overflow for large bitmaps (e.g., iconSizePercent=15 and bmp.Width > 143M pixels). The new code casts bmp.Width to float first, avoiding the overflow. While the overflow scenario is unlikely in practice (QR bitmaps are small), the fix is correct and makes the arithmetic more robust. The same pattern is applied to iconDestHeight, iconX, iconY, and centerDest calculations.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (!attr.HasValue) | ||
| return false; |
There was a problem hiding this comment.
📝 Info: SvgQRCode null check is defensive — callers already guard against null attr
The new null check if (!attr.HasValue) return false at QRCoder.Core/Renderers/SvgQRCode.cs:229 is purely defensive. Both call sites (lines 143 and 195) use short-circuit evaluation: logo == null || !logo.FillLogoBackground() || !IsBlockedByLogo(...). Since logoAttr is only set when logo != null (line 129-130), and IsBlockedByLogo is only reached when logo != null and logo.FillLogoBackground() is true, attr will always have a value when the method executes. The check prevents a potential InvalidOperationException if the method were ever called directly with a null attr in the future.
Was this helpful? React with 👍 or 👎 to provide feedback.
| #pragma warning disable CS0618 | ||
| var generator = new PayloadGenerator.BezahlCode(AuthorityType.singlepayment, name, account: account, bnc: bnc, amount: amount); | ||
| #pragma warning restore CS0612 | ||
| #pragma warning restore CS0618 |
There was a problem hiding this comment.
📝 Info: Pragma change from CS0612 to CS0618 is correct for Obsolete attributes with messages
The [Obsolete("message")] form on the AuthorityType enum values (e.g., singlepayment, singledirectdebit, periodicsinglepayment at QRCoder.Core/Generators/PayloadGenerator.cs:2841-2865) generates compiler warning CS0618 (not CS0612). CS0612 is for [Obsolete] without a message string. The old pragmas were suppressing the wrong warning, meaning the actual CS0618 warnings were likely still being emitted. This fix ensures the correct warning is suppressed.
Was this helpful? React with 👍 or 👎 to provide feedback.
| { | ||
| var bgOffset = 4 - offset; | ||
| using (var resizedImage = Resize(backgroundImage, size - (2 * bgOffset * pixelsPerModule))) | ||
| var dataAreaSize = size - (int)(2f * bgOffset * pixelsPerModule); |
There was a problem hiding this comment.
📝 Info: ArtQRCode dataAreaSize calculation changes from pure int to float-then-int-cast
The old expression size - (2 * bgOffset * pixelsPerModule) was pure integer arithmetic. The new expression size - (int)(2f * bgOffset * pixelsPerModule) computes via float then truncates back to int. For the values involved (bgOffset is either 0 or 4, pixelsPerModule is a small positive int), the results are identical since 2 * bgOffset * pixelsPerModule is always an exact integer representable in float. The change is consistent with the broader pattern of using float arithmetic throughout the renderer, but the intermediate float conversion is unnecessary for these specific values.
Was this helpful? React with 👍 or 👎 to provide feedback.
f6308b3
into
feature/devin-20260712-sonar-quality
1 participant
All Submissions:
New Feature Submissions:
Changes to Core Features:
Summary
Resolves the CodeQL alerts introduced in the SonarCloud cleanup branch:
float(pixelsPerModuleF) before casting toSKRect/DrawBitmapcoordinates.DrawIconwidth/height and border-radius calculations by casting operands tofloatbefore multiplication.ImageAttributes?inIsBlockedByLogowithHasValue.GetGraphicoverloads with their non-obsolete*GraphicOptionsequivalents and corrects the legacyBezahlCodepragma fromCS0612toCS0618.Verification:
dotnet build QRCoder.Core.sln --configuration Release0 warnings / 0 errorsdotnet test QRCoder.Core.Tests/ -f net10.0 --configuration Release502 passedThis is a follow-up phase PR targeting the integration branch
feature/devin-20260712-sonar-quality.Link to Devin session: https://app.devin.ai/sessions/20c92a1bad724f63b8be89a9e195e664
Requested by: @afonsoft