Dogface2k commented on issue #13616:
URL: https://github.com/apache/cloudstack/issues/13616#issuecomment-5200068295
I traced this through the current 4.22 code and can confirm the exact
failure mechanism. This is a regression caused by the interaction with #11249.
#11249 changed `networks.cidr` for shared networks from a single CIDR into a
comma-separated list when multiple Guest IP ranges exist. It also introduced
`com.cloud.utils.StringUtils#getFirstValueFromCommaSeparatedString()` and used
that for backward-compatible API responses, but the internal singular CIDR
helper was not updated.
The failing path is:
```text
VirtualNetworkApplianceManagerImpl#createGuestBootLoadArgs()
-> NetworkModelImpl#getValidNetworkCidr()
-> NetUtils#getCidrNetmask()
```
`getValidNetworkCidr()` currently returns `guestNetwork.getCidr()` unchanged
whenever `network_cidr` is null.
For:
```text
172.30.10.0/24,172.30.11.0/24
```
`NetUtils#getCidrNetmask()` performs `cidr.split("/")`, producing:
```text
[172.30.10.0, 24,172.30.11.0, 24]
```
It then calls:
```java
Long.parseLong("24,172.30.11.0")
```
which reproduces the exact `NumberFormatException` reported here.
The correct repair point is `NetworkModelImpl#getValidNetworkCidr()`,
because all of its production callers expect one CIDR and immediately pass it
to single-CIDR netmask/DHCP calculations. `NetUtils` itself should remain a
strict single-CIDR parser.
Suggested implementation:
```java
@Override
public String getValidNetworkCidr(Network guestNetwork) {
String networkCidr = guestNetwork.getNetworkCidr();
String validNetworkCidr = networkCidr == null ? guestNetwork.getCidr() :
networkCidr;
return
com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(validNetworkCidr);
}
```
This preserves `network_cidr` precedence, leaves null/empty/single-CIDR
behaviour unchanged, and applies the same first-value compatibility already
established by #11249.
Regression coverage should verify:
1. `network_cidr` still takes precedence;
2. a single fallback `cidr` is unchanged;
3. a comma-separated fallback `cidr` returns its first CIDR;
4. both reported runtime flows succeed:
- initial VR deployment after adding a second Guest IP range;
- network restart with cleanup after adding a second Guest IP range.
The vulnerable implementation is still present on `4.22` and `main`, and I
did not find a linked fix PR. Since this issue is assigned, is a patch already
in progress? Otherwise this is ready for a focused bug-fix PR against `4.22`
for the 4.22.2 milestone.
--
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]
