홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Thomas Vachuska
a6c0d044 1 parent f1fa96cc

Fixed a dual-level NPE involving flow-rule payload. We need more unit tests and … 

…automated system tests!!!

Change-Id: I821214cc274377dc22cb5ab88c48567eaab778cb
Inline Side-by-side
Showing 1 changed file with 17 additions and 8 deletions
...@@ -37,6 +37,7 @@ import org.onosproject.net.flow.FlowEntry; ...@@ -37,6 +37,7 @@ import org.onosproject.net.flow.FlowEntry;
37 import org.onosproject.net.flow.FlowRule; 37 import org.onosproject.net.flow.FlowRule;
38 import org.onosproject.net.flow.FlowRuleBatchEntry; 38 import org.onosproject.net.flow.FlowRuleBatchEntry;
39 import org.onosproject.net.flow.FlowRuleBatchOperation; 39 import org.onosproject.net.flow.FlowRuleBatchOperation;
40 +import org.onosproject.net.flow.FlowRuleExtPayLoad;
40 import org.onosproject.net.flow.FlowRuleProvider; 41 import org.onosproject.net.flow.FlowRuleProvider;
41 import org.onosproject.net.flow.FlowRuleProviderRegistry; 42 import org.onosproject.net.flow.FlowRuleProviderRegistry;
42 import org.onosproject.net.flow.FlowRuleProviderService; 43 import org.onosproject.net.flow.FlowRuleProviderService;
...@@ -147,8 +148,9 @@ public class OpenFlowRuleProvider extends AbstractProvider ...@@ -147,8 +148,9 @@ public class OpenFlowRuleProvider extends AbstractProvider
147 private void applyRule(FlowRule flowRule) { 148 private void applyRule(FlowRule flowRule) {
148 OpenFlowSwitch sw = controller.getSwitch(Dpid.dpid(flowRule.deviceId() 149 OpenFlowSwitch sw = controller.getSwitch(Dpid.dpid(flowRule.deviceId()
149 .uri())); 150 .uri()));
150 - if (flowRule.payLoad().payLoad().length > 0) { 151 + FlowRuleExtPayLoad flowRuleExtPayLoad = flowRule.payLoad();
151 - OFMessage msg = new ThirdPartyMessage(flowRule.payLoad().payLoad()); 152 + if (hasPayload(flowRuleExtPayLoad)) {
153 + OFMessage msg = new ThirdPartyMessage(flowRuleExtPayLoad.payLoad());
152 sw.sendMsg(msg); 154 sw.sendMsg(msg);
153 return; 155 return;
154 } 156 }
...@@ -161,14 +163,14 @@ public class OpenFlowRuleProvider extends AbstractProvider ...@@ -161,14 +163,14 @@ public class OpenFlowRuleProvider extends AbstractProvider
161 for (FlowRule flowRule : flowRules) { 163 for (FlowRule flowRule : flowRules) {
162 removeRule(flowRule); 164 removeRule(flowRule);
163 } 165 }
164 -
165 } 166 }
166 167
167 private void removeRule(FlowRule flowRule) { 168 private void removeRule(FlowRule flowRule) {
168 OpenFlowSwitch sw = controller.getSwitch(Dpid.dpid(flowRule.deviceId() 169 OpenFlowSwitch sw = controller.getSwitch(Dpid.dpid(flowRule.deviceId()
169 .uri())); 170 .uri()));
170 - if (flowRule.payLoad().payLoad().length > 0) { 171 + FlowRuleExtPayLoad flowRuleExtPayLoad = flowRule.payLoad();
171 - OFMessage msg = new ThirdPartyMessage(flowRule.payLoad().payLoad()); 172 + if (hasPayload(flowRuleExtPayLoad)) {
173 + OFMessage msg = new ThirdPartyMessage(flowRuleExtPayLoad.payLoad());
172 sw.sendMsg(msg); 174 sw.sendMsg(msg);
173 return; 175 return;
174 } 176 }
...@@ -192,9 +194,10 @@ public class OpenFlowRuleProvider extends AbstractProvider ...@@ -192,9 +194,10 @@ public class OpenFlowRuleProvider extends AbstractProvider
192 OFFlowMod mod; 194 OFFlowMod mod;
193 for (FlowRuleBatchEntry fbe : batch.getOperations()) { 195 for (FlowRuleBatchEntry fbe : batch.getOperations()) {
194 // flow is the third party privacy flow 196 // flow is the third party privacy flow
195 - if (fbe.target().payLoad().payLoad().length > 0) { 197 +
196 - OFMessage msg = new ThirdPartyMessage(fbe.target().payLoad() 198 + FlowRuleExtPayLoad flowRuleExtPayLoad = fbe.target().payLoad();
197 - .payLoad()); 199 + if (hasPayload(flowRuleExtPayLoad)) {
200 + OFMessage msg = new ThirdPartyMessage(flowRuleExtPayLoad.payLoad());
198 sw.sendMsg(msg); 201 sw.sendMsg(msg);
199 continue; 202 continue;
200 } 203 }
...@@ -222,6 +225,12 @@ public class OpenFlowRuleProvider extends AbstractProvider ...@@ -222,6 +225,12 @@ public class OpenFlowRuleProvider extends AbstractProvider
222 sw.sendMsg(builder.build()); 225 sw.sendMsg(builder.build());
223 } 226 }
224 227
228 + private boolean hasPayload(FlowRuleExtPayLoad flowRuleExtPayLoad) {
229 + return flowRuleExtPayLoad != null &&
230 + flowRuleExtPayLoad.payLoad() != null &&
231 + flowRuleExtPayLoad.payLoad().length > 0;
232 + }
233 +
225 private class InternalFlowProvider 234 private class InternalFlowProvider
226 implements OpenFlowSwitchListener, OpenFlowEventListener { 235 implements OpenFlowSwitchListener, OpenFlowEventListener {
227 236
......