Skip to content

Commit aa6875c

Browse files
l46kokcopybara-github
authored andcommitted
Fix regex matches runtime error to throw INVALID_ARGUMENT as an error code
PiperOrigin-RevId: 586062469
1 parent 5776206 commit aa6875c

File tree

6 files changed

+51
-7
lines changed

6 files changed

+51
-7
lines changed

runtime/src/main/java/dev/cel/runtime/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ java_library(
6161
"@maven//:com_google_guava_guava",
6262
"@maven//:com_google_protobuf_protobuf_java",
6363
"@maven//:com_google_protobuf_protobuf_java_util",
64+
"@maven//:com_google_re2j_re2j",
6465
"@maven//:org_jspecify_jspecify",
6566
],
6667
)

runtime/src/main/java/dev/cel/runtime/StandardFunctions.java

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.google.protobuf.Timestamp;
2525
import com.google.protobuf.util.Durations;
2626
import com.google.protobuf.util.Timestamps;
27+
import com.google.re2j.PatternSyntaxException;
2728
import dev.cel.common.CelErrorCode;
2829
import dev.cel.common.CelOptions;
2930
import dev.cel.common.annotations.Internal;
@@ -62,13 +63,31 @@ public static void add(Registrar registrar, DynamicProto dynamicProto, CelOption
6263
"matches",
6364
String.class,
6465
String.class,
65-
(String string, String regexp) -> RuntimeHelpers.matches(string, regexp, celOptions));
66+
(String string, String regexp) -> {
67+
try {
68+
return RuntimeHelpers.matches(string, regexp, celOptions);
69+
} catch (PatternSyntaxException e) {
70+
throw new InterpreterException.Builder(e.getMessage())
71+
.setCause(e)
72+
.setErrorCode(CelErrorCode.INVALID_ARGUMENT)
73+
.build();
74+
}
75+
});
6676
// Duplicate receiver-style matches overload.
6777
registrar.add(
6878
"matches_string",
6979
String.class,
7080
String.class,
71-
(String string, String regexp) -> RuntimeHelpers.matches(string, regexp, celOptions));
81+
(String string, String regexp) -> {
82+
try {
83+
return RuntimeHelpers.matches(string, regexp, celOptions);
84+
} catch (PatternSyntaxException e) {
85+
throw new InterpreterException.Builder(e.getMessage())
86+
.setCause(e)
87+
.setErrorCode(CelErrorCode.INVALID_ARGUMENT)
88+
.build();
89+
}
90+
});
7291
// In operator: b in a
7392
registrar.add(
7493
"in_list",
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
Source: matches("alpha", "**")
2+
=====>
3+
bindings: {}
4+
error: evaluation error: error parsing regexp: missing argument to repetition operator: `*`
5+
error_code: INVALID_ARGUMENT
6+
7+
Source: "alpha".matches("**")
8+
=====>
9+
bindings: {}
10+
error: evaluation error: error parsing regexp: missing argument to repetition operator: `*`
11+
error_code: INVALID_ARGUMENT

runtime/src/test/resources/regexpMatchingTests.baseline

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,4 +316,4 @@ declare s {
316316
}
317317
=====>
318318
bindings: {s=alpha, regexp=.*ha.$}
319-
result: true
319+
result: true

testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,6 +1475,15 @@ public void regexpMatchingTests() throws Exception {
14751475
runTest(Activation.copyOf(ImmutableMap.of("s", "alpha", "regexp", ".*ha.$")));
14761476
}
14771477

1478+
@Test
1479+
public void regexpMatches_error() throws Exception {
1480+
source = "matches(\"alpha\", \"**\")";
1481+
runTest(Activation.EMPTY);
1482+
1483+
source = "\"alpha\".matches(\"**\")";
1484+
runTest(Activation.EMPTY);
1485+
}
1486+
14781487
@Test
14791488
public void int64Conversions() throws Exception {
14801489
source = "int('-1')"; // string converts to -1

validator/src/test/java/dev/cel/validator/validators/RegexLiteralValidatorTest.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ public void regex_globalWithVariable_noOp() throws Exception {
9696
() -> CEL.createProgram(ast).eval(ImmutableMap.of("str_var", "**")));
9797
assertThat(e)
9898
.hasMessageThat()
99-
.contains("evaluation error: Function 'matches' failed with arg(s) 'test, **'");
99+
.contains(
100+
"evaluation error: error parsing regexp: missing argument to repetition operator: `*`");
100101
}
101102

102103
@Test
@@ -116,7 +117,8 @@ public void regex_receiverWithVariable_noOp() throws Exception {
116117
() -> CEL.createProgram(ast).eval(ImmutableMap.of("str_var", "**")));
117118
assertThat(e)
118119
.hasMessageThat()
119-
.contains("evaluation error: Function 'matches_string' failed with arg(s) 'test, **'");
120+
.contains(
121+
"evaluation error: error parsing regexp: missing argument to repetition operator: `*`");
120122
}
121123

122124
@Test
@@ -141,7 +143,8 @@ public void regex_globalWithFunction_noOp() throws Exception {
141143
// However, the same AST fails on evaluation when the function dispatch fails.
142144
assertThat(e)
143145
.hasMessageThat()
144-
.contains("evaluation error: Function 'matches' failed with arg(s) 'test, **'");
146+
.contains(
147+
"evaluation error: error parsing regexp: missing argument to repetition operator: `*`");
145148
}
146149

147150
@Test
@@ -166,7 +169,8 @@ public void regex_receiverWithFunction_noOp() throws Exception {
166169
// However, the same AST fails on evaluation when the function dispatch fails.
167170
assertThat(e)
168171
.hasMessageThat()
169-
.contains("evaluation error: Function 'matches_string' failed with arg(s) 'test, **'");
172+
.contains(
173+
"evaluation error: error parsing regexp: missing argument to repetition operator: `*`");
170174
}
171175

172176
@Test

0 commit comments

Comments
 (0)