Add CelOptions to disable comprehension

Fixes #484 PiperOrigin-RevId: 700440718
google · Nov 26, 2024 · 4ca869b · 4ca869b
1 parent e104ba7
commit 4ca869b
Show file tree

Hide file tree

Showing 5 changed files with 127 additions and 3 deletions.
diff --git a/bundle/src/test/java/dev/cel/bundle/BUILD.bazel b/bundle/src/test/java/dev/cel/bundle/BUILD.bazel
@@ -18,6 +18,7 @@ java_library(
         "//checker:proto_type_mask",
         "//common",
         "//common:compiler_common",
+        "//common:error_codes",
         "//common:options",
         "//common:proto_ast",
         "//common/ast",

diff --git a/bundle/src/test/java/dev/cel/bundle/CelImplTest.java b/bundle/src/test/java/dev/cel/bundle/CelImplTest.java
@@ -63,6 +63,7 @@
 import dev.cel.checker.ProtoTypeMask;
 import dev.cel.checker.TypeProvider;
 import dev.cel.common.CelAbstractSyntaxTree;
+import dev.cel.common.CelErrorCode;
 import dev.cel.common.CelIssue;
 import dev.cel.common.CelOptions;
 import dev.cel.common.CelProtoAbstractSyntaxTree;
@@ -1961,6 +1962,73 @@ public void program_nativeTypeUnknownsEnabled_asCallArguments() throws Exception
     assertThat(result.attributes()).isEmpty();
   }
 
+  @Test
+  @TestParameters("{expression: 'string(123)'}")
+  @TestParameters("{expression: 'string(123u)'}")
+  @TestParameters("{expression: 'string(1.5)'}")
+  @TestParameters("{expression: 'string(\"foo\")'}")
+  @TestParameters("{expression: 'string(b\"foo\")'}")
+  @TestParameters("{expression: 'string(timestamp(100))'}")
+  @TestParameters("{expression: 'string(duration(\"1h\"))'}")
+  public void program_stringConversionDisabled_throws(String expression) throws Exception {
+    Cel cel =
+        CelFactory.standardCelBuilder()
+            .setOptions(
+                CelOptions.current()
+                    .enableTimestampEpoch(true)
+                    .enableStringConversion(false)
+                    .build())
+            .build();
+    CelAbstractSyntaxTree ast = cel.compile(expression).getAst();
+
+    CelEvaluationException e =
+        assertThrows(CelEvaluationException.class, () -> cel.createProgram(ast).eval());
+    assertThat(e).hasMessageThat().contains("No matching overload for function 'string'");
+    assertThat(e.getErrorCode()).isEqualTo(CelErrorCode.OVERLOAD_NOT_FOUND);
+  }
+
+  @Test
+  public void program_stringConcatenationDisabled_throws() throws Exception {
+    Cel cel =
+        CelFactory.standardCelBuilder()
+            .setOptions(CelOptions.current().enableStringConcatenation(false).build())
+            .build();
+    CelAbstractSyntaxTree ast = cel.compile("'foo' + 'bar'").getAst();
+
+    CelEvaluationException e =
+        assertThrows(CelEvaluationException.class, () -> cel.createProgram(ast).eval());
+    assertThat(e).hasMessageThat().contains("No matching overload for function '_+_'");
+    assertThat(e.getErrorCode()).isEqualTo(CelErrorCode.OVERLOAD_NOT_FOUND);
+  }
+
+  @Test
+  public void program_listConcatenationDisabled_throws() throws Exception {
+    Cel cel =
+        CelFactory.standardCelBuilder()
+            .setOptions(CelOptions.current().enableListConcatenation(false).build())
+            .build();
+    CelAbstractSyntaxTree ast = cel.compile("[1] + [2]").getAst();
+
+    CelEvaluationException e =
+        assertThrows(CelEvaluationException.class, () -> cel.createProgram(ast).eval());
+    assertThat(e).hasMessageThat().contains("No matching overload for function '_+_'");
+    assertThat(e.getErrorCode()).isEqualTo(CelErrorCode.OVERLOAD_NOT_FOUND);
+  }
+
+  @Test
+  public void program_comprehensionDisabled_throws() throws Exception {
+    Cel cel =
+        standardCelBuilderWithMacros()
+            .setOptions(CelOptions.current().enableComprehension(false).build())
+            .build();
+    CelAbstractSyntaxTree ast = cel.compile("['foo', 'bar'].map(x, x)").getAst();
+
+    CelEvaluationException e =
+        assertThrows(CelEvaluationException.class, () -> cel.createProgram(ast).eval());
+    assertThat(e).hasMessageThat().contains("Iteration budget exceeded: 0");
+    assertThat(e.getErrorCode()).isEqualTo(CelErrorCode.ITERATION_BUDGET_EXCEEDED);
+  }
+
   @Test
   public void toBuilder_isImmutable() {
     CelBuilder celBuilder = CelFactory.standardCelBuilder();

diff --git a/common/src/main/java/dev/cel/common/CelOptions.java b/common/src/main/java/dev/cel/common/CelOptions.java
@@ -109,6 +109,14 @@ public enum ProtoUnsetFieldOptions {
 
   public abstract ProtoUnsetFieldOptions fromProtoUnsetFieldOption();
 
+  public abstract boolean enableStringConversion();
+
+  public abstract boolean enableStringConcatenation();
+
+  public abstract boolean enableListConcatenation();
+
+  public abstract boolean enableComprehension();
+
   public abstract Builder toBuilder();
 
   public ImmutableSet<ExprFeatures> toExprFeatures() {
@@ -200,7 +208,11 @@ public static Builder newBuilder() {
         .enableCelValue(false)
         .comprehensionMaxIterations(-1)
         .unwrapWellKnownTypesOnFunctionDispatch(true)
-        .fromProtoUnsetFieldOption(ProtoUnsetFieldOptions.BIND_DEFAULT);
+        .fromProtoUnsetFieldOption(ProtoUnsetFieldOptions.BIND_DEFAULT)
+        .enableStringConversion(true)
+        .enableStringConcatenation(true)
+        .enableListConcatenation(true)
+        .enableComprehension(true);
   }
 
   /**
@@ -504,6 +516,31 @@ public abstract static class Builder {
      */
     public abstract Builder fromProtoUnsetFieldOption(ProtoUnsetFieldOptions value);
 
+    /**
+     * Enables string() overloads for the runtime. This option exists to maintain parity with
+     * cel-cpp interpreter options.
+     */
+    public abstract Builder enableStringConversion(boolean value);
+
+    /**
+     * Enables string concatenation overload for the runtime. This option exists to maintain parity
+     * with cel-cpp interpreter options.
+     */
+    public abstract Builder enableStringConcatenation(boolean value);
+
+    /**
+     * Enables list concatenation overload for the runtime. This option exists to maintain parity
+     * with cel-cpp interpreter options.
+     */
+    public abstract Builder enableListConcatenation(boolean value);
+
+    /**
+     * Enables comprehension (macros) for the runtime. Setting false has the same effect with
+     * assigning 0 for {@link #comprehensionMaxIterations()}. This option exists to maintain parity
+     * with cel-cpp interpreter options.
+     */
+    public abstract Builder enableComprehension(boolean value);
+
     public abstract CelOptions build();
   }
 }
diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java
@@ -43,6 +43,7 @@
 import dev.cel.common.types.CelTypes;
 import dev.cel.common.values.CelValueProvider;
 import dev.cel.common.values.ProtoMessageValueProvider;
+import dev.cel.runtime.CelStandardFunctions.StandardFunction.Overload.Arithmetic;
 import dev.cel.runtime.CelStandardFunctions.StandardFunction.Overload.Comparison;
 import dev.cel.runtime.CelStandardFunctions.StandardFunction.Overload.Conversions;
 import java.util.Arrays;
@@ -317,6 +318,22 @@ private ImmutableSet<CelFunctionBinding> newStandardFunctionBindings(
                             return options.enableTimestampEpoch();
                           }
                           break;
+                        case STRING:
+                          if (!options.enableStringConversion()) {
+                            return false;
+                          }
+                          break;
+                        case ADD:
+                          Arithmetic arithmetic = (Arithmetic) standardOverload;
+                          if (!options.enableStringConcatenation()
+                              && arithmetic.equals(Arithmetic.ADD_STRING)) {
+                            return false;
+                          }
+                          if (!options.enableListConcatenation()
+                              && arithmetic.equals(Arithmetic.ADD_LIST)) {
+                            return false;
+                          }
+                          break;
                         default:
                           if (standardOverload instanceof Comparison
                               && !options.enableHeterogeneousNumericComparisons()) {

diff --git a/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java b/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java
@@ -179,9 +179,10 @@ public Object evalTrackingUnknowns(
         Optional<? extends FunctionResolver> functionResolver,
         CelEvaluationListener listener)
         throws InterpreterException {
+      int comprehensionMaxIterations =
+          celOptions.enableComprehension() ? celOptions.comprehensionMaxIterations() : 0;
       ExecutionFrame frame =
-          new ExecutionFrame(
-              listener, resolver, functionResolver, celOptions.comprehensionMaxIterations());
+          new ExecutionFrame(listener, resolver, functionResolver, comprehensionMaxIterations);
       IntermediateResult internalResult = evalInternal(frame, ast.getExpr());
       return internalResult.value();
     }