From aa6875c1476067b6fb681d6dfaa4c11d8e3d1544 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Tue, 28 Nov 2023 12:04:52 -0800 Subject: [PATCH] Fix regex matches runtime error to throw INVALID_ARGUMENT as an error code PiperOrigin-RevId: 586062469 --- .../src/main/java/dev/cel/runtime/BUILD.bazel | 1 + .../dev/cel/runtime/StandardFunctions.java | 23 +++++++++++++++++-- .../resources/regexpMatches_error.baseline | 11 +++++++++ .../resources/regexpMatchingTests.baseline | 2 +- .../dev/cel/testing/BaseInterpreterTest.java | 9 ++++++++ .../validators/RegexLiteralValidatorTest.java | 12 ++++++---- 6 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 runtime/src/test/resources/regexpMatches_error.baseline diff --git a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel index 267c6b0d..d1b4da86 100644 --- a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel @@ -61,6 +61,7 @@ java_library( "@maven//:com_google_guava_guava", "@maven//:com_google_protobuf_protobuf_java", "@maven//:com_google_protobuf_protobuf_java_util", + "@maven//:com_google_re2j_re2j", "@maven//:org_jspecify_jspecify", ], ) diff --git a/runtime/src/main/java/dev/cel/runtime/StandardFunctions.java b/runtime/src/main/java/dev/cel/runtime/StandardFunctions.java index 3b859aa1..d7d00fda 100644 --- a/runtime/src/main/java/dev/cel/runtime/StandardFunctions.java +++ b/runtime/src/main/java/dev/cel/runtime/StandardFunctions.java @@ -24,6 +24,7 @@ import com.google.protobuf.Timestamp; import com.google.protobuf.util.Durations; import com.google.protobuf.util.Timestamps; +import com.google.re2j.PatternSyntaxException; import dev.cel.common.CelErrorCode; import dev.cel.common.CelOptions; import dev.cel.common.annotations.Internal; @@ -62,13 +63,31 @@ public static void add(Registrar registrar, DynamicProto dynamicProto, CelOption "matches", String.class, String.class, - (String string, String regexp) -> RuntimeHelpers.matches(string, regexp, celOptions)); + (String string, String regexp) -> { + try { + return RuntimeHelpers.matches(string, regexp, celOptions); + } catch (PatternSyntaxException e) { + throw new InterpreterException.Builder(e.getMessage()) + .setCause(e) + .setErrorCode(CelErrorCode.INVALID_ARGUMENT) + .build(); + } + }); // Duplicate receiver-style matches overload. registrar.add( "matches_string", String.class, String.class, - (String string, String regexp) -> RuntimeHelpers.matches(string, regexp, celOptions)); + (String string, String regexp) -> { + try { + return RuntimeHelpers.matches(string, regexp, celOptions); + } catch (PatternSyntaxException e) { + throw new InterpreterException.Builder(e.getMessage()) + .setCause(e) + .setErrorCode(CelErrorCode.INVALID_ARGUMENT) + .build(); + } + }); // In operator: b in a registrar.add( "in_list", diff --git a/runtime/src/test/resources/regexpMatches_error.baseline b/runtime/src/test/resources/regexpMatches_error.baseline new file mode 100644 index 00000000..a9d3174e --- /dev/null +++ b/runtime/src/test/resources/regexpMatches_error.baseline @@ -0,0 +1,11 @@ +Source: matches("alpha", "**") +=====> +bindings: {} +error: evaluation error: error parsing regexp: missing argument to repetition operator: `*` +error_code: INVALID_ARGUMENT + +Source: "alpha".matches("**") +=====> +bindings: {} +error: evaluation error: error parsing regexp: missing argument to repetition operator: `*` +error_code: INVALID_ARGUMENT diff --git a/runtime/src/test/resources/regexpMatchingTests.baseline b/runtime/src/test/resources/regexpMatchingTests.baseline index f30e5684..4700946e 100644 --- a/runtime/src/test/resources/regexpMatchingTests.baseline +++ b/runtime/src/test/resources/regexpMatchingTests.baseline @@ -316,4 +316,4 @@ declare s { } =====> bindings: {s=alpha, regexp=.*ha.$} -result: true \ No newline at end of file +result: true diff --git a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java index 4f7ba5b3..03fa65e2 100644 --- a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java +++ b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java @@ -1475,6 +1475,15 @@ public void regexpMatchingTests() throws Exception { runTest(Activation.copyOf(ImmutableMap.of("s", "alpha", "regexp", ".*ha.$"))); } + @Test + public void regexpMatches_error() throws Exception { + source = "matches(\"alpha\", \"**\")"; + runTest(Activation.EMPTY); + + source = "\"alpha\".matches(\"**\")"; + runTest(Activation.EMPTY); + } + @Test public void int64Conversions() throws Exception { source = "int('-1')"; // string converts to -1 diff --git a/validator/src/test/java/dev/cel/validator/validators/RegexLiteralValidatorTest.java b/validator/src/test/java/dev/cel/validator/validators/RegexLiteralValidatorTest.java index 2a79438a..f54e487d 100644 --- a/validator/src/test/java/dev/cel/validator/validators/RegexLiteralValidatorTest.java +++ b/validator/src/test/java/dev/cel/validator/validators/RegexLiteralValidatorTest.java @@ -96,7 +96,8 @@ public void regex_globalWithVariable_noOp() throws Exception { () -> CEL.createProgram(ast).eval(ImmutableMap.of("str_var", "**"))); assertThat(e) .hasMessageThat() - .contains("evaluation error: Function 'matches' failed with arg(s) 'test, **'"); + .contains( + "evaluation error: error parsing regexp: missing argument to repetition operator: `*`"); } @Test @@ -116,7 +117,8 @@ public void regex_receiverWithVariable_noOp() throws Exception { () -> CEL.createProgram(ast).eval(ImmutableMap.of("str_var", "**"))); assertThat(e) .hasMessageThat() - .contains("evaluation error: Function 'matches_string' failed with arg(s) 'test, **'"); + .contains( + "evaluation error: error parsing regexp: missing argument to repetition operator: `*`"); } @Test @@ -141,7 +143,8 @@ public void regex_globalWithFunction_noOp() throws Exception { // However, the same AST fails on evaluation when the function dispatch fails. assertThat(e) .hasMessageThat() - .contains("evaluation error: Function 'matches' failed with arg(s) 'test, **'"); + .contains( + "evaluation error: error parsing regexp: missing argument to repetition operator: `*`"); } @Test @@ -166,7 +169,8 @@ public void regex_receiverWithFunction_noOp() throws Exception { // However, the same AST fails on evaluation when the function dispatch fails. assertThat(e) .hasMessageThat() - .contains("evaluation error: Function 'matches_string' failed with arg(s) 'test, **'"); + .contains( + "evaluation error: error parsing regexp: missing argument to repetition operator: `*`"); } @Test