From 3ae4dd06f355ce81da4c672d1305c7707ccc6d8a Mon Sep 17 00:00:00 2001
From: Daan Hoogland <daan@onecht.net>
Date: Tue, 10 Nov 2015 16:18:52 +0100
Subject: [PATCH] CID-1116482 cidrToLong cleanup of bitshift problem

 and some trivial testing
---
 .../java/com/cloud/utils/net/NetUtils.java    | 43 +++++++++++--------
 .../com/cloud/utils/net/NetUtilsTest.java     | 30 +++++++++++++
 2 files changed, 55 insertions(+), 18 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 b0eb2f2faab..28e5ab5e572 100644
--- a/utils/src/main/java/com/cloud/utils/net/NetUtils.java
+++ b/utils/src/main/java/com/cloud/utils/net/NetUtils.java
@@ -870,31 +870,44 @@ public class NetUtils {
 
     public static Long[] cidrToLong(final String cidr) {
         if (cidr == null || cidr.isEmpty()) {
-            return null;
+            throw new CloudRuntimeException("empty cidr can not be converted to longs");
         }
         final String[] cidrPair = cidr.split("\\/");
         if (cidrPair.length != 2) {
-            return null;
+            throw new CloudRuntimeException("cidr is not formatted correctly: "+ cidr);
         }
         final String cidrAddress = cidrPair[0];
         final String cidrSize = cidrPair[1];
         if (!isValidIp(cidrAddress)) {
-            return null;
+            throw new CloudRuntimeException("cidr is not bvalid in ip space" + cidr);
         }
-        int cidrSizeNum = -1;
-
-        try {
-            cidrSizeNum = Integer.parseInt(cidrSize);
-        } catch (final Exception e) {
-            return null;
-        }
-        final long numericNetmask = 0xffffffff >> MAX_CIDR - cidrSizeNum << MAX_CIDR - cidrSizeNum;
+        long cidrSizeNum = getCidrSizeFromString(cidrSize);
+        final long numericNetmask = (long)0xffffffff >> MAX_CIDR - cidrSizeNum << MAX_CIDR - cidrSizeNum;
         final long ipAddr = ip2Long(cidrAddress);
         final Long[] cidrlong = {ipAddr & numericNetmask, (long)cidrSizeNum};
         return cidrlong;
 
     }
 
+    /**
+     * @param cidrSize
+     * @return
+     * @throws CloudRuntimeException
+     */
+    static long getCidrSizeFromString(final String cidrSize) throws CloudRuntimeException {
+        long cidrSizeNum = -1;
+
+        try {
+            cidrSizeNum = Integer.parseInt(cidrSize);
+        } catch (final NumberFormatException e) {
+            throw new CloudRuntimeException("cidrsize is not a valid int: " + cidrSize, e);
+        }
+        if(cidrSizeNum > 32 || cidrSizeNum < 0) {// assuming IPv4
+            throw new CloudRuntimeException("cidr size out of range: " + cidrSizeNum);
+        }
+        return cidrSizeNum;
+    }
+
     public static String getCidrSubNet(final String cidr) {
         if (cidr == null || cidr.isEmpty()) {
             return null;
@@ -908,13 +921,7 @@ public class NetUtils {
         if (!isValidIp(cidrAddress)) {
             return null;
         }
-        long cidrSizeNum = -1;
-
-        try {
-            cidrSizeNum = Integer.parseInt(cidrSize);
-        } catch (final NumberFormatException e) {
-            throw new CloudRuntimeException("cidrsize is not valid", e);
-        }
+        long cidrSizeNum = getCidrSizeFromString(cidrSize);
         final long numericNetmask = (long)0xffffffff >> MAX_CIDR - cidrSizeNum << MAX_CIDR - cidrSizeNum;
         final String netmask = NetUtils.long2Ip(numericNetmask);
         return getSubNet(cidrAddress, netmask);
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 76fc4fa89fb..64ceb72411b 100644
--- a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java
+++ b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java
@@ -38,6 +38,7 @@ import java.util.TreeSet;
 import org.apache.log4j.Logger;
 import org.junit.Test;
 
+import com.cloud.utils.exception.CloudRuntimeException;
 import com.googlecode.ipv6.IPv6Address;
 
 public class NetUtilsTest {
@@ -439,4 +440,33 @@ public class NetUtilsTest {
         String subnet = NetUtils.getCidrSubNet("10.10.10.10", 16);
         assertTrue(cidr + " does not contain " + subnet,NetUtils.isIpWithtInCidrRange(subnet, cidr));
     }
+
+    @Test
+    public void testIsValidCidrSize() {
+        final String cidrsize = "16";
+        long netbits = NetUtils.getCidrSizeFromString(cidrsize);
+        assertTrue(" does not compute " + cidrsize,netbits == 16);
+    }
+
+    @Test(expected=CloudRuntimeException.class)
+    public void testIsInvalidCidrSize() {
+        final String cidrsize = "33";
+        long netbits = NetUtils.getCidrSizeFromString(cidrsize);
+        assertTrue(" does not compute " + cidrsize,netbits == 16);
+    }
+
+    @Test(expected=CloudRuntimeException.class)
+    public void testIsInvalidCidrString() {
+        final String cidrsize = "ggg";
+        long netbits = NetUtils.getCidrSizeFromString(cidrsize);
+        assertTrue(" does not compute " + cidrsize,netbits == 16);
+    }
+
+    @Test
+    public void testCidrToLongArray() {
+        final String cidr = "10.192.10.10/10";
+        Long[] netbits = NetUtils.cidrToLong(cidr);
+        assertEquals("unexpected cidrsize " + netbits[1],10l, netbits[1].longValue());
+        assertEquals("(un)expected <" + 0x0ac00000L + "> netaddress " + netbits[0].longValue(),netbits[0].longValue(),0x0ac00000l);
+    }
 }
\ No newline at end of file