From 42190e1f9095c81c0073f503e60af1f9d0a670ee Mon Sep 17 00:00:00 2001 From: Muhammad Askri Date: Wed, 29 Jul 2026 17:23:35 -0700 Subject: [PATCH] Updated MessageToJson conversion for FieldMask in CEL C++ PiperOrigin-RevId: 956173251 --- common/BUILD | 1 + common/values/parsed_message_value.cc | 19 +-- common/values/parsed_message_value_test.cc | 14 ++- conformance/BUILD | 5 - eval/public/structs/BUILD | 7 +- eval/public/structs/cel_proto_wrap_util.cc | 72 +++++++++++- .../structs/cel_proto_wrap_util_test.cc | 108 +++++++++++++++++- eval/public/structs/cel_proto_wrapper_test.cc | 51 ++++++++- policy/BUILD | 1 + policy/yaml_policy_parser.cc | 77 ++++++++++++- policy/yaml_policy_parser_test.cc | 11 ++ 11 files changed, 328 insertions(+), 38 deletions(-) diff --git a/common/BUILD b/common/BUILD index a1d4ac3bd..401a9a8cb 100644 --- a/common/BUILD +++ b/common/BUILD @@ -886,6 +886,7 @@ cc_test( "@com_google_absl//absl/time", "@com_google_absl//absl/types:optional", "@com_google_cel_spec//proto/cel/expr/conformance/proto3:test_all_types_cc_proto", + "@com_google_protobuf//:field_mask_cc_proto", "@com_google_protobuf//:protobuf", "@com_google_protobuf//:struct_cc_proto", "@com_google_protobuf//:type_cc_proto", diff --git a/common/values/parsed_message_value.cc b/common/values/parsed_message_value.cc index 8a2b8030d..a2d02dfd0 100644 --- a/common/values/parsed_message_value.cc +++ b/common/values/parsed_message_value.cc @@ -34,12 +34,11 @@ #include "base/attribute.h" #include "common/memory.h" #include "common/value.h" +#include "common/values/values.h" #include "extensions/protobuf/internal/qualify.h" -#include "internal/empty_descriptors.h" #include "internal/json.h" #include "internal/message_equality.h" #include "internal/status_macros.h" -#include "internal/well_known_types.h" #include "runtime/runtime_options.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" @@ -51,8 +50,6 @@ namespace cel { namespace { -using ::cel::well_known_types::ValueReflection; - template std::enable_if_t, const google::protobuf::Message* absl_nonnull> @@ -60,14 +57,6 @@ EmptyParsedMessageValue() { return &T::default_instance(); } -template -std::enable_if_t< - std::conjunction_v, - std::negation>>, - const google::protobuf::Message* absl_nonnull> -EmptyParsedMessageValue() { - return internal::GetEmptyDefaultInstance(); -} } // namespace @@ -114,12 +103,8 @@ absl::Status ParsedMessageValue::ConvertToJson( ABSL_DCHECK_EQ(json->GetDescriptor()->well_known_type(), google::protobuf::Descriptor::WELLKNOWNTYPE_VALUE); - ValueReflection value_reflection; - CEL_RETURN_IF_ERROR(value_reflection.Initialize(json->GetDescriptor())); - google::protobuf::Message* json_object = value_reflection.MutableStructValue(json); - return internal::MessageToJson(*value_, descriptor_pool, message_factory, - json_object); + json); } absl::Status ParsedMessageValue::ConvertToJsonObject( diff --git a/common/values/parsed_message_value_test.cc b/common/values/parsed_message_value_test.cc index 7a84f82ba..bf679c004 100644 --- a/common/values/parsed_message_value_test.cc +++ b/common/values/parsed_message_value_test.cc @@ -14,9 +14,9 @@ #include +#include "google/protobuf/field_mask.pb.h" #include "google/protobuf/struct.pb.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" @@ -74,6 +74,18 @@ TEST_F(ParsedMessageValueTest, SerializeTo) { EXPECT_THAT(std::move(output).Consume(), IsEmpty()); } +TEST_F(ParsedMessageValueTest, ConvertToJson_FieldMask) { + auto value = + MakeParsedMessage(R"pb(paths: "foo.bar" + paths: "baz")pb"); + auto json = DynamicParseTextProto(R"pb()pb"); + EXPECT_THAT(value.ConvertToJson(descriptor_pool(), message_factory(), + cel::to_address(json)), + IsOk()); + EXPECT_THAT(*json, EqualsTextProto( + R"pb(string_value: "foo.bar,baz")pb")); +} + TEST_F(ParsedMessageValueTest, ConvertToJson) { MessageValue value = MakeParsedMessage(); auto json = DynamicParseTextProto(R"pb()pb"); diff --git a/conformance/BUILD b/conformance/BUILD index 6f11f345b..a38e59c15 100644 --- a/conformance/BUILD +++ b/conformance/BUILD @@ -173,14 +173,9 @@ _TESTS_TO_SKIP = [ "fields/qualified_identifier_resolution/qualified_ident,map_field_select,ident_with_longest_prefix_check,qualified_identifier_resolution_unchecked", "namespace/qualified/self_eval_qualified_lookup", "namespace/namespace/self_eval_container_lookup,self_eval_container_lookup_unchecked", - # TODO(issues/117): Integer overflow on enum assignments should error. - "enums/legacy_proto2/select_big,select_neg", # Skip until fixed. - "wrappers/field_mask/to_json", - "wrappers/empty/to_json", "fields/qualified_identifier_resolution/map_value_repeat_key_heterogeneous", - "parse/receiver_function_names", # Future features for CEL 1.0 # TODO(issues/119): Strong typing support for enums, specified but not implemented. diff --git a/eval/public/structs/BUILD b/eval/public/structs/BUILD index 4e4d5481c..8ac0f5059 100644 --- a/eval/public/structs/BUILD +++ b/eval/public/structs/BUILD @@ -96,24 +96,22 @@ cc_test( ], deps = [ ":cel_proto_wrap_util", - ":protobuf_value_factory", ":trivial_legacy_type_info", "//eval/public:cel_value", - "//eval/public:message_wrapper", "//eval/public/containers:container_backed_list_impl", "//eval/public/containers:container_backed_map_impl", "//eval/testutil:test_message_cc_proto", "//internal:proto_time_encoding", - "//internal:status_macros", "//internal:testing", "//testutil:util", - "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/time", + "@com_google_absl//absl/types:span", "@com_google_protobuf//:any_cc_proto", "@com_google_protobuf//:duration_cc_proto", "@com_google_protobuf//:empty_cc_proto", + "@com_google_protobuf//:field_mask_cc_proto", "@com_google_protobuf//:protobuf", "@com_google_protobuf//:struct_cc_proto", "@com_google_protobuf//:wrappers_cc_proto", @@ -218,6 +216,7 @@ cc_test( "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/time", + "@com_google_absl//absl/types:span", "@com_google_protobuf//:any_cc_proto", "@com_google_protobuf//:duration_cc_proto", "@com_google_protobuf//:empty_cc_proto", diff --git a/eval/public/structs/cel_proto_wrap_util.cc b/eval/public/structs/cel_proto_wrap_util.cc index 7bfe81fe6..c95d5c036 100644 --- a/eval/public/structs/cel_proto_wrap_util.cc +++ b/eval/public/structs/cel_proto_wrap_util.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -50,6 +51,7 @@ #include "internal/well_known_types.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" +#include "google/protobuf/json/json.h" #include "google/protobuf/message.h" #include "google/protobuf/message_lite.h" @@ -79,7 +81,8 @@ using google::protobuf::Descriptor; using google::protobuf::DescriptorPool; using google::protobuf::Message; using google::protobuf::MessageFactory; - +using google::protobuf::json::MessageToJsonString; +using google::protobuf::json::PrintOptions; // kMaxIntJSON is defined as the Number.MAX_SAFE_INTEGER value per EcmaScript 6. constexpr int64_t kMaxIntJSON = (1ll << 53) - 1; @@ -1079,6 +1082,38 @@ google::protobuf::Message* ValueFromValue(google::protobuf::Message* message, co return message; } } break; + case CelValue::Type::kMessage: { + const google::protobuf::Message* message_ptr = value.MessageOrDie(); + if (message_ptr->GetDescriptor()->full_name() == + "google.protobuf.Empty") { + reflection.MutableStructValue(message); + return message; + } + if (message_ptr->GetDescriptor()->full_name() == + "google.protobuf.FieldMask") { + PrintOptions json_options; + std::string json_str; + auto status = + MessageToJsonString(*message_ptr, &json_str, json_options); + if (!status.ok()) { + return nullptr; + } + // TODO(b/540507668): Refactor to pipe descriptor_pool through + // ValueFromValue to use internal::MessageToJson. + // If JSON marshalling is correct, we know we'll always get a plain + // JSON string value and it shouldn't contain any escapes that we need + // to interpret. + if (json_str.size() >= 2 && json_str.front() == '"' && + json_str.back() == '"') { + reflection.SetStringValue(message, + json_str.substr(1, json_str.size() - 2)); + } else { + reflection.SetStringValue(message, json_str); + } + return message; + } + return nullptr; + } break; case CelValue::Type::kNullType: reflection.SetNullValue(message); return message; @@ -1229,6 +1264,37 @@ bool ValueFromValue(Value* json, const CelValue& value, google::protobuf::Arena* return ListFromValue(json->mutable_list_value(), value, arena); case CelValue::Type::kMap: return StructFromValue(json->mutable_struct_value(), value, arena); + case CelValue::Type::kMessage: { + const google::protobuf::Message* message_ptr = value.MessageOrDie(); + if (message_ptr->GetDescriptor()->full_name() == + "google.protobuf.Empty") { + json->mutable_struct_value(); + return true; + } + if (message_ptr->GetDescriptor()->full_name() == + "google.protobuf.FieldMask") { + PrintOptions json_options; + std::string json_str; + auto status = + MessageToJsonString(*message_ptr, &json_str, json_options); + if (!status.ok()) { + return false; + } + // TODO(b/540507668): Refactor to pipe descriptor_pool through + // ValueFromValue to use internal::MessageToJson. + // If JSON marshalling is correct, we know we'll always get a plain + // JSON string value and it shouldn't contain any escapes that we need + // to interpret. + if (json_str.size() >= 2 && json_str.front() == '"' && + json_str.back() == '"') { + json->set_string_value(json_str.substr(1, json_str.size() - 2)); + } else { + json->set_string_value(json_str); + } + return true; + } + return false; + } case CelValue::Type::kNullType: json->set_null_value(protobuf::NULL_VALUE); return true; @@ -1254,7 +1320,7 @@ google::protobuf::Message* AnyFromValue(const google::protobuf::Message* prototy case CelValue::Type::kBytes: { BytesValue v; type_name = v.GetTypeName(); - v.set_value(std::string(value.BytesOrDie().value())); + v.set_value(value.BytesOrDie().value()); payload = v.SerializeAsCord(); } break; case CelValue::Type::kDouble: { @@ -1280,7 +1346,7 @@ google::protobuf::Message* AnyFromValue(const google::protobuf::Message* prototy case CelValue::Type::kString: { StringValue v; type_name = v.GetTypeName(); - v.set_value(std::string(value.StringOrDie().value())); + v.set_value(value.StringOrDie().value()); payload = v.SerializeAsCord(); } break; case CelValue::Type::kTimestamp: { diff --git a/eval/public/structs/cel_proto_wrap_util_test.cc b/eval/public/structs/cel_proto_wrap_util_test.cc index 59597fe8f..b0aadc2ff 100644 --- a/eval/public/structs/cel_proto_wrap_util_test.cc +++ b/eval/public/structs/cel_proto_wrap_util_test.cc @@ -15,6 +15,7 @@ #include "eval/public/structs/cel_proto_wrap_util.h" #include +#include #include #include #include @@ -24,23 +25,22 @@ #include "google/protobuf/any.pb.h" #include "google/protobuf/duration.pb.h" #include "google/protobuf/empty.pb.h" +#include "google/protobuf/field_mask.pb.h" #include "google/protobuf/struct.pb.h" #include "google/protobuf/wrappers.pb.h" -#include "absl/base/no_destructor.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/time/time.h" +#include "absl/types/span.h" #include "eval/public/cel_value.h" #include "eval/public/containers/container_backed_list_impl.h" #include "eval/public/containers/container_backed_map_impl.h" -#include "eval/public/message_wrapper.h" -#include "eval/public/structs/protobuf_value_factory.h" #include "eval/public/structs/trivial_legacy_type_info.h" #include "eval/testutil/test_message.pb.h" #include "internal/proto_time_encoding.h" -#include "internal/status_macros.h" #include "internal/testing.h" #include "testutil/util.h" +#include "google/protobuf/arena.h" #include "google/protobuf/dynamic_message.h" #include "google/protobuf/message.h" @@ -436,6 +436,106 @@ TEST_F(CelProtoWrapperTest, UnwrapInvalidAny) { UnwrapMessageToValue(&any, &ProtobufValueFactoryImpl, arena()).IsError()); } +TEST_F(CelProtoWrapperTest, WrapFieldMaskToValue) { + google::protobuf::FieldMask field_mask; + field_mask.add_paths("foo.bar"); + field_mask.add_paths("baz"); + CelValue value = ProtobufValueFactoryImpl(&field_mask); + + Value expected_message; + expected_message.set_string_value("foo.bar,baz"); + + ExpectWrappedMessage(value, expected_message); +} + +TEST_F(CelProtoWrapperTest, WrapMapWithFieldMaskToAny) { + const std::string kField = "field_mask"; + google::protobuf::FieldMask field_mask; + field_mask.add_paths("foo.bar"); + field_mask.add_paths("baz"); + CelValue value = ProtobufValueFactoryImpl(&field_mask); + + std::vector> args = { + {CelValue::CreateString(CelValue::StringHolder(&kField)), value}}; + auto cel_map = + CreateContainerBackedMap( + absl::Span>(args.data(), args.size())) + .value(); + auto cel_value = CelValue::CreateMap(cel_map.get()); + + Struct expected_struct; + (*expected_struct.mutable_fields())[kField].set_string_value("foo.bar,baz"); + Any expected_message; + expected_message.PackFrom(expected_struct); + + ExpectWrappedMessage(cel_value, expected_message); +} + +TEST_F(CelProtoWrapperTest, WrapListWithFieldMaskToAny) { + google::protobuf::FieldMask field_mask; + field_mask.add_paths("foo.bar"); + field_mask.add_paths("baz"); + CelValue value = ProtobufValueFactoryImpl(&field_mask); + + std::vector list_entries = {value}; + ContainerBackedListImpl cel_list(list_entries); + CelValue list_value = CelValue::CreateList(&cel_list); + + ListValue expected_list; + expected_list.add_values()->set_string_value("foo.bar,baz"); + Any expected_message; + expected_message.PackFrom(expected_list); + + ExpectWrappedMessage(list_value, expected_message); +} + +TEST_F(CelProtoWrapperTest, WrapEmptyToValue) { + google::protobuf::Empty empty; + CelValue value = ProtobufValueFactoryImpl(&empty); + + Value expected_message; + expected_message.mutable_struct_value(); + + ExpectWrappedMessage(value, expected_message); +} + +TEST_F(CelProtoWrapperTest, WrapMapWithEmptyToAny) { + const std::string kField = "empty"; + google::protobuf::Empty empty; + CelValue value = ProtobufValueFactoryImpl(&empty); + + std::vector> args = { + {CelValue::CreateString(CelValue::StringHolder(&kField)), value}}; + auto cel_map = + CreateContainerBackedMap( + absl::Span>(args.data(), args.size())) + .value(); + auto cel_value = CelValue::CreateMap(cel_map.get()); + + Struct expected_struct; + (*expected_struct.mutable_fields())[kField].mutable_struct_value(); + Any expected_message; + expected_message.PackFrom(expected_struct); + + ExpectWrappedMessage(cel_value, expected_message); +} + +TEST_F(CelProtoWrapperTest, WrapListWithEmptyToAny) { + google::protobuf::Empty empty; + CelValue value = ProtobufValueFactoryImpl(&empty); + + std::vector list_entries = {value}; + ContainerBackedListImpl cel_list(list_entries); + CelValue list_value = CelValue::CreateList(&cel_list); + + ListValue expected_list; + expected_list.add_values()->mutable_struct_value(); + Any expected_message; + expected_message.PackFrom(expected_list); + + ExpectWrappedMessage(list_value, expected_message); +} + // Test support of google.protobuf.Value wrappers in CelValue. TEST_F(CelProtoWrapperTest, UnwrapBoolWrapper) { bool value = true; diff --git a/eval/public/structs/cel_proto_wrapper_test.cc b/eval/public/structs/cel_proto_wrapper_test.cc index b9fcd6b51..2d658537b 100644 --- a/eval/public/structs/cel_proto_wrapper_test.cc +++ b/eval/public/structs/cel_proto_wrapper_test.cc @@ -1,6 +1,7 @@ #include "eval/public/structs/cel_proto_wrapper.h" #include +#include #include #include #include @@ -16,14 +17,15 @@ #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/time/time.h" +#include "absl/types/span.h" #include "eval/public/cel_value.h" #include "eval/public/containers/container_backed_list_impl.h" #include "eval/public/containers/container_backed_map_impl.h" #include "eval/testutil/test_message.pb.h" #include "internal/proto_time_encoding.h" -#include "internal/status_macros.h" #include "internal/testing.h" #include "testutil/util.h" +#include "google/protobuf/arena.h" #include "google/protobuf/dynamic_message.h" #include "google/protobuf/message.h" @@ -801,6 +803,53 @@ TEST_F(CelProtoWrapperTest, WrapStruct) { ExpectWrappedMessage(cel_value, any); } +TEST_F(CelProtoWrapperTest, WrapEmptyToValue) { + google::protobuf::Empty empty; + CelValue value = CelProtoWrapper::CreateMessage(&empty, arena()); + + Value expected_message; + expected_message.mutable_struct_value(); + + ExpectWrappedMessage(value, expected_message); +} + +TEST_F(CelProtoWrapperTest, WrapMapWithEmptyToAny) { + const std::string kField = "empty"; + google::protobuf::Empty empty; + CelValue value = CelProtoWrapper::CreateMessage(&empty, arena()); + + std::vector> args = { + {CelValue::CreateString(CelValue::StringHolder(&kField)), value}}; + auto cel_map = + CreateContainerBackedMap( + absl::Span>(args.data(), args.size())) + .value(); + auto cel_value = CelValue::CreateMap(cel_map.get()); + + Struct expected_struct; + (*expected_struct.mutable_fields())[kField].mutable_struct_value(); + Any expected_message; + expected_message.PackFrom(expected_struct); + + ExpectWrappedMessage(cel_value, expected_message); +} + +TEST_F(CelProtoWrapperTest, WrapListWithEmptyToAny) { + google::protobuf::Empty empty; + CelValue value = CelProtoWrapper::CreateMessage(&empty, arena()); + + std::vector list_entries = {value}; + ContainerBackedListImpl cel_list(list_entries); + CelValue list_value = CelValue::CreateList(&cel_list); + + ListValue expected_list; + expected_list.add_values()->mutable_struct_value(); + Any expected_message; + expected_message.PackFrom(expected_list); + + ExpectWrappedMessage(list_value, expected_message); +} + TEST_F(CelProtoWrapperTest, WrapFailureStructBadKeyType) { std::vector> args = { {CelValue::CreateInt64(1L), CelValue::CreateBool(true)}}; diff --git a/policy/BUILD b/policy/BUILD index cace76f5b..6a204d07c 100644 --- a/policy/BUILD +++ b/policy/BUILD @@ -84,6 +84,7 @@ cc_library( ":cel_policy_parser", "//common:source", "//internal:status_macros", + "//internal:utf8", "//policy/internal:yaml_string_element_scanner", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", diff --git a/policy/yaml_policy_parser.cc b/policy/yaml_policy_parser.cc index 4b4a37cb9..29070f8c7 100644 --- a/policy/yaml_policy_parser.cc +++ b/policy/yaml_policy_parser.cc @@ -14,10 +14,12 @@ #include "policy/yaml_policy_parser.h" +#include #include #include #include #include +#include #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -25,24 +27,88 @@ #include "absl/strings/string_view.h" #include "common/source.h" #include "internal/status_macros.h" +#include "internal/utf8.h" #include "policy/cel_policy.h" #include "policy/cel_policy_parse_context.h" #include "policy/cel_policy_parse_result.h" #include "policy/cel_policy_parser.h" #include "policy/internal/yaml_string_element_scanner.h" #include "yaml-cpp/exceptions.h" +#include "yaml-cpp/mark.h" #include "yaml-cpp/node/node.h" #include "yaml-cpp/node/parse.h" #include "yaml-cpp/null.h" #include "yaml-cpp/yaml.h" // IWYU pragma: keep namespace cel { +namespace { + +class ByteToCodepointMapper { + public: + explicit ByteToCodepointMapper(const Source* source) { + if (source == nullptr) return; + std::string text = source->content().ToString(); + mapping_.resize(text.size() + 1, 0); + size_t byte_offset = 0; + SourcePosition codepoint = 0; + absl::string_view view = text; + while (!view.empty()) { + auto [code_point, code_units] = cel::internal::Utf8Decode(view); + if (code_units == 0) break; + for (size_t i = 0; i < code_units; ++i) { + if (byte_offset + i < mapping_.size()) { + mapping_[byte_offset + i] = codepoint; + } + } + byte_offset += code_units; + view.remove_prefix(code_units); + codepoint++; + } + for (size_t i = byte_offset; i < mapping_.size(); ++i) { + mapping_[i] = codepoint; + } + } + + SourcePosition GetMarkCodepointPosition(const YAML::Mark& mark) const { + if (mark.is_null() || mark.pos < 0) return -1; + size_t byte_pos = static_cast(mark.pos); + if (mapping_.empty()) return static_cast(byte_pos); + if (byte_pos >= mapping_.size()) return mapping_.back(); + return mapping_[byte_pos]; + } + + private: + std::vector mapping_; +}; + +thread_local const ByteToCodepointMapper* current_mapper = nullptr; + +struct MapperScope { + explicit MapperScope(const ByteToCodepointMapper* mapper) + : prev(current_mapper) { + current_mapper = mapper; + } + ~MapperScope() { current_mapper = prev; } + const ByteToCodepointMapper* prev; +}; + +SourcePosition GetMarkCodepointPosition(const CelPolicyParseContext& ctx, + const YAML::Mark& mark) { + if (current_mapper != nullptr) { + return current_mapper->GetMarkCodepointPosition(mark); + } + if (mark.is_null() || mark.pos < 0) return -1; + return static_cast(mark.pos); +} + +} // namespace CelPolicyElementId YamlPolicyParser::CollectMetadata( CelPolicyParseContext& ctx, const YAML::Node& node) const { CelPolicyElementId element_id = ctx.next_element_id(); if (!node.Mark().is_null()) { - ctx.policy_source().NoteSourcePosition(element_id, node.Mark().pos); + ctx.policy_source().NoteSourcePosition( + element_id, GetMarkCodepointPosition(ctx, node.Mark())); } return element_id; } @@ -62,9 +128,10 @@ std::optional YamlPolicyParser::GetValueString( } if (!node.Mark().is_null() && ctx.policy_source().content() != nullptr) { + SourcePosition codepoint_pos = GetMarkCodepointPosition(ctx, node.Mark()); policy_internal::YamlStringElement element = policy_internal::ScanYamlStringElement( - ctx.policy_source().content()->content(), node.Mark().pos, + ctx.policy_source().content()->content(), codepoint_pos, node.as()); ctx.policy_source().NoteSourcePosition(id, element.starting_position); @@ -87,6 +154,9 @@ absl::Status YamlPolicyParser::ParsePolicy(CelPolicyParseContext& ctx) const { return absl::OkStatus(); } + ByteToCodepointMapper mapper(source); + MapperScope scope(&mapper); + ctx.policy().set_description(ValueString(-1, source->description())); std::string text = source->content().ToString(); YAML::Node node; @@ -94,7 +164,8 @@ absl::Status YamlPolicyParser::ParsePolicy(CelPolicyParseContext& ctx) const { node = YAML::Load(text); } catch (YAML::Exception& e) { if (!e.mark.is_null()) { - ctx.policy_source().NoteSourcePosition(0, e.mark.pos); + ctx.policy_source().NoteSourcePosition( + 0, GetMarkCodepointPosition(ctx, e.mark)); } ctx.ReportError(0, "Invalid CEL policy YAML syntax"); return absl::OkStatus(); diff --git a/policy/yaml_policy_parser_test.cc b/policy/yaml_policy_parser_test.cc index 4e7dfc49c..90c368546 100644 --- a/policy/yaml_policy_parser_test.cc +++ b/policy/yaml_policy_parser_test.cc @@ -198,6 +198,17 @@ std::vector GetParseTestCases() { " | - cel.expr.conformance\n" " | ....................^", }, + ParseTestCase{ + .yaml = R"yaml( + # Comment with multi-byte char: € + imports: + - name: + - cel.expr.conformance + )yaml", + .expected_error = "5:21: Import name is not a string\n" + " | - cel.expr.conformance\n" + " | ....................^", + }, ParseTestCase{ .yaml = R"yaml( rule: do something