fix: incorrect conversion between integer types#461
Conversation
lbajolet-hashicorp
left a comment
There was a problem hiding this comment.
Thanks for the PR @tenthirtyam,
At first glance it would seem that the clamping is unnecessary as ParseInt already errors if the value can't fit on the provided bitSize, that said I understand this is meant to silence the errors reported by CodeQL, but I wonder if there's some other way we can document that those checks don't need to exist, and that they shouldn't be reported.
That said, those checks don't fundamentally harm the code so I'm not completely against merging this, but I think we would benefit from investigating how we can silence this check as it is being too cautious I think.
5cb9506 to
4acaf07
Compare
lbajolet-hashicorp
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the reroll @tenthirtyam.
Only thing I can think of left is maybe using %s for interpolating errors in the error string, as you did a PR not too long ago to harmonise that, it may be good to continue using %s for those use cases.
That said it's very nitpicky so feel free to disregard if you think it's find as-is
Pre-approving to not block later
Fixes incorrect conversion of an unsigned 32-bit integer from to a lower bit size type int32 without an upper bound check. Signed-off-by: Ryan Johnson <ryan.johnson@broadcom.com>
4acaf07 to
8c8e408
Compare
|
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Summary
Fixes incorrect conversion of an unsigned 32-bit integer from to a lower bit size type int32 without an upper bound check.
Testing
Reference