Skip to content

Commit 9602b67

Browse files
CEL Dev Teamcopybara-github
CEL Dev Team
authored and
copybara-github
committed
Propagate unsafe field access through ParsedMessageValue::Qualify.
Field selection on messages wrapped with the `*Unsafe` APIs now borrows sub-messages and strings instead of copying them into the evaluation arena, matching `ParsedMessageValue::GetField`. PiperOrigin-RevId: 991037181
1 parent 96346c7 commit 9602b67

2 files changed

Lines changed: 122 additions & 9 deletions

File tree

‎common/values/parsed_message_value.cc‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -278,12 +278,13 @@ class ParsedMessageValueQualifyState final
278278
const google::protobuf::Message* absl_nonnull message,
279279
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
280280
google::protobuf::MessageFactory* absl_nonnull message_factory,
281-
google::protobuf::Arena* absl_nonnull arena)
281+
google::protobuf::Arena* absl_nonnull arena, bool unsafe)
282282
: ProtoQualifyState(message, message->GetDescriptor(),
283283
message->GetReflection()),
284284
descriptor_pool_(descriptor_pool),
285285
message_factory_(message_factory),
286-
arena_(arena) {}
286+
arena_(arena),
287+
unsafe_(unsafe) {}
287288

288289
absl::optional<Value>& result() { return result_; }
289290

@@ -294,11 +295,18 @@ class ParsedMessageValueQualifyState final
294295

295296
void SetResultFromBool(bool value) override { result_ = BoolValue(value); }
296297

298+
// When `unsafe_` is set, the qualified message is externally managed (see
299+
// `UnsafeParsedMessageValue()`), so borrow field values instead of copying
300+
// them onto `arena_`. This matches `ParsedMessageValue::GetField()`.
297301
absl::Status SetResultFromField(const google::protobuf::Message* message,
298302
const google::protobuf::FieldDescriptor* field,
299303
ProtoWrapperTypeOptions unboxing_option,
300304
cel::MemoryManagerRef) override {
301-
result_ = Value::WrapField(unboxing_option, message, field,
305+
result_ =
306+
unsafe_
307+
? Value::WrapFieldUnsafe(unboxing_option, message, field,
308+
descriptor_pool_, message_factory_, arena_)
309+
: Value::WrapField(unboxing_option, message, field,
302310
descriptor_pool_, message_factory_, arena_);
303311
return absl::OkStatus();
304312
}
@@ -307,23 +315,32 @@ class ParsedMessageValueQualifyState final
307315
const google::protobuf::FieldDescriptor* field,
308316
int index,
309317
cel::MemoryManagerRef) override {
310-
result_ = Value::WrapRepeatedField(index, message, field, descriptor_pool_,
311-
message_factory_, arena_);
318+
result_ = unsafe_ ? Value::WrapRepeatedFieldUnsafe(index, message, field,
319+
descriptor_pool_,
320+
message_factory_, arena_)
321+
: Value::WrapRepeatedField(index, message, field,
322+
descriptor_pool_,
323+
message_factory_, arena_);
312324
return absl::OkStatus();
313325
}
314326

315327
absl::Status SetResultFromMapField(const google::protobuf::Message* message,
316328
const google::protobuf::FieldDescriptor* field,
317329
const google::protobuf::MapValueConstRef& value,
318330
cel::MemoryManagerRef) override {
319-
result_ = Value::WrapMapFieldValue(value, message, field, descriptor_pool_,
320-
message_factory_, arena_);
331+
result_ = unsafe_ ? Value::WrapMapFieldValueUnsafe(value, message, field,
332+
descriptor_pool_,
333+
message_factory_, arena_)
334+
: Value::WrapMapFieldValue(value, message, field,
335+
descriptor_pool_,
336+
message_factory_, arena_);
321337
return absl::OkStatus();
322338
}
323339

324340
const google::protobuf::DescriptorPool* absl_nonnull const descriptor_pool_;
325341
google::protobuf::MessageFactory* absl_nonnull const message_factory_;
326342
google::protobuf::Arena* absl_nonnull const arena_;
343+
const bool unsafe_;
327344
absl::optional<Value> result_;
328345
};
329346

@@ -345,8 +362,8 @@ absl::Status ParsedMessageValue::Qualify(
345362
if (ABSL_PREDICT_FALSE(qualifiers.empty())) {
346363
return absl::InvalidArgumentError("invalid select qualifier path.");
347364
}
348-
ParsedMessageValueQualifyState qualify_state(value_, descriptor_pool,
349-
message_factory, arena);
365+
ParsedMessageValueQualifyState qualify_state(
366+
value_, descriptor_pool, message_factory, arena, is_unsafe());
350367
for (int i = 0; i < qualifiers.size() - 1; i++) {
351368
const auto& qualifier = qualifiers[i];
352369
CEL_RETURN_IF_ERROR(qualify_state.ApplySelectQualifier(

‎common/values/parsed_message_value_test.cc‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "absl/status/status_matchers.h"
2020
#include "absl/strings/cord.h"
2121
#include "absl/strings/string_view.h"
22+
#include "base/attribute.h"
2223
#include "common/memory.h"
2324
#include "common/type.h"
2425
#include "common/value.h"
@@ -122,5 +123,100 @@ TEST_F(ParsedMessageValueTest, GetFieldByNumber) {
122123
IsOkAndHolds(BoolValueIs(false)));
123124
}
124125

126+
// A message that is not owned by the evaluation arena, wrapped via the unsafe
127+
// (borrowing) API, must not be deep-copied onto the arena when fields are
128+
// selected through `Qualify()`.
129+
TEST_F(ParsedMessageValueTest, QualifyUnsafeBorrowsMessageField) {
130+
TestAllTypesProto3 message; // Heap-allocated, not on `arena()`.
131+
message.mutable_standalone_message()->set_bb(42);
132+
Value wrapped = Value::WrapMessageUnsafe(&message, descriptor_pool(),
133+
message_factory(), arena());
134+
ASSERT_TRUE(wrapped.IsParsedMessage());
135+
136+
const SelectQualifier qualifiers[] = {FieldSpecifier{
137+
TestAllTypesProto3::kStandaloneMessageFieldNumber, "standalone_message"}};
138+
Value result;
139+
int count = 0;
140+
ASSERT_THAT(wrapped.GetParsedMessage().Qualify(
141+
qualifiers, /*presence_test=*/false, descriptor_pool(),
142+
message_factory(), arena(), &result, &count),
143+
IsOk());
144+
ASSERT_TRUE(result.IsParsedMessage());
145+
EXPECT_EQ(result.GetParsedMessage().message(), &message.standalone_message());
146+
147+
ParsedMessageValue safe_wrapped(&message, arena());
148+
ASSERT_THAT(safe_wrapped.Qualify(qualifiers, /*presence_test=*/false,
149+
descriptor_pool(), message_factory(),
150+
arena(), &result, &count),
151+
IsOk());
152+
ASSERT_TRUE(result.IsParsedMessage());
153+
EXPECT_NE(result.GetParsedMessage().message(), &message.standalone_message());
154+
EXPECT_EQ(result.GetParsedMessage().message()->GetArena(), arena());
155+
}
156+
157+
TEST_F(ParsedMessageValueTest, QualifyUnsafeBorrowsRepeatedMessageField) {
158+
TestAllTypesProto3 message; // Heap-allocated, not on `arena()`.
159+
message.add_repeated_nested_message()->set_bb(42);
160+
Value wrapped = Value::WrapMessageUnsafe(&message, descriptor_pool(),
161+
message_factory(), arena());
162+
ASSERT_TRUE(wrapped.IsParsedMessage());
163+
164+
const SelectQualifier qualifiers[] = {
165+
FieldSpecifier{TestAllTypesProto3::kRepeatedNestedMessageFieldNumber,
166+
"repeated_nested_message"},
167+
AttributeQualifier::OfInt(0)};
168+
Value result;
169+
int count = 0;
170+
ASSERT_THAT(wrapped.GetParsedMessage().Qualify(
171+
qualifiers, /*presence_test=*/false, descriptor_pool(),
172+
message_factory(), arena(), &result, &count),
173+
IsOk());
174+
ASSERT_TRUE(result.IsParsedMessage());
175+
EXPECT_EQ(result.GetParsedMessage().message(),
176+
&message.repeated_nested_message(0));
177+
178+
ParsedMessageValue safe_wrapped(&message, arena());
179+
ASSERT_THAT(safe_wrapped.Qualify(qualifiers, /*presence_test=*/false,
180+
descriptor_pool(), message_factory(),
181+
arena(), &result, &count),
182+
IsOk());
183+
ASSERT_TRUE(result.IsParsedMessage());
184+
EXPECT_NE(result.GetParsedMessage().message(),
185+
&message.repeated_nested_message(0));
186+
EXPECT_EQ(result.GetParsedMessage().message()->GetArena(), arena());
187+
}
188+
189+
TEST_F(ParsedMessageValueTest, QualifyUnsafeBorrowsMapMessageField) {
190+
TestAllTypesProto3 message; // Heap-allocated, not on `arena()`.
191+
(*message.mutable_map_string_message())["key"].set_bb(42);
192+
Value wrapped = Value::WrapMessageUnsafe(&message, descriptor_pool(),
193+
message_factory(), arena());
194+
ASSERT_TRUE(wrapped.IsParsedMessage());
195+
196+
const SelectQualifier qualifiers[] = {
197+
FieldSpecifier{TestAllTypesProto3::kMapStringMessageFieldNumber,
198+
"map_string_message"},
199+
AttributeQualifier::OfString("key")};
200+
Value result;
201+
int count = 0;
202+
ASSERT_THAT(wrapped.GetParsedMessage().Qualify(
203+
qualifiers, /*presence_test=*/false, descriptor_pool(),
204+
message_factory(), arena(), &result, &count),
205+
IsOk());
206+
ASSERT_TRUE(result.IsParsedMessage());
207+
EXPECT_EQ(result.GetParsedMessage().message(),
208+
&message.map_string_message().at("key"));
209+
210+
ParsedMessageValue safe_wrapped(&message, arena());
211+
ASSERT_THAT(safe_wrapped.Qualify(qualifiers, /*presence_test=*/false,
212+
descriptor_pool(), message_factory(),
213+
arena(), &result, &count),
214+
IsOk());
215+
ASSERT_TRUE(result.IsParsedMessage());
216+
EXPECT_NE(result.GetParsedMessage().message(),
217+
&message.map_string_message().at("key"));
218+
EXPECT_EQ(result.GetParsedMessage().message()->GetArena(), arena());
219+
}
220+
125221
} // namespace
126222
} // namespace cel

0 commit comments

Comments
 (0)