server: avoid NPE updating a network ACL rule with no protocol

On a full (non-partial) network ACL item update, transferDataToNetworkAclRulePojo
clears the protocol to null before updateIcmpCodeAndTypeFullUpgrade runs, and
that method called networkACLItemVo.getProtocol().equalsIgnoreCase(...) on the
null protocol, throwing NullPointerException. A full upgrade with no protocol is
a valid input: the parameter is optional and the mode is meant to disregard the
current configuration. Compare against the constant first so a null protocol
falls through to clearing the icmp fields.

Author: nagaboinaramgopal

server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java

@@ -946,7 +946,7 @@ protected void updateIcmpCodeAndType (boolean isPartialUpgrade, UpdateNetworkACL
     }
 
     private void updateIcmpCodeAndTypeFullUpgrade (Integer icmpCode, Integer icmpType, NetworkACLItemVO networkACLItemVo) {
-        if (networkACLItemVo.getProtocol().equalsIgnoreCase(NetUtils.ICMP_PROTO)) {
+        if (NetUtils.ICMP_PROTO.equalsIgnoreCase(networkACLItemVo.getProtocol())) {
             networkACLItemVo.setIcmpCode(icmpCode != null ? icmpCode : -1);
             networkACLItemVo.setIcmpType(icmpType != null ? icmpType : -1);
         } else {

server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java

@@ -45,6 +45,7 @@
 import org.apache.cloudstack.context.CallContext;
 import org.apache.commons.lang3.StringUtils;
 import org.junit.Assert;
+import org.springframework.test.util.ReflectionTestUtils;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -1506,4 +1507,16 @@ public void validateAclAssociatedToVpcTestNullVpcShouldThrowInvalidParameterValu
 
         networkAclServiceImpl.validateAclAssociatedToVpc(networkMockVpcMockId, accountMock, SOME_UUID);
     }
+
+    @Test
+    public void updateIcmpCodeAndTypeFullUpgradeHandlesNullProtocol() {
+        NetworkACLItemVO rule = new NetworkACLItemVO();
+        rule.setIcmpCode(5);
+        rule.setIcmpType(8);
+
+        ReflectionTestUtils.invokeMethod(networkAclServiceImpl, "updateIcmpCodeAndTypeFullUpgrade", 1, 2, rule);
+
+        Assert.assertNull(rule.getIcmpCode());
+        Assert.assertNull(rule.getIcmpType());
+    }
 }