-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix 2 findbugs SQL_PREPARED_STATEMENT_GENERATED_FROM_NONCONSTANT_STRI… #476
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
Conversation
…NG warnings in ConfigurationManagerImpl.java
|
cloudstack-pull-requests #479 SUCCESS |
| @@ -926,7 +926,7 @@ protected void checkIfPodIsDeletable(final long podId) { | |||
| dbName = "cloud"; | |||
| } | |||
|
|
|||
| String selectSql = "SELECT * FROM `" + dbName + "`.`" + tableName + "` WHERE " + column + " = ?"; | |||
| String selectSql = "SELECT * FROM `?`.`?` WHERE ? = ?"; | |||
|
|
|||
| if(tableName.equals("vm_instance")) { | |||
| selectSql += " AND state != '" + VirtualMachine.State.Expunging.toString() + "' AND removed IS NULL"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also move the state to a variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't be necessary :)
findbugs is smart enough to check that the returned string is static and cannot be polluted by user input
that's only not the case for other variables that it can't track the origin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering why are we even using a prepareStatement here (and doing a fix to satisfy findbugs). Why dont we use createStatement() and execute sql directly?
|
@bhaisaab did travis CI run on the updated PR? |
|
@bhaisaab ignore my previous comment. looks like code change was in the other PR. |
…NG warnings in ConfigurationManagerImpl.java Signed-off-by: Rohit Yadav <[email protected]> This closes apache#476
Fixes #476 Signed-off-by: Rohit Yadav <[email protected]>
…update_time_fix Host capacity calculation: use VM creation time if update time is null
…NG warnings in ConfigurationManagerImpl.java