diff --git a/runtime/src/main/java/dev/cel/runtime/CelAttributePattern.java b/runtime/src/main/java/dev/cel/runtime/CelAttributePattern.java index ff5f3f5bf..9b9f70112 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelAttributePattern.java +++ b/runtime/src/main/java/dev/cel/runtime/CelAttributePattern.java @@ -19,6 +19,7 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.primitives.UnsignedLong; import com.google.errorprone.annotations.Immutable; /** @@ -124,9 +125,7 @@ public boolean isMatch(CelAttribute attribute) { int qualifierCount = min(qualifiers().size(), attribute.qualifiers().size()); int i = 0; for (; i < qualifierCount; i++) { - CelAttribute.Qualifier pattern = qualifiers().get(i); - CelAttribute.Qualifier qualifier = attribute.qualifiers().get(i); - if (pattern.kind() != CelAttribute.Qualifier.Kind.WILD_CARD && !pattern.equals(qualifier)) { + if (!matches(qualifiers().get(i), attribute.qualifiers().get(i))) { return false; } } @@ -154,9 +153,7 @@ public boolean isPartialMatch(CelAttribute attribute) { } int qualifierCount = min(qualifiers().size(), attribute.qualifiers().size()); for (int i = 0; i < qualifierCount; i++) { - CelAttribute.Qualifier pattern = qualifiers().get(i); - CelAttribute.Qualifier qualifier = attribute.qualifiers().get(i); - if (pattern.kind() != CelAttribute.Qualifier.Kind.WILD_CARD && !pattern.equals(qualifier)) { + if (!matches(qualifiers().get(i), attribute.qualifiers().get(i))) { return false; } } @@ -165,6 +162,24 @@ public boolean isPartialMatch(CelAttribute attribute) { return true; } + // Numeric qualifiers match by value across int and uint, as map keys and list indices do. + private static boolean matches(CelAttribute.Qualifier pattern, CelAttribute.Qualifier qualifier) { + if (pattern.kind() == CelAttribute.Qualifier.Kind.WILD_CARD || pattern.equals(qualifier)) { + return true; + } + if (pattern.kind() == CelAttribute.Qualifier.Kind.AS_INT + && qualifier.kind() == CelAttribute.Qualifier.Kind.AS_UINT) { + return intEqualsUint(pattern.asInt(), qualifier.asUint()); + } + return pattern.kind() == CelAttribute.Qualifier.Kind.AS_UINT + && qualifier.kind() == CelAttribute.Qualifier.Kind.AS_INT + && intEqualsUint(qualifier.asInt(), pattern.asUint()); + } + + private static boolean intEqualsUint(long intValue, UnsignedLong uintValue) { + return intValue >= 0 && UnsignedLong.valueOf(intValue).equals(uintValue); + } + /** * Return an appropriate attribute for a pattern match. * diff --git a/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel index c25bd9ca0..5abc2ee25 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel @@ -320,6 +320,9 @@ java_library( name = "eval_index", srcs = ["EvalIndex.java"], deps = [ + ":attribute", + ":eval_attribute", + ":eval_const", ":eval_helpers", ":planned_interpretable", "//common/ast", @@ -329,6 +332,7 @@ java_library( "//runtime:interpretable", "//runtime:resolved_overload", "@maven//:com_google_errorprone_error_prone_annotations", + "@maven//:com_google_guava_guava", ], ) @@ -985,6 +989,9 @@ cel_android_library( name = "eval_index_android", srcs = ["EvalIndex.java"], deps = [ + ":attribute_android", + ":eval_attribute_android", + ":eval_const_android", ":eval_helpers_android", ":planned_interpretable_android", "//common/ast:ast_android", @@ -994,6 +1001,7 @@ cel_android_library( "//runtime:interpretable_android", "//runtime:resolved_overload_android", "@maven//:com_google_errorprone_error_prone_annotations", + "@maven_android//:com_google_guava_guava", ], ) diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java index 012432f45..bca40c235 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java @@ -59,16 +59,8 @@ static Object evalStrictly( Object val; try { val = interpretable.eval(resolver, frame); - } catch (LocalizedEvaluationException e) { - // Already localized - propagate as-is to preserve inner expression ID - throw e; - } catch (CelRuntimeException e) { - // Wrap with current interpretable's location - throw new LocalizedEvaluationException(e, interpretable.expr().id()); } catch (Exception e) { - // Wrap generic exceptions with location - throw new LocalizedEvaluationException( - e, CelErrorCode.INTERNAL_ERROR, interpretable.expr().id()); + throw localize(e, interpretable.expr().id()); } if (val instanceof ErrorValue) { throw ((ErrorValue) val).value(); @@ -76,6 +68,17 @@ static Object evalStrictly( return val; } + /** Localizes {@code e} to {@code exprId} unless an inner expression already did. */ + static LocalizedEvaluationException localize(Exception e, long exprId) { + if (e instanceof LocalizedEvaluationException) { + return (LocalizedEvaluationException) e; + } + if (e instanceof CelRuntimeException) { + return new LocalizedEvaluationException((CelRuntimeException) e, exprId); + } + return new LocalizedEvaluationException(e, CelErrorCode.INTERNAL_ERROR, exprId); + } + static Object dispatch( String functionName, CelResolvedOverload overload, diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalIndex.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalIndex.java index d7fec0088..d012cea03 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalIndex.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalIndex.java @@ -17,6 +17,7 @@ import static dev.cel.runtime.planner.EvalHelpers.evalNonstrictly; import static dev.cel.runtime.planner.EvalHelpers.evalStrictly; +import com.google.common.primitives.UnsignedLong; import com.google.errorprone.annotations.Immutable; import dev.cel.common.ast.CelExpr; import dev.cel.common.values.CelValueConverter; @@ -53,16 +54,79 @@ Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) throws CelEva functionName, resolvedOverload, celValueConverter, targetVal, indexVal); } - static EvalIndex create( + /** Plans {@code target[index]}. */ + static PlannedInterpretable create( CelExpr expr, String functionName, CelResolvedOverload resolvedOverload, PlannedInterpretable target, PlannedInterpretable index, CelValueConverter celValueConverter) { + // Qualifies the attribute so unknown patterns match through constant indices, as in CEL-C++. + if (target instanceof EvalAttribute + && index instanceof EvalConstant + && resolvedOverload.isStrict()) { + Object constant = ((EvalConstant) index).constantValue(); + // Kinds representable as a CelAttribute.Qualifier. + if (constant instanceof Long + || constant instanceof UnsignedLong + || constant instanceof String + || constant instanceof Boolean) { + return ((EvalAttribute) target) + .addQualifier( + expr, + new ConstantIndexQualifier( + expr.id(), resolvedOverload, constant, celValueConverter)); + } + } return new EvalIndex(expr, functionName, resolvedOverload, target, index, celValueConverter); } + /** Indexes an attribute's value by a constant through the {@code _[_]} overload. */ + @Immutable + private static final class ConstantIndexQualifier implements Qualifier { + private final long exprId; + private final CelResolvedOverload resolvedOverload; + + @SuppressWarnings("Immutable") // Immutable CEL constant. + private final Object index; + + private final CelValueConverter celValueConverter; + + @Override + public Object value() { + return index; + } + + @Override + public Object qualify(Object operand) { + // Mirrors EvalIndex: the overload receives the same adapted target, and failures are + // localized to the index expression. + try { + return celValueConverter.toTraversalTarget( + EvalHelpers.dispatch( + resolvedOverload.getFunctionName(), + resolvedOverload, + celValueConverter, + EvalHelpers.convertAndAdaptResult(celValueConverter, operand), + index)); + } catch (Exception e) { + throw EvalHelpers.localize(e, exprId); + } + } + + private ConstantIndexQualifier( + long exprId, + CelResolvedOverload resolvedOverload, + Object index, + CelValueConverter celValueConverter) { + this.exprId = exprId; + this.resolvedOverload = resolvedOverload; + this.index = index; + this.celValueConverter = celValueConverter; + } + } + private EvalIndex( CelExpr expr, String functionName, diff --git a/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java b/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java index d83e74f00..16e036559 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java @@ -114,9 +114,9 @@ public Object resolve(long exprId, GlobalResolver ctx, ExecutionFrame frame) { } // The name itself could be a fully qualified reference to an enum value - // (e.g: my.enum_type.BAR) + // (e.g: my.enum_type.BAR), which can't be qualified further. int lastDotIndex = name.lastIndexOf('.'); - if (lastDotIndex > 0) { + if (lastDotIndex > 0 && qualifiers.isEmpty()) { String enumTypeName = name.substring(0, lastDotIndex); String enumValueQualifier = name.substring(lastDotIndex + 1); diff --git a/runtime/src/test/java/dev/cel/runtime/CelAttributePatternTest.java b/runtime/src/test/java/dev/cel/runtime/CelAttributePatternTest.java index de265cc77..bb7eab9a1 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelAttributePatternTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelAttributePatternTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; +import com.google.common.primitives.UnsignedLong; import dev.cel.runtime.CelAttribute.Qualifier; import org.junit.Test; import org.junit.runner.RunWith; @@ -244,6 +245,36 @@ public void isPartialMatch_doesNotMatch() { .isFalse(); } + @Test + public void isMatch_intAndUintQualifiersWithSameValue_match() { + CelAttributePattern intPattern = + CelAttributePattern.create("identifier").qualify(Qualifier.ofInt(1)); + CelAttributePattern uintPattern = + CelAttributePattern.create("identifier").qualify(Qualifier.ofUint(1)); + CelAttribute intAttribute = CelAttribute.create("identifier").qualify(Qualifier.ofInt(1)); + CelAttribute uintAttribute = CelAttribute.create("identifier").qualify(Qualifier.ofUint(1)); + + assertThat(intPattern.isMatch(uintAttribute)).isTrue(); + assertThat(uintPattern.isMatch(intAttribute)).isTrue(); + assertThat(intPattern.isPartialMatch(uintAttribute)).isTrue(); + assertThat(uintPattern.isPartialMatch(intAttribute)).isTrue(); + } + + @Test + public void isMatch_negativeIntAndUintWithSameBits_doNotMatch() { + CelAttributePattern intPattern = + CelAttributePattern.create("identifier").qualify(Qualifier.ofInt(-1)); + CelAttributePattern uintPattern = + CelAttributePattern.create("identifier") + .qualify(Qualifier.ofUint(UnsignedLong.fromLongBits(-1))); + CelAttribute intAttribute = CelAttribute.create("identifier").qualify(Qualifier.ofInt(-1)); + CelAttribute uintAttribute = + CelAttribute.create("identifier").qualify(Qualifier.ofUint(UnsignedLong.fromLongBits(-1))); + + assertThat(intPattern.isMatch(uintAttribute)).isFalse(); + assertThat(uintPattern.isMatch(intAttribute)).isFalse(); + } + @Test public void simplify_simplifies() { assertThat( diff --git a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java index 4e006cdd4..dbe0a2f1d 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java @@ -47,6 +47,7 @@ import dev.cel.common.ast.CelExpr.CelComprehension; import dev.cel.common.exceptions.CelDivideByZeroException; import dev.cel.common.exceptions.CelInvalidArgumentException; +import dev.cel.common.exceptions.CelRuntimeException; import dev.cel.common.internal.CelDescriptorPool; import dev.cel.common.internal.DefaultDescriptorPool; import dev.cel.common.internal.DefaultMessageFactory; @@ -769,7 +770,7 @@ public void plan_call_index_withUnknownTarget() throws Exception { assertThat(result) .isEqualTo( CelUnknownSet.create( - ImmutableSet.of(CelAttribute.create("unk_list")), ImmutableSet.of(1L))); + ImmutableSet.of(CelAttribute.create("unk_list")), ImmutableSet.of(2L))); } @Test @@ -811,6 +812,114 @@ public void plan_call_index_withMultipleUnknowns_mergesUnknowns() throws Excepti ImmutableSet.of(1L, 3L))); } + @Test + public void plan_call_constantIndex_withPartialVarsOnOtherElement_returnsValue() + throws Exception { + CelAbstractSyntaxTree ast = compile("msg.repeated_nested_message[0].bb"); + Program program = PLANNER.plan(ast); + TestAllTypes msg = + TestAllTypes.newBuilder() + .addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(1)) + .addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(2)) + .build(); + + Object result = + program.eval( + PartialVars.of( + ImmutableMap.of("msg", msg), + CelAttributePattern.fromQualifiedIdentifier("msg.repeated_nested_message") + .qualify(CelAttribute.Qualifier.ofInt(1)))); + + assertThat(result).isEqualTo(1L); + } + + @Test + public void plan_call_constantIndex_withPartialVarsOnIndexedElement_returnsUnknown() + throws Exception { + CelAbstractSyntaxTree ast = compile("msg.repeated_nested_message[0].bb"); + Program program = PLANNER.plan(ast); + CelAttribute indexedElement = + CelAttribute.fromQualifiedIdentifier("msg.repeated_nested_message") + .qualify(CelAttribute.Qualifier.ofInt(0)); + + Object result = + program.eval( + PartialVars.of( + CelAttributePattern.fromQualifiedIdentifier("msg.repeated_nested_message") + .qualify(CelAttribute.Qualifier.ofInt(0)))); + + assertThat(result) + .isEqualTo(CelUnknownSet.create(ImmutableSet.of(indexedElement), ImmutableSet.of(5L))); + } + + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum ConstantIndexUnknownTestCase { + UINT_INDEX_MATCHES_INT_PATTERN( + "dyn_var[1u]", + CelAttribute.Qualifier.ofInt(1), + CelAttribute.create("dyn_var").qualify(CelAttribute.Qualifier.ofUint(1))), + INT_INDEX_MATCHES_UINT_PATTERN( + "dyn_var[1]", + CelAttribute.Qualifier.ofUint(1), + CelAttribute.create("dyn_var").qualify(CelAttribute.Qualifier.ofInt(1))), + BOOL_INDEX( + "dyn_var[true]", + CelAttribute.Qualifier.ofBool(true), + CelAttribute.create("dyn_var").qualify(CelAttribute.Qualifier.ofBool(true))), + // A double isn't an attribute qualifier, so the index isn't folded into the attribute. + DOUBLE_INDEX_STOPS_AT_INDEX( + "dyn_var[1.0]", CelAttribute.Qualifier.ofInt(1), CelAttribute.create("dyn_var")); + + private final String expression; + private final CelAttribute.Qualifier patternIndex; + private final CelAttribute expectedAttribute; + + ConstantIndexUnknownTestCase( + String expression, CelAttribute.Qualifier patternIndex, CelAttribute expectedAttribute) { + this.expression = expression; + this.patternIndex = patternIndex; + this.expectedAttribute = expectedAttribute; + } + } + + @Test + public void plan_call_constantIndex_withPartialVars_returnsUnknown( + @TestParameter ConstantIndexUnknownTestCase testCase) throws Exception { + CelAbstractSyntaxTree ast = compile(testCase.expression); + Program program = PLANNER.plan(ast); + + CelUnknownSet result = + (CelUnknownSet) + program.eval( + PartialVars.of( + CelAttributePattern.create("dyn_var").qualify(testCase.patternIndex))); + + assertThat(result.attributes()).containsExactly(testCase.expectedAttribute); + } + + @Test + public void plan_call_constantIndexOnEnumConstant_throws() throws Exception { + CelAbstractSyntaxTree ast = CEL_COMPILER.parse("GlobalEnum.GAR[0]").getAst(); + Program program = PLANNER.plan(ast); + + assertThrows(CelEvaluationException.class, program::eval); + } + + @Test + public void plan_call_constantIndex_outOfBounds_localizesErrorToIndex() throws Exception { + CelAbstractSyntaxTree ast = compile("msg.repeated_nested_message[1].bb"); + Program program = PLANNER.plan(ast); + + CelEvaluationException e = + assertThrows( + CelEvaluationException.class, + () -> program.eval(ImmutableMap.of("msg", TestAllTypes.getDefaultInstance()))); + + assertThat(e) + .hasMessageThat() + .isEqualTo("evaluation error at :27: Index out of bounds: 1"); + } + @Test public void plan_call_noMatchingOverload_throws() throws Exception { CelAbstractSyntaxTree ast = compile("concat(b'abc', dyn_var)"); @@ -1286,10 +1395,10 @@ public void plan_partialEval_withWildcardQualification() throws Exception { .isEqualTo( CelUnknownSet.create( ImmutableSet.of( - CelAttribute.create("unk"), + CelAttribute.create("unk").qualify(CelAttribute.Qualifier.ofString("c")), CelAttribute.create("unk").qualify(CelAttribute.Qualifier.ofString("a")), CelAttribute.create("unk").qualify(CelAttribute.Qualifier.ofString("b"))), - ImmutableSet.of(2L, 5L, 7L))); + ImmutableSet.of(2L, 5L, 8L))); } @Test @@ -2377,7 +2486,8 @@ public void plan_comprehension_iterRangeIsMutableAccumulator_iteratesElements() } @Test - public void plan_nonStrictFunction_withErrorArg_adaptsToException() throws Exception { + public void plan_nonStrictFunction_withErrorArg_adaptsToExceptionWithErrorCode() + throws Exception { CelCompiler compiler = CelCompilerFactory.standardCelCompilerBuilder() .addFunctionDeclarations( @@ -2396,7 +2506,9 @@ public void plan_nonStrictFunction_withErrorArg_adaptsToException() throws Excep "is_error_int", ImmutableList.of(Long.class), /* isStrict= */ false, - args -> args[0] instanceof Exception); + args -> + args[0] instanceof CelRuntimeException + && ((CelRuntimeException) args[0]).getErrorCode() == CelErrorCode.DIVIDE_BY_ZERO); ProgramPlanner planner = ProgramPlanner.newPlanner( TYPE_PROVIDER, diff --git a/runtime/src/test/resources/planner_unknownFieldSelection.baseline b/runtime/src/test/resources/planner_unknownFieldSelection.baseline index 0cbc75299..51eb23b49 100644 --- a/runtime/src/test/resources/planner_unknownFieldSelection.baseline +++ b/runtime/src/test/resources/planner_unknownFieldSelection.baseline @@ -36,7 +36,7 @@ declare x { } =====> bindings: {unknown_attributes=[x]} -result: CelUnknownSet{attributes=[x], unknownExprIds=[2]} +result: CelUnknownSet{attributes=[x], unknownExprIds=[3]} Source: x.map_int32_int64[22] declare x { @@ -44,7 +44,7 @@ declare x { } =====> bindings: {unknown_attributes=[x.map_int32_int64]} -result: CelUnknownSet{attributes=[x.map_int32_int64], unknownExprIds=[2]} +result: CelUnknownSet{attributes=[x.map_int32_int64], unknownExprIds=[3]} Source: x.repeated_nested_message[1] declare x { @@ -52,7 +52,7 @@ declare x { } =====> bindings: {unknown_attributes=[x]} -result: CelUnknownSet{attributes=[x], unknownExprIds=[2]} +result: CelUnknownSet{attributes=[x], unknownExprIds=[3]} Source: x.repeated_nested_message[1] declare x { @@ -60,7 +60,7 @@ declare x { } =====> bindings: {unknown_attributes=[x.repeated_nested_message]} -result: CelUnknownSet{attributes=[x.repeated_nested_message], unknownExprIds=[2]} +result: CelUnknownSet{attributes=[x.repeated_nested_message], unknownExprIds=[3]} Source: x.single_nested_message.bb declare x {