Matching inverted exact/regex stream rules when field is not present #2172
Conversation
Before this change, inverted exact/regex rules did not match when the field is not present or null. This is changed and reflected in the tests. Fixes #2160.
| return rule.getInverted() ^ value.toString().trim().equals(rule.getValue()); | ||
| final String value = msg.getField(rule.getField()).toString(); | ||
|
|
||
| return rule.getInverted() ^ value.trim().equals(rule.getValue()); |
There was a problem hiding this comment.
Minor nitpick: While this works, I'd prefer using the logical OR operator here.
There was a problem hiding this comment.
This is not part of the scope of this PR.
There was a problem hiding this comment.
I think we could do some opportunistic refactoring in this case (and actually fix an error that is accidentally working).
There was a problem hiding this comment.
What is the error here?
There was a problem hiding this comment.
Using the bitwise OR operator (^) instead of the logical OR operator (||) as it's a logical expression to be evaluated and not some bit-shifting.
There was a problem hiding this comment.
This is perfectly fine for booleans, as specified in the JLS: http://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.22.2
There was a problem hiding this comment.
Hm, I think I'm getting confused now. The ^ bitwise operator and the || logical operator also have different characteristics:
For
^, the result value istrueif the operand values are different; otherwise, the result isfalse.
Is this the intended behavior? The || operator would return true if at least one of the operands is true while the ^ operator would return true only if the operands are different (e. g. both are false). So ^ is like a logical XOR.
There was a problem hiding this comment.
Yeah, this is intended. The critical difference is that if the rule is inverted, then it should not match if the message and the rule fields are equal. (field "foo" should not be "bar")
There was a problem hiding this comment.
✅ I interpreted the intention of that expression completely wrong. Everything's fine.
|
LGTM. 👍 |
Before this change, inverted exact/regex stream rules did not match when the field was not present. This was counter intuitive and perceived as erroneus, this PR Is changing this behavior.
Fixes #2160