server: fix ClassCastException importing an ACL rule without a traffic type - #14051
server: fix ClassCastException importing an ACL rule without a traffic type#14051nagaboinaramgopal wants to merge 1 commit into
Conversation
…c type createACLRuleFromMap passed the enum NetworkACLItem.TrafficType.Ingress as the getOrDefault default for a String variable, so importing a rule that omits the optional traffictype threw ClassCastException and every such rule was silently rejected into the error list. Use the enum's string form so the intended Ingress default applies.
| } | ||
| String action = (String) ruleMap.getOrDefault(ApiConstants.ACTION, "deny"); | ||
| String trafficType = (String) ruleMap.getOrDefault(ApiConstants.TRAFFIC_TYPE, NetworkACLItem.TrafficType.Ingress); | ||
| String trafficType = (String) ruleMap.getOrDefault(ApiConstants.TRAFFIC_TYPE, NetworkACLItem.TrafficType.Ingress.toString()); |
There was a problem hiding this comment.
I do not think this is needed, the object can be passed as is and the string cast will take care toString() is called on the result.
There was a problem hiding this comment.
@nagaboinaramgopal , did you encounter an issue that made you implement this?
There was a problem hiding this comment.
Thanks @DaanHoogland. I dug into this one a bit and the catch is the default value. When traffictype is left out of the rule map, getOrDefault(TRAFFIC_TYPE, TrafficType.Ingress) returns the TrafficType enum constant, and casting an enum to String with (String) throws a ClassCastException rather than converting it, so the rule drops into the errors list. The two neighbouring defaults on the same lines are Strings ("deny" and "true"), which is why only the traffic type trips. The test createACLRuleFromMapDefaultsTrafficTypeToIngress covers the no-traffictype path. If you would rather, I can switch it to String.valueOf(...) or lift the default into a constant, whatever reads cleanest to you.
There was a problem hiding this comment.
best reading ;) this would be an implementation of toString in the enum. But to be honest, if this is not solving a real live issue, … we are poor on test-resources so again, is this a real live issue?
2 participants
createACLRuleFromMap passed the enum NetworkACLItem.TrafficType.Ingress as the getOrDefault default for a String variable, so importing a rule that omits the optional traffictype threw ClassCastException and every such rule was silently rejected. Use the enum's string form so the intended Ingress default applies.
Tested: new unit test createACLRuleFromMapDefaultsTrafficTypeToIngress (fails before, passes after); NetworkACLServiceImplTest green.