Fix for exceptions seen in FlowProgrammer NB, when get/put is done for invalid flow

Change-Id: I6a247072bab9a21c82d7247a852c0d19766bcf13
Signed-off-by: Pramila Singh <pramisin@cisco.com>

diff --git a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java
index 0304af493d9fe64abab2df516bc5b3c2e63c098c..ba69c8a3d29f8286f2d4f3157f6547a8f54c717b 100644 (file)
--- a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java
+++ b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java
@@ -832,127 +832,128 @@ public class FlowConfig implements Serializable {
             }
 
             Matcher sstr;
-            if (actions != null && !actions.isEmpty()) {
-                for (String actiongrp : actions) {
-                    // check output ports
-                    sstr = Pattern.compile("OUTPUT=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        for (String t : sstr.group(1).split(",")) {
-                            Matcher n = Pattern.compile("(?:(\\d+))").matcher(t);
-                            if (n.matches()) {
-                                if (n.group(1) != null) {
-                                    Short port = Short.parseShort(n.group(1));
-                                    if (isPortValid(sw, port) == false) {
-                                        String msg = String.format("Output port %d is not valid for this switch", port);
-                                        if (!containerName.equals(GlobalConstants.DEFAULT.toString())) {
-                                            msg += " in Container " + containerName;
-                                        }
-                                        return new Status(StatusCode.BADREQUEST, msg);
+            if (actions == null || actions.isEmpty()) {
+                return new Status(StatusCode.BADREQUEST, "Actions value is null or empty");
+            }
+            for (String actiongrp : actions) {
+                // check output ports
+                sstr = Pattern.compile("OUTPUT=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    for (String t : sstr.group(1).split(",")) {
+                        Matcher n = Pattern.compile("(?:(\\d+))").matcher(t);
+                        if (n.matches()) {
+                            if (n.group(1) != null) {
+                                Short port = Short.parseShort(n.group(1));
+                                if (isPortValid(sw, port) == false) {
+                                    String msg = String.format("Output port %d is not valid for this switch", port);
+                                    if (!containerName.equals(GlobalConstants.DEFAULT.toString())) {
+                                        msg += " in Container " + containerName;
                                     }
+                                    return new Status(StatusCode.BADREQUEST, msg);
                                 }
                             }
                         }
-                        continue;
                     }
-                    // Check src IP
-                    sstr = Pattern.compile(ActionType.FLOOD.toString()).matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if (!containerName.equals(GlobalConstants.DEFAULT.toString())) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "flood is not allowed in container %s", containerName));
-                        }
-                        continue;
+                    continue;
+                }
+                // Check src IP
+                sstr = Pattern.compile(ActionType.FLOOD.toString()).matcher(actiongrp);
+                if (sstr.matches()) {
+                    if (!containerName.equals(GlobalConstants.DEFAULT.toString())) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "flood is not allowed in container %s", containerName));
                     }
-                    // Check src IP
-                    sstr = Pattern.compile(ActionType.SET_NW_SRC.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if (!NetUtils.isIPv4AddressValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format("IP source address %s is not valid",
-                                    sstr.group(1)));
-                        }
-                        continue;
+                    continue;
+                }
+                // Check src IP
+                sstr = Pattern.compile(ActionType.SET_NW_SRC.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if (!NetUtils.isIPv4AddressValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format("IP source address %s is not valid",
+                                sstr.group(1)));
                     }
-                    // Check dst IP
-                    sstr = Pattern.compile(ActionType.SET_NW_DST.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if (!NetUtils.isIPv4AddressValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "IP destination address %s is not valid", sstr.group(1)));
-                        }
-                        continue;
+                    continue;
+                }
+                // Check dst IP
+                sstr = Pattern.compile(ActionType.SET_NW_DST.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if (!NetUtils.isIPv4AddressValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "IP destination address %s is not valid", sstr.group(1)));
                     }
+                    continue;
+                }
 
-                    sstr = Pattern.compile(ActionType.SET_VLAN_ID.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if ((sstr.group(1) != null) && !isVlanIdValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "Vlan ID %s is not in the range 0 - 4095", sstr.group(1)));
-                        }
-                        continue;
+                sstr = Pattern.compile(ActionType.SET_VLAN_ID.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if ((sstr.group(1) != null) && !isVlanIdValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "Vlan ID %s is not in the range 0 - 4095", sstr.group(1)));
                     }
+                    continue;
+                }
 
-                    sstr = Pattern.compile(ActionType.SET_VLAN_PCP.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if ((sstr.group(1) != null) && !isVlanPriorityValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "Vlan priority %s is not in the range 0 - 7", sstr.group(1)));
-                        }
-                        continue;
+                sstr = Pattern.compile(ActionType.SET_VLAN_PCP.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if ((sstr.group(1) != null) && !isVlanPriorityValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "Vlan priority %s is not in the range 0 - 7", sstr.group(1)));
                     }
+                    continue;
+                }
 
-                    sstr = Pattern.compile(ActionType.SET_DL_SRC.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if ((sstr.group(1) != null) && !isL2AddressValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "Ethernet source address %s is not valid. Example: 00:05:b9:7c:81:5f",
-                                    sstr.group(1)));
-                        }
-                        continue;
+                sstr = Pattern.compile(ActionType.SET_DL_SRC.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if ((sstr.group(1) != null) && !isL2AddressValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "Ethernet source address %s is not valid. Example: 00:05:b9:7c:81:5f",
+                                sstr.group(1)));
                     }
-                    sstr = Pattern.compile(ActionType.SET_DL_DST.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if ((sstr.group(1) != null) && !isL2AddressValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "Ethernet destination address %s is not valid. Example: 00:05:b9:7c:81:5f",
-                                    sstr.group(1)));
-                        }
-                        continue;
+                    continue;
+                }
+                sstr = Pattern.compile(ActionType.SET_DL_DST.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if ((sstr.group(1) != null) && !isL2AddressValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "Ethernet destination address %s is not valid. Example: 00:05:b9:7c:81:5f",
+                                sstr.group(1)));
                     }
+                    continue;
+                }
 
-                    sstr = Pattern.compile(ActionType.SET_NW_TOS.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if ((sstr.group(1) != null) && !isTOSBitsValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "IP ToS bits %s is not in the range 0 - 63", sstr.group(1)));
-                        }
-                        continue;
+                sstr = Pattern.compile(ActionType.SET_NW_TOS.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if ((sstr.group(1) != null) && !isTOSBitsValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "IP ToS bits %s is not in the range 0 - 63", sstr.group(1)));
                     }
+                    continue;
+                }
 
-                    sstr = Pattern.compile(ActionType.SET_TP_SRC.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if ((sstr.group(1) != null) && !isTpPortValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "Transport source port %s is not valid", sstr.group(1)));
-                        }
-                        continue;
+                sstr = Pattern.compile(ActionType.SET_TP_SRC.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if ((sstr.group(1) != null) && !isTpPortValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "Transport source port %s is not valid", sstr.group(1)));
                     }
+                    continue;
+                }
 
-                    sstr = Pattern.compile(ActionType.SET_TP_DST.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if ((sstr.group(1) != null) && !isTpPortValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "Transport destination port %s is not valid", sstr.group(1)));
-                        }
-                        continue;
+                sstr = Pattern.compile(ActionType.SET_TP_DST.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if ((sstr.group(1) != null) && !isTpPortValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "Transport destination port %s is not valid", sstr.group(1)));
                     }
-                    sstr = Pattern.compile(ActionType.SET_NEXT_HOP.toString() + "=(.*)").matcher(actiongrp);
-                    if (sstr.matches()) {
-                        if (!NetUtils.isIPAddressValid(sstr.group(1))) {
-                            return new Status(StatusCode.BADREQUEST, String.format(
-                                    "IP destination address %s is not valid", sstr.group(1)));
-                        }
-                        continue;
+                    continue;
+                }
+                sstr = Pattern.compile(ActionType.SET_NEXT_HOP.toString() + "=(.*)").matcher(actiongrp);
+                if (sstr.matches()) {
+                    if (!NetUtils.isIPAddressValid(sstr.group(1))) {
+                        return new Status(StatusCode.BADREQUEST, String.format(
+                                "IP destination address %s is not valid", sstr.group(1)));
                     }
+                    continue;
                 }
             }
             // Check against the container flow

diff --git a/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java b/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java
index 176d8e9ebdfea8a2f0bb341eed68177d69a235ed..a3e1ded14178ff8d00a3efe21d02ea798fa19af2 100644 (file)
--- a/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java
+++ b/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java
@@ -19,8 +19,6 @@ import java.util.concurrent.ConcurrentMap;
 
 import org.junit.Assert;
 import org.junit.Test;
-import org.opendaylight.controller.forwardingrulesmanager.FlowConfig;
-import org.opendaylight.controller.forwardingrulesmanager.FlowEntry;
 import org.opendaylight.controller.sal.action.Action;
 import org.opendaylight.controller.sal.action.ActionType;
 import org.opendaylight.controller.sal.action.Controller;
@@ -523,6 +521,12 @@ public class frmTest {
         Assert.assertTrue(status.getDescription().contains("Node is null"));
 
         fc.setNode(Node.fromString(Node.NodeIDType.OPENFLOW, "1"));
+        Assert.assertFalse(fc.validate(null).isSuccess());
+        List<String> actions = new ArrayList<String>();
+        fc.setActions(actions);
+        Assert.assertFalse(fc.validate(null).isSuccess());
+        actions.add("OUTPUT=2");
+        fc.setActions(actions);
         Assert.assertTrue(fc.validate(null).isSuccess());
 
         fc.setPriority("-1");

diff --git a/opendaylight/northbound/flowprogrammer/src/main/java/org/opendaylight/controller/flowprogrammer/northbound/FlowProgrammerNorthbound.java b/opendaylight/northbound/flowprogrammer/src/main/java/org/opendaylight/controller/flowprogrammer/northbound/FlowProgrammerNorthbound.java
index 2d270b44f98e23d65c0f03bddab1f913aabda7d5..7bd36a3cbf5b7cebde6665f23987a28a048b6d9c 100644 (file)
--- a/opendaylight/northbound/flowprogrammer/src/main/java/org/opendaylight/controller/flowprogrammer/northbound/FlowProgrammerNorthbound.java
+++ b/opendaylight/northbound/flowprogrammer/src/main/java/org/opendaylight/controller/flowprogrammer/northbound/FlowProgrammerNorthbound.java
@@ -425,6 +425,10 @@ public class FlowProgrammerNorthbound {
                     "User is not authorized to perform this operation on container "
                             + containerName);
         }
+        if (flowConfig.getValue().getNode() == null) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("Invalid Configuration. Node is null or empty")
+                    .build();
+        }
         handleResourceCongruence(name, flowConfig.getValue().getName());
         handleResourceCongruence(nodeId, flowConfig.getValue().getNode().getNodeIDString());
         handleDefaultDisabled(containerName);