Changed the behavior of methods that use NetUtils.cidrToLong(String)

Given that the method com.cloud.utils.net.NetUtils.cidrToLong(String)
now throws an exception when receiving null or empty cidrs, there is the
need to change methods that use it. Those methods were changed and test
cases created.

Author: rafaelweingartner

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

@@ -40,6 +40,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.SystemUtils;
 import org.apache.commons.net.util.SubnetUtils;
 import org.apache.commons.validator.routines.InetAddressValidator;
@@ -838,13 +839,13 @@ public static enum SupersetOrSubset {
     }
 
     public static SupersetOrSubset isNetowrkASubsetOrSupersetOfNetworkB(final String cidrA, final String cidrB) {
+        if (!areCidrsNotEmpty(cidrA, cidrB)) {
+            return SupersetOrSubset.errorInCidrFormat;
+        }
         final Long[] cidrALong = cidrToLong(cidrA);
         final Long[] cidrBLong = cidrToLong(cidrB);
         long shift = 0;
-        if (cidrALong == null || cidrBLong == null) {
-            //implies error in the cidr format
-            return SupersetOrSubset.errorInCidrFormat;
-        }
+
         if (cidrALong[1] >= cidrBLong[1]) {
             shift = MAX_CIDR - cidrBLong[1];
         } else {
@@ -867,15 +868,20 @@ public static SupersetOrSubset isNetowrkASubsetOrSupersetOfNetworkB(final String
     }
 
     public static boolean isNetworkAWithinNetworkB(final String cidrA, final String cidrB) {
-        final Long[] cidrALong = cidrToLong(cidrA);
-        final Long[] cidrBLong = cidrToLong(cidrB);
-        if (cidrALong == null || cidrBLong == null) {
+        if (!areCidrsNotEmpty(cidrA, cidrB)) {
             return false;
         }
-        final long shift = MAX_CIDR - cidrBLong[1];
+        Long[] cidrALong = cidrToLong(cidrA);
+        Long[] cidrBLong = cidrToLong(cidrB);
+
+        long shift = MAX_CIDR - cidrBLong[1];
         return cidrALong[0] >> shift == cidrBLong[0] >> shift;
     }
 
+    static boolean areCidrsNotEmpty(String cidrA, String cidrB) {
+        return StringUtils.isNotEmpty(cidrA) && StringUtils.isNotEmpty(cidrB);
+    }
+
     public static Long[] cidrToLong(final String cidr) {
         if (cidr == null || cidr.isEmpty()) {
             throw new CloudRuntimeException("empty cidr can not be converted to longs");
@@ -887,12 +893,12 @@ public static Long[] cidrToLong(final String cidr) {
         final String cidrAddress = cidrPair[0];
         final String cidrSize = cidrPair[1];
         if (!isValidIp(cidrAddress)) {
-            throw new CloudRuntimeException("cidr is not bvalid in ip space" + cidr);
+            throw new CloudRuntimeException("cidr is not valid in ip space" + cidr);
         }
         long cidrSizeNum = getCidrSizeFromString(cidrSize);
         final long numericNetmask = netMaskFromCidr(cidrSizeNum);
         final long ipAddr = ip2Long(cidrAddress);
-        final Long[] cidrlong = {ipAddr & numericNetmask, (long)cidrSizeNum};
+        final Long[] cidrlong = {ipAddr & numericNetmask, cidrSizeNum};
         return cidrlong;
 
     }
@@ -1164,11 +1170,12 @@ public static boolean verifyInstanceName(final String instanceName) {
     }
 
     public static boolean isNetworksOverlap(final String cidrA, final String cidrB) {
-        final Long[] cidrALong = cidrToLong(cidrA);
-        final Long[] cidrBLong = cidrToLong(cidrB);
-        if (cidrALong == null || cidrBLong == null) {
+        if (!areCidrsNotEmpty(cidrA, cidrB)) {
             return false;
         }
+        Long[] cidrALong = cidrToLong(cidrA);
+        Long[] cidrBLong = cidrToLong(cidrB);
+
         final long shift = MAX_CIDR - (cidrALong[1] > cidrBLong[1] ? cidrBLong[1] : cidrALong[1]);
         return cidrALong[0] >> shift == cidrBLong[0] >> shift;
     }

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

@@ -39,6 +39,7 @@
 import org.junit.Test;
 
 import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils.SupersetOrSubset;
 import com.googlecode.ipv6.IPv6Address;
 
 public class NetUtilsTest {
@@ -477,4 +478,34 @@ public void testNetmaskFromCidr() {
         mask = NetUtils.netMaskFromCidr(32l);
         assertEquals("mask not right: " + mask, 0xffffffff, mask);
     }
+
+    @Test
+    public void testIsCidrsNotEmptyWithNullCidrs() {
+        assertEquals(false, NetUtils.areCidrsNotEmpty(null, null));
+    }
+
+    @Test
+    public void testIsCidrsNotEmptyWithEmptyCidrs() {
+        assertEquals(false, NetUtils.areCidrsNotEmpty("", "  "));
+    }
+
+    @Test
+    public void testIsCidrsNotEmpty() {
+        assertEquals(true, NetUtils.areCidrsNotEmpty("10.10.0.0/16", "10.1.2.3/16"));
+    }
+
+    @Test
+    public void testIsNetowrkASubsetOrSupersetOfNetworkBWithEmptyValues() {
+        assertEquals(SupersetOrSubset.errorInCidrFormat, NetUtils.isNetowrkASubsetOrSupersetOfNetworkB("", null));
+    }
+
+    @Test
+    public void testIsNetworkAWithinNetworkBWithEmptyValues() {
+        assertEquals(false, NetUtils.isNetworkAWithinNetworkB("", null));
+    }
+
+    @Test
+    public void testIsNetworksOverlapWithEmptyValues() {
+        assertEquals(false, NetUtils.isNetworksOverlap("", null));
+    }
 }