Add missing return statements after errors in C++ planner.

Fixes a couple of cases where malformed expressions might lead to index out of bounds errors.

PiperOrigin-RevId: 697727147

Author: jnthntatum

eval/compiler/flat_expr_builder.cc

@@ -933,6 +933,7 @@ class FlatExprVisitor : public cel::AstVisitor {
     if (expr->call_expr().args().size() != 3) {
       SetProgressStatusError(absl::InvalidArgumentError(
           "unexpected number of args for builtin ternary"));
+      return;
     }
 
     const cel::ast_internal::Expr* condition_expr =
@@ -981,6 +982,7 @@ class FlatExprVisitor : public cel::AstVisitor {
     if (expr->call_expr().args().size() != 2) {
       SetProgressStatusError(absl::InvalidArgumentError(
           "unexpected number of args for builtin boolean operator &&/||"));
+      return;
     }
     const cel::ast_internal::Expr* left_expr = &expr->call_expr().args()[0];
     const cel::ast_internal::Expr* right_expr = &expr->call_expr().args()[1];
@@ -1028,6 +1030,7 @@ class FlatExprVisitor : public cel::AstVisitor {
         expr->call_expr().args().size() != 1) {
       SetProgressStatusError(absl::InvalidArgumentError(
           "unexpected number of args for optional.or{Value}"));
+      return;
     }
     const cel::ast_internal::Expr* left_expr = &expr->call_expr().target();
     const cel::ast_internal::Expr* right_expr = &expr->call_expr().args()[0];
@@ -1188,6 +1191,7 @@ class FlatExprVisitor : public cel::AstVisitor {
         if (args.size() != 2) {
           SetProgressStatusError(absl::InvalidArgumentError(
               "unexpected number of args for builtin index operator"));
+          return;
         }
         SetRecursiveStep(CreateDirectContainerAccessStep(
                              std::move(args[0]), std::move(args[1]),
@@ -1966,6 +1970,7 @@ void TernaryCondVisitor::PostVisitArg(int arg_num,
     auto jump_after_first = CreateJumpStep({}, expr->id());
     if (!jump_after_first.ok()) {
       visitor_->SetProgressStatusError(jump_after_first.status());
+      return;
     }
 
     jump_after_first_ =

runtime/BUILD

@@ -262,6 +262,7 @@ cc_test(
         ":runtime_issue",
         ":runtime_options",
         ":standard_runtime_builder_factory",
+        "//base:builtins",
         "//common:memory",
         "//common:source",
         "//common:value",
@@ -277,6 +278,7 @@ cc_test(
         "@com_google_absl//absl/base:no_destructor",
         "@com_google_absl//absl/log:absl_check",
         "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:status_matchers",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings:string_view",
         "@com_google_cel_spec//proto/cel/expr:syntax_cc_proto",

runtime/standard_runtime_builder_factory_test.cc

@@ -24,8 +24,10 @@
 #include "absl/base/no_destructor.h"
 #include "absl/log/absl_check.h"
 #include "absl/status/status.h"
+#include "absl/status/status_matchers.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
+#include "base/builtins.h"
 #include "common/memory.h"
 #include "common/source.h"
 #include "common/value.h"
@@ -51,6 +53,7 @@
 namespace cel {
 namespace {
 
+using ::absl_testing::StatusIs;
 using ::cel::extensions::ProtobufRuntimeAdapter;
 using ::cel::extensions::ProtoMemoryManagerRef;
 using ::cel::test::BoolValueIs;
@@ -544,5 +547,116 @@ TEST(StandardRuntimeTest, RuntimeIssueSupport) {
   }
 }
 
+enum class EvalStrategy { kIterative, kRecursive };
+
+class StandardRuntimeEvalStrategyTest
+    : public ::testing::TestWithParam<EvalStrategy> {};
+
+// Check that calls to specialized builtins are validated.
+TEST_P(StandardRuntimeEvalStrategyTest, InvalidBuiltinBoolOp) {
+  EvalStrategy eval_strategy = GetParam();
+  RuntimeOptions options;
+  if (eval_strategy == EvalStrategy::kRecursive) {
+    options.max_recursion_depth = -1;
+  } else {
+    options.max_recursion_depth = 0;
+  }
+
+  google::protobuf::Arena arena;
+
+  ASSERT_OK_AND_ASSIGN(auto builder,
+                       CreateStandardRuntimeBuilder(
+                           google::protobuf::DescriptorPool::generated_pool(), options));
+
+  ASSERT_OK_AND_ASSIGN(auto runtime, std::move(builder).Build());
+
+  ParsedExpr expr;
+  expr.mutable_expr()->mutable_call_expr()->set_function(cel::builtin::kOr);
+  auto* arg = expr.mutable_expr()->mutable_call_expr()->add_args();
+  arg->mutable_const_expr()->set_bool_value(true);
+
+  EXPECT_THAT(ProtobufRuntimeAdapter::CreateProgram(*runtime, expr),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
+TEST_P(StandardRuntimeEvalStrategyTest, InvalidBuiltinTernaryOp) {
+  EvalStrategy eval_strategy = GetParam();
+  RuntimeOptions options;
+  if (eval_strategy == EvalStrategy::kRecursive) {
+    options.max_recursion_depth = -1;
+  } else {
+    options.max_recursion_depth = 0;
+  }
+
+  google::protobuf::Arena arena;
+
+  ASSERT_OK_AND_ASSIGN(auto builder,
+                       CreateStandardRuntimeBuilder(
+                           google::protobuf::DescriptorPool::generated_pool(), options));
+
+  ASSERT_OK_AND_ASSIGN(auto runtime, std::move(builder).Build());
+
+  ParsedExpr expr;
+  expr.mutable_expr()->mutable_call_expr()->set_function(
+      cel::builtin::kTernary);
+  expr.mutable_expr()
+      ->mutable_call_expr()
+      ->add_args()
+      ->mutable_const_expr()
+      ->set_bool_value(true);
+  expr.mutable_expr()
+      ->mutable_call_expr()
+      ->add_args()
+      ->mutable_const_expr()
+      ->set_bool_value(true);
+  expr.mutable_expr()
+      ->mutable_call_expr()
+      ->add_args()
+      ->mutable_const_expr()
+      ->set_bool_value(true);
+  expr.mutable_expr()
+      ->mutable_call_expr()
+      ->add_args()
+      ->mutable_const_expr()
+      ->set_bool_value(true);
+
+  EXPECT_THAT(ProtobufRuntimeAdapter::CreateProgram(*runtime, expr),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
+TEST_P(StandardRuntimeEvalStrategyTest, InvalidBuiltinIndex) {
+  EvalStrategy eval_strategy = GetParam();
+  RuntimeOptions options;
+  if (eval_strategy == EvalStrategy::kRecursive) {
+    options.max_recursion_depth = -1;
+  } else {
+    options.max_recursion_depth = 0;
+  }
+
+  google::protobuf::Arena arena;
+
+  ASSERT_OK_AND_ASSIGN(auto builder,
+                       CreateStandardRuntimeBuilder(
+                           google::protobuf::DescriptorPool::generated_pool(), options));
+
+  ASSERT_OK_AND_ASSIGN(auto runtime, std::move(builder).Build());
+
+  ParsedExpr expr;
+  expr.mutable_expr()->mutable_call_expr()->set_function(cel::builtin::kIndex);
+  auto* arg = expr.mutable_expr()->mutable_call_expr()->add_args();
+  arg->mutable_list_expr()
+      ->add_elements()
+      ->mutable_const_expr()
+      ->set_int64_value(1);
+
+  EXPECT_THAT(ProtobufRuntimeAdapter::CreateProgram(*runtime, expr),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
+INSTANTIATE_TEST_SUITE_P(StandardRuntimeEvalStrategyTest,
+                         StandardRuntimeEvalStrategyTest,
+                         testing::Values(EvalStrategy::kIterative,
+                                         EvalStrategy::kRecursive));
+
 }  // namespace
 }  // namespace cel