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.

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

Jump to bottom

Open

erikbocks wants to merge 1 commit into apache:main

from scclouds:fix-npe-when-listing-ips-of-removed-network

Open

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

erikbocks wants to merge 1 commit into apache:main from scclouds:fix-npe-when-listing-ips-of-removed-network

+14 −4

Conversation

@erikbocks

Copy link

Collaborator

@erikbocks erikbocks commented Jan 5, 2026

Description

When listing public IPs associated to a network, the database query only considers records that are not marked as removed. If the ID of an removed resource is informed, a null value is returned in the database query. If the caller is a User type account, during the access checks the code tries to access one of the value's properties, resulting in an NullPointerException.

A validation was added to this flow to prevent the property access when the resource is null, and the listing behavior was standardized. The same validation was added to handle removed VPCs, as the same behavior was presented for them.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Bug Severity

BLOCKER
Critical
Major
Minor
Trivial

Screenshots (if appropriate):

Behavior before the change

image

Behavior after the change

image

How Has This Been Tested?

First, I created an isolated network, informing a public IP address to be assigned as the network's source NAT. After that, the network was removed. Then, I authenticated with an User type account in CloudMonkey and called the listPublicIpAddresses API, informing the removed network's UUID as the associatednetworkid parameter. As expected, the exception was thrown.

Exception stack trace in Management Server's logs

2026年01月05日 15:07:39,520 ERROR [c.c.a.ApiServer] (qtp1404565079-21:[ctx-8bd6ddc5, ctx-459e3280]) (logid:4b10ae2c) unhandled exception executing api command: [Ljava.lang.String;@1b1ed74b java.lang.NullPointerException: Cannot invoke "org.apache.cloudstack.acl.ControlledEntity.getDomainId()" because "entity" is null
	at com.cloud.user.AccountManagerImpl.checkAccess(AccountManagerImpl.java:738)
	at com.cloud.user.AccountManagerImpl.checkAccess(AccountManagerImpl.java:706)
	[...]
	at com.cloud.server.ManagementServerImpl.searchForIPAddresses(ManagementServerImpl.java:2651)

After applying the code with my changes to my local environment, the same steps were executed, and I validated that the behavior was fixed. The same steps were reproduced for removed VPCs.

The code was also compiled with tests on Maven.

@erikbocks


 Fix NPE when listing public IPs associated to removed networks or VPCs

3e34da6

@boring-cyborg boring-cyborg bot added the component:management-server label

Jan 5, 2026

@erikbocks erikbocks changed the title ~~(削除) Fix NPE when listing public IPs associated to removed networks or VPCs (削除ここまで)~~ (追記) Fix NPE during public IP listing when a removed network or VPC ID is informed for associatenetworkid parameter (追記ここまで)

Jan 5, 2026

@codecov

Copy link

codecov bot commented Jan 5, 2026 •

edited

Loading

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.76%. Comparing base (4628385) to head (3e34da6).

Files with missing lines	Patch %	Lines
...in/java/com/cloud/server/ManagementServerImpl.java	0.00%	8 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@
## main #12372 +/- ##
============================================
- Coverage 17.76% 17.76% -0.01% 
 Complexity 15861 15861 
============================================
 Files 5923 5923 
 Lines 530470 530474 +4 
 Branches 64823 64825 +2 
============================================
- Hits 94253 94251 -2 
- Misses 425673 425678 +5 
- Partials 10544 10545 +1

Flag	Coverage Δ
uitests	`3.57% <ø> (ø)`
unittests	`18.85% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sureshanaparti sureshanaparti requested a review from Copilot

January 5, 2026 17:23

@sureshanaparti

Copy link

Contributor

sureshanaparti commented Jan 5, 2026

@blueorangutan package

@blueorangutan

Copy link

blueorangutan commented Jan 5, 2026

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

Copilot started reviewing on behalf of sureshanaparti

January 5, 2026 17:48

View session

Copilot AI reviewed

Jan 5, 2026

View reviewed changes

Copy link

Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a NullPointerException that occurs when listing public IP addresses with a removed network or VPC ID as a parameter. When a User account attempts to list IPs using a removed resource ID, the database query returns null, causing an NPE during access checks.

Key Changes

Added null checks before accessing network and VPC objects when validating access permissions
Search criteria parameters are now conditionally set only when the associated resource exists
Added VpcVO import to support the VPC null check logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

server/src/main/java/com/cloud/server/ManagementServerImpl.java

Comment on lines 2651 to 2667

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);

}

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);

}

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.

server/src/main/java/com/cloud/server/ManagementServerImpl.java

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);

} else {

throw new InvalidParameterValueException("Unable to find network by id " + associatedNetworkId);

Copilot uses AI. Check for mistakes.

server/src/main/java/com/cloud/server/ManagementServerImpl.java

Comment on lines +2663 to +2666

if (vpc != null) {

_accountMgr.checkAccess(caller, null, false, vpc);

sc.setParameters("vpcId", vpcId);

}

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.

@blueorangutan

Copy link

blueorangutan commented Jan 5, 2026

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16258

@DaanHoogland

Copy link

Contributor

DaanHoogland commented Jan 6, 2026

@erikbocks , would this also apply to version 20 and 22? (not just main?)

Labels

component:management-server

4 participants

@erikbocks @sureshanaparti @blueorangutan @DaanHoogland

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

Are you sure you want to change the base?

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

Conversation

@erikbocks erikbocks commented Jan 5, 2026

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

codecov bot commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sureshanaparti commented Jan 5, 2026

Uh oh!

blueorangutan commented Jan 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jan 5, 2026

Uh oh!

DaanHoogland commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Jan 5, 2026 •

edited

Loading