Copilot commented on code in PR #12419:
URL: https://github.com/apache/cloudstack/pull/12419#discussion_r2688164260
##########
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java:
##########
@@ -235,6 +234,7 @@ public List<LdapUser> getUsers(final String username, final
LdapContext context,
}
@Override
+ @SuppressWarnings("BanJNDI")
Review Comment:
The `@SuppressWarnings(\"BanJNDI\")` annotation is being used to suppress
Error Prone warnings about JNDI usage. While JNDI usage in LDAP operations is
legitimate, ensure that all JNDI contexts are properly secured and validated to
prevent LDAP injection attacks. Review the input validation in these methods
(getUsersInGroup, getUserForDn, searchUser, searchUsers) to ensure
user-controlled data is properly sanitized before being used in LDAP queries.
##########
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java:
##########
@@ -34,6 +34,7 @@ public class ADLdapUserManagerImpl extends
OpenLdapUserManagerImpl implements Ld
private static final String MICROSOFT_AD_MEMBERS_FILTER = "memberOf";
@Override
+ @SuppressWarnings("BanJNDI")
Review Comment:
The `@SuppressWarnings(\"BanJNDI\")` annotation suppresses JNDI-related
warnings. Ensure that the `groupName` parameter and any other user-controlled
inputs are properly validated and sanitized before being used in LDAP queries
to prevent LDAP injection vulnerabilities.
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java:
##########
@@ -497,6 +498,11 @@ public void process(final Answer[] answers) {
*/
protected abstract boolean isClosed();
+ @Override
+ public int hashCode() {
+ return Objects.hash(logger, _id, _uuid, _name, _waitForList,
_requests, _currentSequence, _status, _maintenance, _nextSequence, _agentMgr);
Review Comment:
The `hashCode()` implementation includes mutable fields (_waitForList,
_requests, _currentSequence, _status, _maintenance, _nextSequence) and a logger
instance. This violates the hashCode contract, as the hash code can change
during the object's lifetime. The logger field should not be included in
hashCode calculations. If this class is used in hash-based collections, only
use immutable fields like _id, _uuid, and _name in the hash code calculation.
```suggestion
return Objects.hash(_id, _uuid, _name);
```
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java:
##########
@@ -148,6 +149,11 @@ private synchronized void scheduleFromQueue() {
}
}
+ @Override
+ public int hashCode() {
+ return Objects.hash(super.hashCode(), _HostPingRetryCount,
_HostPingRetryTimer, _resource, _futures, _seq, tasks, _outstandingTaskCount,
_outstandingCronTaskCount);
Review Comment:
The `hashCode()` implementation includes mutable fields (_futures, tasks,
_outstandingTaskCount, _outstandingCronTaskCount) that can change during the
object's lifetime. This violates the hashCode contract which requires that if
two objects are equal, they must have the same hash code throughout their
lifetime. If this class is used as a key in hash-based collections, the hash
code should only include immutable fields or fields that don't change after the
object is added to the collection.
```suggestion
return super.hashCode();
```
##########
plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/util/NexentaStorAppliance.java:
##########
@@ -248,6 +254,11 @@ static final class LuParams {
public boolean equals(Object other) {
return other instanceof LuParams;
}
+
+ @Override
+ public int hashCode() {
+ return 1;
Review Comment:
The `hashCode()` implementation for `LuParams` returns a constant value of
1, which violates the hashCode contract. Since the `equals()` method considers
all instances of `LuParams` to be equal (line 255), this constant hashCode is
technically correct but indicates a potential design issue. If all instances
are equal, consider whether this class should be used as a key in hash-based
collections, or if the equality logic needs to be refined.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
