update 1

Author: weizhouapache

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

@@ -72,6 +72,7 @@
 import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.net.NetworkProtocols;
 
 @Component
 public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLService {
@@ -588,7 +589,9 @@ protected void validateProtocol(NetworkACLItemVO networkACLItemVO) {
             throw new InvalidParameterValueException("Can specify icmpCode and icmpType for ICMP protocol only");
         }
         if (isIcmpProtocol) {
-            NetUtils.validateIcmpTypeAndCode(icmpType, icmpCode);
+            if (!NetworkProtocols.validateIcmpTypeAndCode(icmpType, icmpCode)) {
+                throw new InvalidParameterValueException(String.format("Unsupported icmptype %s or icmpcode %s", icmpType, icmpCode));
+            }
         }
 
         Integer sourcePortStart = networkACLItemVO.getSourcePortStart();

utils/src/main/java/com/cloud/utils/net/NetworkProtocols.java

@@ -19,6 +19,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 
 /**
  * Network protocols and parameters.
@@ -274,6 +275,23 @@ static void addIcmpCode(IcmpCode code) {
         type.addIcmpCodes(code);
     }
 
+    public static boolean validateIcmpTypeAndCode(Integer type, Integer code) {
+        if (type != null && type != -1) {
+            Optional<IcmpType> icmpTypeOptional = IcmpTypes.stream().filter(t -> t.getType().equals(type)).findFirst();
+            if (icmpTypeOptional == null || icmpTypeOptional.isEmpty()) {
+                return false;
+            }
+            IcmpType icmpType = icmpTypeOptional.get();
+            if (code != null && code != -1) {
+                Optional<IcmpCode> icmpCodeOptional = icmpType.getIcmpCodes().stream().filter(c -> c.getCode().equals(code)).findFirst();
+                if (icmpCodeOptional == null || icmpCodeOptional.isEmpty()) {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+
     static {
         addIcmpCode(new IcmpCode(0, 0, "Echo reply"));
         addIcmpCode(new IcmpCode(3, 0, "Net unreachable"));

utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java

@@ -843,4 +843,31 @@ public void getNetworkInterfaceTestReturnInterfaceReturnedByGetByName() throws S
         Assert.assertEquals(expected, result);
         networkInterfaceMocked.close();
     }
+
+    private void validateIcmpTypeAndCodeInternal(Integer icmpType, Integer icmpCode, boolean expectedResult) {
+        boolean actualResult = true;
+        try {
+            NetUtils.validateIcmpTypeAndCode(icmpType, icmpCode);
+        } catch (CloudRuntimeException e) {
+            actualResult = false;
+        }
+        Assert.assertEquals(expectedResult, actualResult);
+    }
+
+    @Test
+    @PrepareForTest(NetUtils.class)
+    public void validateIcmpTypeAndCodes() {
+        validateIcmpTypeAndCodeInternal(-1, -1, true);
+        validateIcmpTypeAndCodeInternal(3, 2, true);
+        validateIcmpTypeAndCodeInternal(null, null, false);
+        validateIcmpTypeAndCodeInternal(null, -1, false);
+        validateIcmpTypeAndCodeInternal(-1, null, false);
+        validateIcmpTypeAndCodeInternal(-1, 2, false);
+        validateIcmpTypeAndCodeInternal(3, -1, false);   // need discussion
+        validateIcmpTypeAndCodeInternal(-2, 2, false);
+        validateIcmpTypeAndCodeInternal(257, 2, false);
+        validateIcmpTypeAndCodeInternal(3, -2, false);
+        validateIcmpTypeAndCodeInternal(3, -257, false);
+
+    }
 }

utils/src/test/java/com/cloud/utils/net/NetworkProtocolsTest.java

@@ -0,0 +1,47 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.junit.runner.RunWith;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+
+@RunWith(PowerMockRunner.class)
+public class NetworkProtocolsTest {
+
+    @Test
+    public void validateIcmpTypeAndCode() {
+        validateIcmpTypeAndCodeInternal(null, null, true);
+        validateIcmpTypeAndCodeInternal(null, -1, true);
+        validateIcmpTypeAndCodeInternal(-1, -1, true);
+        validateIcmpTypeAndCodeInternal(3, -1, true);
+        validateIcmpTypeAndCodeInternal(3, 15, true);
+        validateIcmpTypeAndCodeInternal(4, -1, false);
+        validateIcmpTypeAndCodeInternal(5, 10, false);
+    }
+
+    private void validateIcmpTypeAndCodeInternal(Integer type, Integer code, boolean expected) {
+        boolean actual = NetworkProtocols.validateIcmpTypeAndCode(type, code);
+        Assert.assertEquals(expected, actual);
+    }
+}