From 1d6ee2dcb0364c3cc69e800cb8173cd8ac523fa8 Mon Sep 17 00:00:00 2001
From: weingartner <rafaelweingartner@gmail.com>
Date: Fri, 20 Nov 2015 13:31:58 -0200
Subject: [PATCH] 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.
---
 .../java/com/cloud/utils/net/NetUtils.java    | 33 +++++++++++--------
 .../com/cloud/utils/net/NetUtilsTest.java     | 31 +++++++++++++++++
 2 files changed, 51 insertions(+), 13 deletions(-)

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));
+    }
 }