Skip to content

Fix NPE during public IP listing when a removed network or VPC ID is informed for associatenetworkid parameter #12372

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.

Sign up for GitHub

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

Open
erikbocks wants to merge 1 commit into
base: main
Choose a base branch
from
+14 −4
Open

Fix NPE during public IP listing when a removed network or VPC ID is informed for associatenetworkid parameter #12372

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions server/src/main/java/com/cloud/server/ManagementServerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.network.vpc.VpcVO;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.affinity.AffinityGroupProcessor;
Expand Down Expand Up @@ -2648,12 +2649,21 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
}

if (associatedNetworkId != null) {
_accountMgr.checkAccess(caller, null, false, networkDao.findById(associatedNetworkId));
sc.setParameters("associatedNetworkIdEq", associatedNetworkId);
NetworkVO associatedNetwork = networkDao.findById(associatedNetworkId);

if (associatedNetwork != null) {
_accountMgr.checkAccess(caller, null, false, associatedNetwork);
sc.setParameters("associatedNetworkIdEq", associatedNetworkId);
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a removed network ID is provided, the code silently skips the search parameter without informing the user. This creates inconsistent API behavior where a valid but removed network ID returns all public IPs instead of an empty result or an error. Consider throwing an InvalidParameterValueException when the network is not found to provide clearer feedback to the API caller.

Suggested change
sc.setParameters("associatedNetworkIdEq", associatedNetworkId);
sc.setParameters("associatedNetworkIdEq", associatedNetworkId);
} else {
throw new InvalidParameterValueException("Unable to find network by id " + associatedNetworkId);

Copilot uses AI. Check for mistakes.
}
}

if (vpcId != null) {
_accountMgr.checkAccess(caller, null, false, _vpcDao.findById(vpcId));
sc.setParameters("vpcId", vpcId);
VpcVO vpc = _vpcDao.findById(vpcId);

if (vpc != null) {
_accountMgr.checkAccess(caller, null, false, vpc);
sc.setParameters("vpcId", vpcId);
}
Comment on lines +2663 to +2666
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a removed VPC ID is provided, the code silently skips the search parameter without informing the user. This creates inconsistent API behavior where a valid but removed VPC ID returns all public IPs instead of an empty result or an error. Consider throwing an InvalidParameterValueException when the VPC is not found to provide clearer feedback to the API caller.

Suggested change
if (vpc != null) {
_accountMgr.checkAccess(caller, null, false, vpc);
sc.setParameters("vpcId", vpcId);
}
if (vpc == null) {
throw new InvalidParameterValueException("Unable to find VPC by id " + vpcId);
}
_accountMgr.checkAccess(caller, null, false, vpc);
sc.setParameters("vpcId", vpcId);

Copilot uses AI. Check for mistakes.
}
Comment on lines 2651 to 2667
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix for the NPE is incomplete. While null checks are added for associatedNetworkId and vpcId here, there's no test coverage for these specific scenarios. Since ManagementServerImplTest already has tests for the setParameters method with ListPublicIpAddressesCmd, consider adding tests that verify the behavior when removed network/VPC IDs are provided.

Copilot uses AI. Check for mistakes.

addrs = _publicIpAddressDao.search(sc, searchFilter); // Allocated
Expand Down
Loading