diff --git a/utils/src/main/java/com/cloud/utils/net/NetUtils.java b/utils/src/main/java/com/cloud/utils/net/NetUtils.java index eeeb008a462..db724f6a1f4 100644 --- a/utils/src/main/java/com/cloud/utils/net/NetUtils.java +++ b/utils/src/main/java/com/cloud/utils/net/NetUtils.java @@ -40,6 +40,7 @@ import java.util.TreeSet; 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 class NetUtils { } 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 class NetUtils { } 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 class NetUtils { 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 class NetUtils { } 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; } diff --git a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java index a103f0a179c..bf686c2a241 100644 --- a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java +++ b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java @@ -39,6 +39,7 @@ import org.apache.log4j.Logger; 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 class NetUtilsTest { 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)); + } }