From c7a667cf873ef852f2416370b540ad06e3e69478 Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Fri, 9 Oct 2026 14:44:16 -0700 Subject: [PATCH] Precompile constant regex patterns in ProgramPlanner Fixes https://github.com/cel-expr/cel-java/issues/1038 Note: This feature is only available in the newer planner runtime. PiperOrigin-RevId: 996768728 --- .../src/main/java/dev/cel/runtime/BUILD.bazel | 2 + .../java/dev/cel/runtime/RuntimeHelpers.java | 20 ++- .../java/dev/cel/runtime/planner/BUILD.bazel | 12 ++ .../dev/cel/runtime/planner/EvalConstant.java | 4 + .../cel/runtime/planner/PlannerHelpers.java | 30 ++++ .../cel/runtime/planner/ProgramPlanner.java | 21 ++- .../java/dev/cel/runtime/standard/BUILD.bazel | 12 +- .../cel/runtime/standard/MatchesFunction.java | 87 ++++++---- .../runtime/planner/ProgramPlannerTest.java | 164 +++++++++++++++++- 9 files changed, 308 insertions(+), 44 deletions(-) diff --git a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel index e3329691e..eab664280 100644 --- a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel @@ -427,6 +427,7 @@ cel_android_library( "//common/annotations", "//common/exceptions:divide_by_zero", "//common/exceptions:index_out_of_bounds", + "//common/exceptions:invalid_argument", "//common/exceptions:numeric_overflow", "//common/internal:converter", "//common/values:values_android", @@ -451,6 +452,7 @@ java_library( "//common/annotations", "//common/exceptions:divide_by_zero", "//common/exceptions:index_out_of_bounds", + "//common/exceptions:invalid_argument", "//common/exceptions:numeric_overflow", "//common/internal:converter", "//common/values", diff --git a/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java b/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java index fb512541f..804c30b03 100644 --- a/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java +++ b/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java @@ -23,10 +23,12 @@ import com.google.protobuf.Duration; import com.google.protobuf.MessageLiteOrBuilder; import com.google.re2j.Pattern; +import com.google.re2j.PatternSyntaxException; import dev.cel.common.CelOptions; import dev.cel.common.annotations.Internal; import dev.cel.common.exceptions.CelDivideByZeroException; import dev.cel.common.exceptions.CelIndexOutOfBoundsException; +import dev.cel.common.exceptions.CelInvalidArgumentException; import dev.cel.common.exceptions.CelNumericOverflowException; import dev.cel.common.internal.Converter; import dev.cel.common.values.NullValue; @@ -76,16 +78,24 @@ public static java.time.Duration createJavaDurationFromString(String d) { } } - public static boolean matches(String string, String regexp, CelOptions celOptions) { - Pattern pattern = Pattern.compile(regexp); + public static Pattern compileRegexPattern(String regexp, CelOptions celOptions) { + Pattern pattern; + try { + pattern = Pattern.compile(regexp); + } catch (PatternSyntaxException e) { + throw new CelInvalidArgumentException(e); + } int maxProgramSize = celOptions.maxRegexProgramSize(); if (maxProgramSize >= 0 && pattern.programSize() > maxProgramSize) { - throw new IllegalArgumentException( + throw new CelInvalidArgumentException( String.format( "Regex pattern exceeds allowed program size. Allowed: %d, Provided: %d", maxProgramSize, pattern.programSize())); } + return pattern; + } + public static boolean matches(String string, Pattern pattern, CelOptions celOptions) { if (!celOptions.enableRegexPartialMatch()) { // Uses re2 for consistency across languages. return pattern.matcher(string).matches(); @@ -95,6 +105,10 @@ public static boolean matches(String string, String regexp, CelOptions celOption return pattern.matcher(string).find(); } + public static boolean matches(String string, String regexp, CelOptions celOptions) { + return matches(string, compileRegexPattern(regexp, celOptions), celOptions); + } + /** Concatenates two lists into a new list. */ public static List concat(List first, List second) { if (first instanceof ConcatenatedListView) { 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 32bc00b4a..74b17d43e 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel @@ -53,6 +53,7 @@ java_library( "//common/ast", "//common/ast:cel_block", "//common/exceptions:overload_not_found", + "//common/exceptions:runtime_exception", "//common/types", "//common/types:type_providers", "//common/values", @@ -470,8 +471,13 @@ java_library( deps = [ ":attribute", ":eval_attribute", + ":eval_const", + ":eval_unary", ":planned_interpretable", "//common/ast", + "//common/values", + "//runtime:resolved_overload", + "//runtime/standard:matches", ], ) @@ -718,6 +724,7 @@ cel_android_library( "//common/ast:ast_android", "//common/ast:cel_block_android", "//common/exceptions:overload_not_found", + "//common/exceptions:runtime_exception", "//common/types:type_providers_android", "//common/types:types_android", "//common/values:cel_value_provider_android", @@ -1137,8 +1144,13 @@ cel_android_library( deps = [ ":attribute_android", ":eval_attribute_android", + ":eval_const_android", + ":eval_unary_android", ":planned_interpretable_android", "//common/ast:ast_android", + "//common/values:values_android", + "//runtime:resolved_overload_android", + "//runtime/standard:matches_android", ], ) diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalConstant.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalConstant.java index 55554069b..2c4b0933b 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalConstant.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalConstant.java @@ -29,6 +29,10 @@ Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) { return constant; } + Object constantValue() { + return constant; + } + static EvalConstant create(CelExpr expr, Object value) { return new EvalConstant(expr, value); } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/PlannerHelpers.java b/runtime/src/main/java/dev/cel/runtime/planner/PlannerHelpers.java index f24590f50..d62bc0e09 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/PlannerHelpers.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/PlannerHelpers.java @@ -15,6 +15,11 @@ package dev.cel.runtime.planner; import dev.cel.common.ast.CelConstant; +import dev.cel.common.ast.CelExpr; +import dev.cel.common.values.CelValueConverter; +import dev.cel.runtime.CelResolvedOverload; +import dev.cel.runtime.standard.MatchesFunction; +import java.util.Optional; final class PlannerHelpers { @@ -47,5 +52,30 @@ static Object resolveConstant(CelConstant celConstant) { } } + static Optional maybePlanRegexMatches( + CelExpr expr, + CelResolvedOverload resolvedOverload, + PlannedInterpretable[] evaluatedArgs, + CelValueConverter celValueConverter) { + PlannedInterpretable regexArg = evaluatedArgs[1]; + if (!MatchesFunction.isMatchesOverload(resolvedOverload) + || !(regexArg instanceof EvalConstant)) { + return Optional.empty(); + } + Object rawConstant = ((EvalConstant) regexArg).constantValue(); + if (!(rawConstant instanceof String)) { + return Optional.empty(); + } + CelResolvedOverload unaryOverload = + MatchesFunction.newPrecompiledOverload(resolvedOverload, (String) rawConstant); + return Optional.of( + EvalUnary.create( + expr, + unaryOverload.getFunctionName(), + unaryOverload, + evaluatedArgs[0], + celValueConverter)); + } + private PlannerHelpers() {} } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java index 98f6e054c..c34b2924b 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java @@ -43,6 +43,7 @@ import dev.cel.common.ast.CelExpr.ExprKind.Kind; import dev.cel.common.ast.CelReference; import dev.cel.common.exceptions.CelOverloadNotFoundException; +import dev.cel.common.exceptions.CelRuntimeException; import dev.cel.common.types.CelKind; import dev.cel.common.types.CelType; import dev.cel.common.types.CelTypeProvider; @@ -107,10 +108,15 @@ public PlannedProgram plan(CelAbstractSyntaxTree ast) throws CelEvaluationExcept .map(celBlock -> planBlock(celBlock, ctx)) .orElseGet(() -> plan(ast.getExpr(), ctx)); } catch (RuntimeException e) { - throw CelEvaluationExceptionBuilder.newBuilder(e.getMessage()) - .setMetadata(errorMetadata, ast.getExpr().id()) - .setCause(e) - .build(); + // Preserve specific CelErrorCode (e.g., INVALID_ARGUMENT, OVERLOAD_NOT_FOUND) from + // plan-time validation failures. + CelEvaluationExceptionBuilder builder; + if (e instanceof CelRuntimeException) { + builder = CelEvaluationExceptionBuilder.newBuilder((CelRuntimeException) e); + } else { + builder = CelEvaluationExceptionBuilder.newBuilder(e.getMessage()).setCause(e); + } + throw builder.setMetadata(errorMetadata, ast.getExpr().id()).build(); } return PlannedProgram.create( @@ -355,6 +361,13 @@ private PlannedInterpretable planCall(CelExpr expr, PlannerContext ctx) { evaluatedArgs[1], celValueConverter); } + PlannedInterpretable regexMatches = + PlannerHelpers.maybePlanRegexMatches( + expr, resolvedOverload, evaluatedArgs, celValueConverter) + .orElse(null); + if (regexMatches != null) { + return regexMatches; + } return EvalBinary.create( expr, functionName, diff --git a/runtime/src/main/java/dev/cel/runtime/standard/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/standard/BUILD.bazel index 0a76b6135..3e6e5137a 100644 --- a/runtime/src/main/java/dev/cel/runtime/standard/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/standard/BUILD.bazel @@ -1045,12 +1045,16 @@ java_library( deps = [ ":standard_overload", "//common:options", - "//common/exceptions:invalid_argument", + "//common/annotations", "//runtime:function_binding", + "//runtime:function_overload", + "//runtime:resolved_overload", "//runtime:runtime_equality", "//runtime:runtime_helpers", "//runtime/standard:standard_function", + "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", + "@maven//:com_google_re2j_re2j", ], ) @@ -1063,10 +1067,14 @@ cel_android_library( ":standard_function_android", ":standard_overload_android", "//common:options", - "//common/exceptions:invalid_argument", + "//common/annotations", "//runtime:function_binding_android", + "//runtime:function_overload_android", + "//runtime:resolved_overload_android", "//runtime:runtime_equality_android", "//runtime:runtime_helpers_android", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven//:com_google_re2j_re2j", "@maven_android//:com_google_guava_guava", ], ) diff --git a/runtime/src/main/java/dev/cel/runtime/standard/MatchesFunction.java b/runtime/src/main/java/dev/cel/runtime/standard/MatchesFunction.java index a82a7d439..daa69f57d 100644 --- a/runtime/src/main/java/dev/cel/runtime/standard/MatchesFunction.java +++ b/runtime/src/main/java/dev/cel/runtime/standard/MatchesFunction.java @@ -14,16 +14,22 @@ package dev.cel.runtime.standard; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.errorprone.annotations.Immutable; +import com.google.re2j.Pattern; import dev.cel.common.CelOptions; -import dev.cel.common.exceptions.CelInvalidArgumentException; +import dev.cel.common.annotations.Internal; import dev.cel.runtime.CelFunctionBinding; +import dev.cel.runtime.CelFunctionOverload; +import dev.cel.runtime.CelResolvedOverload; import dev.cel.runtime.RuntimeEquality; import dev.cel.runtime.RuntimeHelpers; import java.util.Arrays; /** Standard function for {@code matches}. */ public final class MatchesFunction extends CelStandardFunction { + private static final String MATCHES_FUNCTION = "matches"; private static final MatchesFunction ALL_OVERLOADS = create(MatchesOverload.values()); public static MatchesFunction create() { @@ -38,51 +44,68 @@ public static MatchesFunction create(Iterable o return new MatchesFunction(ImmutableSet.copyOf(overloads)); } + @Internal + public static boolean isMatchesOverload(CelResolvedOverload resolvedOverload) { + return resolvedOverload.getDefinition() instanceof StandardMatchesOverload; + } + + @Internal + public static CelResolvedOverload newPrecompiledOverload( + CelResolvedOverload resolvedOverload, String regexPattern) { + CelOptions celOptions = ((StandardMatchesOverload) resolvedOverload.getDefinition()).celOptions; + Pattern compiledPattern = RuntimeHelpers.compileRegexPattern(regexPattern, celOptions); + String overloadId = resolvedOverload.getOverloadId(); + CelFunctionBinding binding = + CelFunctionBinding.from( + overloadId, + String.class, + target -> RuntimeHelpers.matches(target, compiledPattern, celOptions)); + return CelResolvedOverload.of( + MATCHES_FUNCTION, + overloadId, + binding.getDefinition(), + binding.isStrict(), + binding.getArgTypes()); + } + /** Overloads for the standard function. */ public enum MatchesOverload implements CelStandardOverload { - MATCHES( - (celOptions, runtimeEquality) -> - CelFunctionBinding.from( - "matches", - String.class, - String.class, - (String string, String regexp) -> { - try { - return RuntimeHelpers.matches(string, regexp, celOptions); - } catch (RuntimeException e) { - throw new CelInvalidArgumentException(e); - } - })), + MATCHES(MATCHES_FUNCTION), // Duplicate receiver-style matches overload. - MATCHES_STRING( - (celOptions, runtimeEquality) -> - CelFunctionBinding.from( - "matches_string", - String.class, - String.class, - (String string, String regexp) -> { - try { - return RuntimeHelpers.matches(string, regexp, celOptions); - } catch (RuntimeException e) { - throw new CelInvalidArgumentException(e); - } - })), + MATCHES_STRING("matches_string"), ; - private final CelStandardOverload standardOverload; + private final String overloadId; @Override public CelFunctionBinding newFunctionBinding( CelOptions celOptions, RuntimeEquality runtimeEquality) { - return standardOverload.newFunctionBinding(celOptions, runtimeEquality); + return CelFunctionBinding.from( + overloadId, + ImmutableList.of(String.class, String.class), + new StandardMatchesOverload(celOptions)); + } + + MatchesOverload(String overloadId) { + this.overloadId = overloadId; + } + } + + @Immutable + private static final class StandardMatchesOverload implements CelFunctionOverload { + private final CelOptions celOptions; + + @Override + public Object apply(Object[] args) { + return RuntimeHelpers.matches((String) args[0], (String) args[1], celOptions); } - MatchesOverload(CelStandardOverload standardOverload) { - this.standardOverload = standardOverload; + private StandardMatchesOverload(CelOptions celOptions) { + this.celOptions = celOptions; } } private MatchesFunction(ImmutableSet overloads) { - super("matches", overloads); + super(MATCHES_FUNCTION, overloads); } } 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 309dc9661..ad44a9f51 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java @@ -216,6 +216,10 @@ public final class ProgramPlannerTest { * driven by the top-level runtime APIs in the future */ private static DefaultDispatcher newDispatcher() { + return newDispatcher(CEL_OPTIONS); + } + + private static DefaultDispatcher newDispatcher(CelOptions celOptions) { DefaultDispatcher.Builder builder = DefaultDispatcher.newBuilder(); // Subsetted StdLib @@ -232,16 +236,17 @@ private static DefaultDispatcher newDispatcher() { StandardFunction.EQUALS, StandardFunction.NOT_STRICTLY_FALSE, StandardFunction.SIZE, + StandardFunction.MATCHES, StandardFunction.DYN) .build(); addBindingsToDispatcher( - builder, stdFunctions.newFunctionBindings(RUNTIME_EQUALITY, CEL_OPTIONS)); + builder, stdFunctions.newFunctionBindings(RUNTIME_EQUALITY, celOptions)); TypeFunction typeFunction = TypeFunction.create( DescriptorTypeResolver.create(TYPE_PROVIDER, CelValueConverter.getDefaultInstance())); addBindingsToDispatcher( - builder, typeFunction.newFunctionBindings(CEL_OPTIONS, RUNTIME_EQUALITY)); + builder, typeFunction.newFunctionBindings(celOptions, RUNTIME_EQUALITY)); addBindingsToDispatcher( builder, CelFunctionBinding.fromOverloads( @@ -2246,11 +2251,164 @@ public void plan_nonStrictFunction_withErrorArg_adaptsToException() throws Excep assertThat(result).isEqualTo(true); } + @Test + @TestParameters("{expression: \"matches('hubba', 'ubb')\", expectedResult: true}") + @TestParameters("{expression: \"'hubba'.matches('ubb')\", expectedResult: true}") + @TestParameters("{expression: \"'hubba'.matches('nomatch')\", expectedResult: false}") + @TestParameters("{expression: \"msg.single_string.matches('^f.*o$')\", expectedResult: true}") + @TestParameters("{expression: \"matches('foobar', msg.single_string)\", expectedResult: true}") + @TestParameters("{expression: \"matches('hubba', msg.single_string)\", expectedResult: false}") + public void plan_call_matches(String expression, boolean expectedResult) throws Exception { + CelAbstractSyntaxTree ast = compile(expression); + Program program = PLANNER.plan(ast); + + boolean result = + (boolean) + program.eval( + ImmutableMap.of("msg", TestAllTypes.newBuilder().setSingleString("foo").build())); + + assertThat(result).isEqualTo(expectedResult); + } + + @Test + @TestParameters("{expression: \"'hubba'.matches('ubb')\", expectedResult: false}") + @TestParameters("{expression: \"'hubba'.matches('hubba')\", expectedResult: true}") + public void plan_call_matches_partialMatchDisabled(String expression, boolean expectedResult) + throws Exception { + CelAbstractSyntaxTree ast = compile(expression); + ProgramPlanner planner = + newPlannerWithOptions(CelOptions.current().enableRegexPartialMatch(false).build()); + Program program = planner.plan(ast); + + boolean result = (boolean) program.eval(); + + assertThat(result).isEqualTo(expectedResult); + } + + @Test + public void plan_call_matches_nonStringTarget_throws() throws Exception { + CelAbstractSyntaxTree ast = compile("dyn_var.matches('a')"); + Program program = PLANNER.plan(ast); + String expectedOverloadId = isParseOnly ? "matches" : "matches_string"; + + CelEvaluationException e = + assertThrows( + CelEvaluationException.class, () -> program.eval(ImmutableMap.of("dyn_var", 123L))); + + assertThat(e) + .hasMessageThat() + .contains( + "No matching overload for function 'matches'. Overload candidates: " + + expectedOverloadId); + } + + @Test + public void plan_call_matches_nonStringConstantRegex_throws() throws Exception { + CelAbstractSyntaxTree ast = CEL_COMPILER.parse("'foo'.matches(123)").getAst(); + Program program = PLANNER.plan(ast); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, program::eval); + + assertThat(e).hasMessageThat().contains("No matching overload for function 'matches'"); + } + + @Test + public void plan_call_matches_withUnknownTarget_propagatesUnknown() throws Exception { + CelAbstractSyntaxTree ast = compile("msg.single_string.matches('a')"); + Program program = PLANNER.plan(ast); + + CelUnknownSet result = + (CelUnknownSet) program.eval(PartialVars.of(CelAttributePattern.create("msg"))); + + assertThat(result) + .isEqualTo( + CelUnknownSet.create(ImmutableSet.of(CelAttribute.create("msg")), ImmutableSet.of(2L))); + } + + @Test + public void plan_call_matches_customOverload( + @TestParameter({"matches('[', '[')", "'['.matches('[')"}) String expression) + throws Exception { + DefaultDispatcher.Builder dispatcherBuilder = DefaultDispatcher.newBuilder(); + addBindingsToDispatcher( + dispatcherBuilder, + CelFunctionBinding.fromOverloads( + "matches", + CelFunctionBinding.from("matches", String.class, String.class, String::equals), + CelFunctionBinding.from("matches_string", String.class, String.class, String::equals))); + ProgramPlanner planner = + ProgramPlanner.newPlanner( + TYPE_PROVIDER, + VALUE_PROVIDER, + dispatcherBuilder.build(), + CEL_VALUE_CONVERTER, + CEL_CONTAINER, + CEL_OPTIONS, + ImmutableSet.of(), + RUNTIME_EQUALITY, + CelAsyncEvaluationOptions.defaultOptions(), + /* asyncExecutor= */ null); + CelAbstractSyntaxTree ast = compile(expression); + Program program = planner.plan(ast); + + boolean result = (boolean) program.eval(); + + assertThat(result).isTrue(); + } + + @Test + @TestParameters("{expression: \"matches('alpha', '**')\", expectedLocation: ':7'}") + @TestParameters("{expression: \"'alpha'.matches('**')\", expectedLocation: ':15'}") + public void plan_call_matches_invalidConstantRegex_throws( + String expression, String expectedLocation) throws Exception { + CelAbstractSyntaxTree ast = compile(expression); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> PLANNER.plan(ast)); + + assertThat(e.getErrorCode()).isEqualTo(CelErrorCode.INVALID_ARGUMENT); + assertThat(e) + .hasMessageThat() + .contains( + "evaluation error at " + + expectedLocation + + ": error parsing regexp: missing argument to repetition operator: `*`"); + } + + @Test + public void plan_call_matches_regexProgramSizeWithinLimit() throws Exception { + CelAbstractSyntaxTree ast = compile("'foo'.matches('(a+b)')"); + ProgramPlanner planner = + newPlannerWithOptions(CelOptions.current().maxRegexProgramSize(7).build()); + Program program = planner.plan(ast); + + boolean result = (boolean) program.eval(); + + assertThat(result).isFalse(); + } + + @Test + public void plan_call_matches_regexProgramSizeExceedsLimit_throws( + @TestParameter({"0", "6"}) int maxProgramSize) throws Exception { + CelAbstractSyntaxTree ast = compile("'foo'.matches('(a+b)')"); + ProgramPlanner planner = + newPlannerWithOptions(CelOptions.current().maxRegexProgramSize(maxProgramSize).build()); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> planner.plan(ast)); + + assertThat(e.getErrorCode()).isEqualTo(CelErrorCode.INVALID_ARGUMENT); + assertThat(e) + .hasMessageThat() + .contains( + "evaluation error at :13: Regex pattern exceeds allowed program size. Allowed: " + + maxProgramSize + + ", Provided: 7"); + } + private static ProgramPlanner newPlannerWithOptions(CelOptions options) { return ProgramPlanner.newPlanner( TYPE_PROVIDER, VALUE_PROVIDER, - newDispatcher(), + newDispatcher(options), CEL_VALUE_CONVERTER, CEL_CONTAINER, options,