Repository: metron Updated Branches: refs/heads/master bfa7491f1 -> 05d309f91
METRON-1877 Nested IF ELSE statements can cause parse errors in Stellar (justinleet) closes apache/metron#1268 Project: http://git-wip-us.apache.org/repos/asf/metron/repo Commit: http://git-wip-us.apache.org/repos/asf/metron/commit/05d309f9 Tree: http://git-wip-us.apache.org/repos/asf/metron/tree/05d309f9 Diff: http://git-wip-us.apache.org/repos/asf/metron/diff/05d309f9 Branch: refs/heads/master Commit: 05d309f916c0eb45a287a105f44e16e93e3c378e Parents: bfa7491 Author: justinleet <[email protected]> Authored: Tue Nov 20 08:36:53 2018 -0500 Committer: leet <[email protected]> Committed: Tue Nov 20 08:36:53 2018 -0500 ---------------------------------------------------------------------- .../metron/stellar/common/StellarCompiler.java | 23 +++++-- .../stellar/dsl/functions/BasicStellarTest.java | 68 ++++++++++++++++++++ 2 files changed, 84 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/metron/blob/05d309f9/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/StellarCompiler.java ---------------------------------------------------------------------- diff --git a/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/StellarCompiler.java b/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/StellarCompiler.java index 8a328a2..ac5412b 100644 --- a/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/StellarCompiler.java +++ b/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/StellarCompiler.java @@ -156,20 +156,21 @@ public class StellarCompiler extends StellarBaseListener { public Object apply(ExpressionState state) { Deque<Token<?>> instanceDeque = new ArrayDeque<>(); { - boolean skipElse = false; + int skipElseCount = 0; boolean skipMatchClauses = false; Token<?> token = null; for (Iterator<Token<?>> it = getTokenDeque().descendingIterator(); it.hasNext(); ) { token = it.next(); //if we've skipped an else previously, then we need to skip the deferred tokens associated with the else. - if (skipElse && token.getUnderlyingType() == ElseExpr.class) { + if (skipElseCount > 0 && token.getUnderlyingType() == ElseExpr.class) { while (it.hasNext()) { token = it.next(); if (token.getUnderlyingType() == EndConditional.class) { break; } } - skipElse = false; + // We've completed a single else. + skipElseCount--; } if (skipMatchClauses && (token.getUnderlyingType() == MatchClauseEnd.class || token.getUnderlyingType() == MatchClauseCheckExpr.class)) { @@ -219,14 +220,22 @@ public class StellarCompiler extends StellarBaseListener { //short circuit the if/then/else instanceDeque.pop(); if((Boolean)curr.getValue()) { - //choose then - skipElse = true; + //choose then. Need to make sure we're keeping track of nesting. + skipElseCount++; } else { //choose else + // Need to count in case we see another if-else, to avoid breaking on wrong else. + int innerIfCount = 0; while (it.hasNext()) { Token<?> t = it.next(); - if (t.getUnderlyingType() == ElseExpr.class) { - break; + if (t.getUnderlyingType() == IfExpr.class) { + innerIfCount++; + } else if (t.getUnderlyingType() == ElseExpr.class) { + if (innerIfCount == 0) { + break; + } else { + innerIfCount--; + } } } } http://git-wip-us.apache.org/repos/asf/metron/blob/05d309f9/metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/dsl/functions/BasicStellarTest.java ---------------------------------------------------------------------- diff --git a/metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/dsl/functions/BasicStellarTest.java b/metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/dsl/functions/BasicStellarTest.java index 79f97bc..97cb970 100644 --- a/metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/dsl/functions/BasicStellarTest.java +++ b/metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/dsl/functions/BasicStellarTest.java @@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableSet; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -917,6 +918,73 @@ public class BasicStellarTest { } @Test + public void testShortCircuit_nestedIf() throws Exception { + // Single nested + // IF true + // THEN IF true + // THEN 'a' + // ELSE 'b' + // ELSE 'c' + Assert.assertEquals("a", run("IF true THEN IF true THEN 'a' ELSE 'b' ELSE 'c'", new HashMap<>())); + Assert.assertEquals("b", run("IF true THEN IF false THEN 'a' ELSE 'b' ELSE 'c'", new HashMap<>())); + Assert.assertEquals("c", run("IF false THEN IF false THEN 'a' ELSE 'b' ELSE 'c'", new HashMap<>())); + + // 3 layer deep + // IF true + // THEN IF true + // THEN IF true + // THEN 'a' + // ELSE 'b' + // ELSE 'c' + // ELSE 'd' + Assert.assertEquals("a", run("IF true THEN IF true THEN IF true THEN 'a' ELSE 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("b", run("IF true THEN IF true THEN IF false THEN 'a' ELSE 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("c", run("IF true THEN IF false THEN IF true THEN 'a' ELSE 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("c", run("IF true THEN IF false THEN IF false THEN 'a' ELSE 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("d", run("IF false THEN IF true THEN IF true THEN 'a' ELSE 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("d", run("IF false THEN IF true THEN IF false THEN 'a' ELSE 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("d", run("IF false THEN IF false THEN IF true THEN 'a' ELSE 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("d", run("IF false THEN IF false THEN IF false THEN 'a' ELSE 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + + // Nested inside inner if + // IF true + // THEN IF true + // THEN 'a' + // ELSE IF true + // THEN 'b' + // ELSE 'c' + // ELSE 'd' + Assert.assertEquals("a", run("IF true THEN IF true THEN 'a' ELSE IF true THEN 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("a", run("IF true THEN IF true THEN 'a' ELSE IF false THEN 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("b", run("IF true THEN IF false THEN 'a' ELSE IF true THEN 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("c", run("IF true THEN IF false THEN 'a' ELSE IF false THEN 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("d", run("IF false THEN IF true THEN 'a' ELSE IF true THEN 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("d", run("IF false THEN IF true THEN 'a' ELSE IF false THEN 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("d", run("IF false THEN IF false THEN 'a' ELSE IF true THEN 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + Assert.assertEquals("d", run("IF false THEN IF false THEN 'a' ELSE IF false THEN 'b' ELSE 'c' ELSE 'd'", new HashMap<>())); + } + + @Test + public void testShortCircuit_complexNested() { + // IF TO_UPPER('foo') == 'FOO' + // THEN IF GET_FIRST(MAP(['test_true'], x -> TO_UPPER(x))) == 'TEST_TRUE' + // THEN match{ var1 < 12 => 'less', var1 >= 10 => 'more', default => 'default'} + // ELSE 'b' + // ELSE 'c' + + // Should hit the match cases + Assert.assertEquals("less", run("IF TO_UPPER('foo') == 'FOO' THEN IF GET_FIRST(MAP(['test_true'], x -> TO_UPPER(x))) == 'TEST_TRUE' THEN match{ var1 < 10 => 'less', var1 >= 12 => 'more', default => 'default'} ELSE 'b' ELSE 'c'", Collections.singletonMap("var1", 1))); + Assert.assertEquals("default", run("IF TO_UPPER('foo') == 'FOO' THEN IF GET_FIRST(MAP(['test_true'], x -> TO_UPPER(x))) == 'TEST_TRUE' THEN match{ var1 < 10 => 'less', var1 >= 12 => 'more', default => 'default'} ELSE 'b' ELSE 'c'", Collections.singletonMap("var1", 11))); + Assert.assertEquals("more", run("IF TO_UPPER('foo') == 'FOO' THEN IF GET_FIRST(MAP(['test_true'], x -> TO_UPPER(x))) == 'TEST_TRUE' THEN match{ var1 < 10 => 'less', var1 >= 12 => 'more', default => 'default'} ELSE 'b' ELSE 'c'", Collections.singletonMap("var1", 100))); + + // Should fail first if and hit 'c' + Assert.assertEquals("c", run("IF TO_UPPER('bar') == 'FOO' THEN IF GET_FIRST(MAP(['test_true'], x -> TO_UPPER(x))) == 'TEST_TRUE' THEN match{ var1 < 10 => 'less', var1 >= 12 => 'more', default => 'default'} ELSE 'b' ELSE 'c'", Collections.singletonMap("var1", 1))); + + // Should fail second if and hit 'b' + Assert.assertEquals("b", run("IF TO_UPPER('foo') == 'FOO' THEN IF GET_FIRST(MAP(['test_false'], x -> TO_UPPER(x))) == 'TEST_TRUE' THEN match{ var1 < 10 => 'less', var1 >= 12 => 'more', default => 'default'} ELSE 'b' ELSE 'c'", Collections.singletonMap("var1", 1))); + } + + @Test public void testShortCircuit_boolean() throws Exception { Assert.assertTrue(runPredicate("'metron' in ['metron', 'metronicus', 'mortron'] or (true or THROW('exception'))", new DefaultVariableResolver(x -> null,x -> false))); Assert.assertTrue(runPredicate("true or (true or THROW('exception'))", new DefaultVariableResolver(x -> null,x -> false)));
