bhouse-nexthop commented on code in PR #14359:
URL: https://github.com/apache/cloudstack/pull/14359#discussion_r4218824852
##########
server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java:
##########
@@ -619,7 +620,8 @@ protected void validateProtocol(NetworkACLItemVO
networkACLItemVO) {
throw new InvalidParameterValueException("Invalid protocol
number: " + protoNumber);
}
} else {
- if
(!supportedProtocolsForAclRules.contains(protocol.toLowerCase())) {
+ // one of the names, not just part of the string: "cp" or "dp,ic"
must not pass
+ if
(!Arrays.asList(supportedProtocolsForAclRules.split(",")).contains(protocol.toLowerCase(Locale.ROOT)))
{
Review Comment:
Yes, good catch. They had the same check, and some provider lists have a
space after the comma (`"tcp,udp,icmp, all"`), so splitting alone would have
rejected `all`. fa1632dbee adds `NetUtils.isProtocolInList()`, which matches a
whole entry in any case and trims the list entries. It is now used by the
network ACL check and by the firewall / port forwarding / static NAT
(`FirewallManagerImpl`), IPv6 firewall (`Ipv6ServiceImpl`) and routing firewall
(`RoutedIpv4ManagerImpl`) checks, with tests for the helper. I updated the
description to match.
--
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]
