Skip to content

Commit

Permalink
Fix regex matches runtime error to throw INVALID_ARGUMENT as an error…
Browse files Browse the repository at this point in the history
… code

PiperOrigin-RevId: 586062469
  • Loading branch information
l46kok authored and copybara-github committed Nov 28, 2023
1 parent 5776206 commit aa6875c
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 7 deletions.
1 change: 1 addition & 0 deletions runtime/src/main/java/dev/cel/runtime/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Expand Down
23 changes: 21 additions & 2 deletions runtime/src/main/java/dev/cel/runtime/StandardFunctions.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand Down
11 changes: 11 additions & 0 deletions runtime/src/test/resources/regexpMatches_error.baseline
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion runtime/src/test/resources/regexpMatchingTests.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -316,4 +316,4 @@ declare s {
}
=====>
bindings: {s=alpha, regexp=.*ha.$}
result: true
result: true
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit aa6875c

Please sign in to comment.