Skip to content

Commit e661775

Browse files
jnthntatumcopybara-github
authored andcommitted
Validate that select expression specifies an operand in FlatExprBuilder.
PiperOrigin-RevId: 742358622
1 parent 6fa6538 commit e661775

File tree

3 files changed

+25
-5
lines changed

3 files changed

+25
-5
lines changed

eval/compiler/flat_expr_builder.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,14 @@ class FlatExprVisitor : public cel::AstVisitor {
840840
}
841841
if (!ValidateOrError(
842842
!select_expr.field().empty(),
843-
"Invalid expression: select 'field' must not be empty")) {
843+
"invalid expression: select 'field' must not be empty")) {
844+
return;
845+
}
846+
if (!ValidateOrError(
847+
select_expr.has_operand() &&
848+
select_expr.operand().kind_case() !=
849+
cel::ExprKindCase::kUnspecifiedExpr,
850+
"invalid expression: select must specify an operand")) {
844851
return;
845852
}
846853

eval/compiler/flat_expr_builder_test.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,23 @@ TEST(FlatExprBuilderTest, SelectExprUnsetField) {
467467
HasSubstr("'field' must not be empty")));
468468
}
469469

470+
TEST(FlatExprBuilderTest, SelectExprUnsetOperand) {
471+
Expr expr;
472+
SourceInfo source_info;
473+
// An empty ident without the name set should error.
474+
google::protobuf::TextFormat::ParseFromString(R"(select_expr{
475+
field: 'field'
476+
operand { id: 1 }
477+
})",
478+
&expr);
479+
480+
CelExpressionBuilderFlatImpl builder(NewTestingRuntimeEnv());
481+
ASSERT_THAT(RegisterBuiltinFunctions(builder.GetRegistry()), IsOk());
482+
EXPECT_THAT(builder.CreateExpression(&expr, &source_info).status(),
483+
StatusIs(absl::StatusCode::kInvalidArgument,
484+
HasSubstr("must specify an operand")));
485+
}
486+
470487
TEST(FlatExprBuilderTest, ComprehensionExprUnsetAccuVar) {
471488
Expr expr;
472489
SourceInfo source_info;

eval/eval/BUILD

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,9 @@ cc_library(
203203
"//common:casting",
204204
"//common:expr",
205205
"//common:kind",
206-
"//common:native_type",
207206
"//common:value",
208207
"//common:value_kind",
209208
"//eval/internal:errors",
210-
"//internal:casts",
211209
"//internal:number",
212210
"//internal:status_macros",
213211
"//runtime/internal:errors",
@@ -312,11 +310,9 @@ cc_library(
312310
":evaluator_core",
313311
":expression_step_base",
314312
"//common:expr",
315-
"//common:native_type",
316313
"//common:value",
317314
"//common:value_kind",
318315
"//eval/internal:errors",
319-
"//internal:casts",
320316
"//internal:status_macros",
321317
"//runtime:runtime_options",
322318
"@com_google_absl//absl/base:nullability",

0 commit comments

Comments
 (0)