From d1e1be5cc112a427312442f43294e5de76fece7b Mon Sep 17 00:00:00 2001 From: Pramila Singh Date: Fri, 6 Sep 2013 14:33:05 -0700 Subject: [PATCH] Fix for exceptions seen in FlowProgrammer NB, when get/put is done for invalid flow Change-Id: I6a247072bab9a21c82d7247a852c0d19766bcf13 Signed-off-by: Pramila Singh --- .../forwardingrulesmanager/FlowConfig.java | 199 +++++++++--------- .../forwardingrulesmanager/frmTest.java | 8 +- .../northbound/FlowProgrammerNorthbound.java | 4 + 3 files changed, 110 insertions(+), 101 deletions(-) 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 0304af493d..ba69c8a3d2 100644 --- 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 176d8e9ebd..a3e1ded141 100644 --- 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 actions = new ArrayList(); + 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 2d270b44f9..7bd36a3cbf 100644 --- 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); -- 2.36.6