Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions common/value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1506,11 +1506,17 @@ Value WrapFieldImpl(
ABSL_ATTRIBUTE_LIFETIME_BOUND,
google::protobuf::Arena* absl_nonnull arena ABSL_ATTRIBUTE_LIFETIME_BOUND) {
ABSL_DCHECK(field != nullptr);
ABSL_DCHECK_EQ(message->GetDescriptor(), field->containing_type());
ABSL_DCHECK(descriptor_pool != nullptr);
ABSL_DCHECK(message_factory != nullptr);
ABSL_DCHECK(!IsWellKnownMessageType(message->GetDescriptor()));

if (ABSL_PREDICT_FALSE(message->GetDescriptor() !=
field->containing_type())) {
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("message ", message->GetDescriptor()->full_name(),
" does not contain field ", field->full_name())));
}

const auto* reflection = message->GetReflection();
if (field->is_map()) {
if (reflection->FieldSize(*message, field) == 0) {
Expand Down Expand Up @@ -1643,14 +1649,20 @@ Value WrapRepeatedFieldImpl(
ABSL_ATTRIBUTE_LIFETIME_BOUND,
google::protobuf::Arena* absl_nonnull arena ABSL_ATTRIBUTE_LIFETIME_BOUND) {
ABSL_DCHECK(field != nullptr);
ABSL_DCHECK_EQ(field->containing_type(), message->GetDescriptor());
ABSL_DCHECK(!field->is_map() && field->is_repeated());
ABSL_DCHECK_GE(index, 0);
ABSL_DCHECK(message != nullptr);
ABSL_DCHECK(descriptor_pool != nullptr);
ABSL_DCHECK(message_factory != nullptr);
ABSL_DCHECK(arena != nullptr);

if (ABSL_PREDICT_FALSE(message->GetDescriptor() !=
field->containing_type())) {
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("message ", message->GetDescriptor()->full_name(),
" does not contain field ", field->full_name())));
}

const auto* reflection = message->GetReflection();
const int size = reflection->FieldSize(*message, field);
if (ABSL_PREDICT_FALSE(index < 0 || index >= size)) {
Expand Down Expand Up @@ -1761,15 +1773,20 @@ Value WrapMapFieldValueImpl(
ABSL_ATTRIBUTE_LIFETIME_BOUND,
google::protobuf::Arena* absl_nonnull arena ABSL_ATTRIBUTE_LIFETIME_BOUND) {
ABSL_DCHECK(field != nullptr);
ABSL_DCHECK_EQ(field->containing_type()->containing_type(),
message->GetDescriptor());
ABSL_DCHECK(!field->is_map() && !field->is_repeated());
ABSL_DCHECK_EQ(value.type(), field->cpp_type());
ABSL_DCHECK(message != nullptr);
ABSL_DCHECK(descriptor_pool != nullptr);
ABSL_DCHECK(message_factory != nullptr);
ABSL_DCHECK(arena != nullptr);

if (ABSL_PREDICT_FALSE(field->containing_type()->containing_type() !=
message->GetDescriptor())) {
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("message ", message->GetDescriptor()->full_name(),
" does not contain field ", field->full_name())));
}

switch (field->type()) {
case google::protobuf::FieldDescriptor::TYPE_DOUBLE:
return DoubleValue(value.GetDoubleValue());
Expand Down
3 changes: 3 additions & 0 deletions common/values/parsed_message_value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,9 @@ bool ParsedMessageValue::HasField(
const google::protobuf::FieldDescriptor* absl_nonnull field) const {
ABSL_DCHECK(field != nullptr);

if (ABSL_PREDICT_FALSE(value_->GetDescriptor() != field->containing_type())) {
return false;
}
const auto* reflection = GetReflection();
if (field->is_map() || field->is_repeated()) {
return reflection->FieldSize(*value_, field) > 0;
Expand Down
24 changes: 23 additions & 1 deletion common/values/parsed_message_value_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,18 @@
#include <utility>

#include "google/protobuf/struct.pb.h"
#include "absl/status/status.h"
#include "absl/status/status_matchers.h"
#include "absl/strings/cord.h"
#include "absl/strings/string_view.h"
#include "common/memory.h"
#include "common/type.h"
#include "common/value.h"
#include "common/value_kind.h"
#include "common/value_testing.h"
#include "internal/testing.h"
#include "runtime/runtime_options.h"
#include "cel/expr/conformance/proto3/test_all_types.pb.h"
#include "google/protobuf/descriptor.h"
#include "google/protobuf/io/zero_copy_stream_impl_lite.h"

namespace cel {
Expand Down Expand Up @@ -108,5 +110,25 @@ TEST_F(ParsedMessageValueTest, GetFieldByNumber) {
IsOkAndHolds(BoolValueIs(false)));
}

TEST_F(ParsedMessageValueTest, GetFieldMismatchedContainingType) {
ParsedMessageValue value = MakeParsedMessage<TestAllTypesProto3>();
const google::protobuf::FieldDescriptor* struct_field =
google::protobuf::Struct::descriptor()->field(0);
Value result;
ASSERT_THAT(
value.GetField(struct_field, ProtoWrapperTypeOptions::kUnsetNull,
descriptor_pool(), message_factory(), arena(), &result),
IsOk());
EXPECT_THAT(result, test::ErrorValueIs(absl_testing::StatusIs(
absl::StatusCode::kInvalidArgument)));
}

TEST_F(ParsedMessageValueTest, HasFieldMismatchedContainingType) {
ParsedMessageValue value = MakeParsedMessage<TestAllTypesProto3>();
const google::protobuf::FieldDescriptor* struct_field =
google::protobuf::Struct::descriptor()->field(0);
EXPECT_FALSE(value.HasField(struct_field));
}

} // namespace
} // namespace cel
7 changes: 7 additions & 0 deletions eval/eval/select_step.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <cstdint>
#include <memory>
#include <optional>
#include <string>
#include <utility>

Expand Down Expand Up @@ -567,6 +568,12 @@ absl::StatusOr<std::unique_ptr<ExpressionStep>> CreateTypedSelectStep(
const google::protobuf::FieldDescriptor* field_descriptor =
resolved_field.GetMessage().descriptor();

if (field_descriptor->containing_type() != descriptor) {
return CreateSelectStep(std::move(field), test_only, expr_id,
enable_wrapper_type_null_unboxing,
enable_optional_types);
}

if (test_only) {
return std::make_unique<ProtoHasStep>(
std::move(field), expr_id, enable_wrapper_type_null_unboxing,
Expand Down
Loading