Block index fix

Author: l46kok

optimizer/src/test/java/dev/cel/optimizer/optimizers/SubexpressionOptimizerTest.java

@@ -52,6 +52,7 @@
 import dev.cel.parser.CelStandardMacro;
 import dev.cel.parser.CelUnparser;
 import dev.cel.parser.CelUnparserFactory;
+import dev.cel.runtime.CelEvaluationException;
 import dev.cel.runtime.CelFunctionBinding;
 import dev.cel.runtime.CelRuntime;
 import dev.cel.runtime.CelRuntimeFactory;
@@ -579,6 +580,23 @@ public void verifyOptimizedAstCorrectness_indexIsNotForwardReferencing_throws(St
         .contains("Illegal block index found. The index value must be less than");
   }
 
+  @Test
+  public void block_containsCycle_throws() throws Exception {
+    CelAbstractSyntaxTree ast = compileUsingInternalFunctions("cel.block([index1,index0],index0)");
+
+    CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> CEL.createProgram(ast).eval());
+    assertThat(e).hasMessageThat().contains("Cycle detected: @index0");
+  }
+
+  @Test
+  public void block_withNonStrictFunctions_noCycle_success() throws Exception {
+    CelAbstractSyntaxTree ast = compileUsingInternalFunctions("cel.block([1/0 > 0, (index0 && false) && (index0 && true)],index1)");
+
+    boolean result = (boolean) CEL.createProgram(ast).eval();
+
+    assertThat(result).isFalse();
+  }
+
   /**
    * Converts AST containing cel.block related test functions to internal functions (e.g: cel.block
    * -> cel.@block)

runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java

@@ -25,6 +25,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.errorprone.annotations.Immutable;
+import java.util.Set;
 import javax.annotation.concurrent.ThreadSafe;
 import com.google.protobuf.ByteString;
 import com.google.protobuf.NullValue;
@@ -343,7 +344,13 @@ private IntermediateResult resolveIdent(ExecutionFrame frame, CelExpr expr, Stri
       Object value = rawResult.value();
       boolean isLazyExpression = value instanceof LazyExpression;
       if (isLazyExpression) {
-        value = evalInternal(frame, ((LazyExpression) value).celExpr).value();
+        frame.markLazyEvaluationOrThrow(name);
+
+        try {
+          value = evalInternal(frame, ((LazyExpression) value).celExpr).value();
+        } finally {
+          frame.endLazyEvaluation(name);
+        }
       }
 
       // Value resolved from Binding, it could be Message, PartialMessage or unbound(null)
@@ -1065,7 +1072,11 @@ private IntermediateResult evalCelBlock(
       }
       frame.pushLazyScope(Collections.unmodifiableMap(blockList));
 
-      return evalInternal(frame, blockCall.args().get(1));
+      try {
+        return evalInternal(frame, blockCall.args().get(1));
+      } finally {
+        frame.popScope();
+      }
     }
 
     private CelType getCheckedTypeOrThrow(CelExpr expr) throws CelEvaluationException {
@@ -1115,6 +1126,7 @@ static class ExecutionFrame {
     private final int maxIterations;
     private final ArrayDeque<RuntimeUnknownResolver> resolvers;
     private final Optional<? extends CelFunctionResolver> lateBoundFunctionResolver;
+    private final Set<String> activeLazyAttributes = new HashSet<>();
     private RuntimeUnknownResolver currentResolver;
     private int iterations;
     @VisibleForTesting int scopeLevel;
@@ -1132,6 +1144,17 @@ private ExecutionFrame(
       this.maxIterations = maxIterations;
     }
 
+    private void markLazyEvaluationOrThrow(String name) {
+      boolean added = activeLazyAttributes.add(name);
+      if (!added) {
+        throw new IllegalStateException(String.format("Cycle detected: %s", name));
+      }
+    }
+
+    private void endLazyEvaluation(String name) {
+      activeLazyAttributes.remove(name);
+    }
+
     private Optional<CelEvaluationListener> getEvaluationListener() {
       return evaluationListener;
     }