Allow non-canonical upb extensions to be described by a regular field. PiperOrigin-RevId: 1002559655
diff --git a/upb/message/compare.c b/upb/message/compare.c index 549cc92..adea382 100644 --- a/upb/message/compare.c +++ b/upb/message/compare.c
@@ -206,7 +206,7 @@ count1++; const upb_MessageValue val1 = upb_Extension_GetValue(ext1); - const upb_MiniTableField* f = &e->UPB_PRIVATE(field); + const upb_MiniTableField* f = upb_MiniTableExtension_ToField(e); const upb_MiniTable* subm = upb_MiniTableField_IsSubMessage(f) ? upb_MiniTableExtension_GetSubMessage(e) : NULL;
diff --git a/upb/message/internal/accessors.h b/upb/message/internal/accessors.h index f38b5b8..47df57a 100644 --- a/upb/message/internal/accessors.h +++ b/upb/message/internal/accessors.h
@@ -222,7 +222,7 @@ UPB_API_INLINE bool upb_Message_HasExtension(const struct upb_Message* msg, const upb_MiniTableExtension* e) { - UPB_ASSERT(upb_MiniTableField_HasPresence(&e->UPB_PRIVATE(field))); + UPB_ASSERT(upb_MiniTableField_HasPresence(upb_MiniTableExtension_ToField(e))); return UPB_PRIVATE(_upb_Message_Getext)(msg, e) != NULL; } @@ -244,7 +244,7 @@ const struct upb_Message* msg, const upb_MiniTableExtension* mt_ext, const void* default_val, void* val) { const upb_Extension* ext = UPB_PRIVATE(_upb_Message_Getext)(msg, mt_ext); - const upb_MiniTableField* f = &mt_ext->UPB_PRIVATE(field); + const upb_MiniTableField* f = upb_MiniTableExtension_ToField(mt_ext); UPB_ASSUME(upb_MiniTableField_IsExtension(f)); if (ext) {
diff --git a/upb/message/internal/extension.c b/upb/message/internal/extension.c index 789985a..df5cebd 100644 --- a/upb/message/internal/extension.c +++ b/upb/message/internal/extension.c
@@ -42,7 +42,13 @@ struct upb_Message* msg, const upb_MiniTableExtension* e, upb_Arena* a, upb_TaggedAuxType tag) { UPB_ASSERT(!upb_Message_IsFrozen(msg)); - UPB_ASSERT(!upb_MiniTableField_IsMap(upb_MiniTableExtension_ToField(e))); + const upb_MiniTableField* f = upb_MiniTableExtension_ToField(e); + UPB_ASSERT(!upb_MiniTableField_IsMap(f)); + // Every extension, including a non-canonical extension that is described by + // a regular field, has a zero offset and presence: its value lives in the + // upb_Extension, and its presence is the existence of the upb_Extension. + UPB_ASSERT(UPB_PRIVATE(_upb_MiniTableField_Offset)(f) == 0); + UPB_ASSERT(f->presence == 0); // For Canonical Extensions, we check whether the extension has already been // set. If we find an extension with the same pointer and tag, we reuse it to // prevent duplicate entries for the same extension.
diff --git a/upb/message/internal/extension.h b/upb/message/internal/extension.h index c4033a0..6e123b4 100644 --- a/upb/message/internal/extension.h +++ b/upb/message/internal/extension.h
@@ -75,7 +75,7 @@ UPB_API_INLINE void upb_Extension_GetField(const upb_Extension* ext, void* val) { const upb_MiniTableField* f = upb_Extension_MiniTableField(ext); - UPB_ASSUME(upb_MiniTableField_IsExtension(f)); + UPB_ASSERT(!upb_MiniTableField_IsMap(f)); const void* data = UPB_PTR_AT(ext, UPB_ONLYBITS(kUpb_Extension_DataOffset), const void); UPB_PRIVATE(_upb_MiniTableField_DataCopy)(f, val, data); @@ -87,7 +87,7 @@ UPB_API_INLINE void upb_Extension_SetField(upb_Extension* ext, const void* val) { const upb_MiniTableField* f = upb_Extension_MiniTableField(ext); - UPB_ASSUME(upb_MiniTableField_IsExtension(f)); + UPB_ASSERT(!upb_MiniTableField_IsMap(f)); void* data = UPB_PTR_AT(ext, UPB_ONLYBITS(kUpb_Extension_DataOffset), void); UPB_PRIVATE(_upb_MiniTableField_DataCopy)(f, data, val); } @@ -368,7 +368,7 @@ // so that the extension can be zeroed with _upb_Message_AlignedMemsetZero(). UPB_NODISCARD UPB_INLINE size_t UPB_PRIVATE(_upb_Extension_Size)(const upb_MiniTableField* f) { - UPB_ASSERT(upb_MiniTableField_IsExtension(f)); + UPB_ASSERT(!upb_MiniTableField_IsMap(f)); static const uint8_t rep_sizes[] = { 1, // kUpb_FieldRep_1Byte 4, // kUpb_FieldRep_4Byte
diff --git a/upb/message/internal/message.h b/upb/message/internal/message.h index 5a3e115..a9a4081 100644 --- a/upb/message/internal/message.h +++ b/upb/message/internal/message.h
@@ -78,15 +78,40 @@ // pointer to the StringView can be anywhere in the allocation, and the // StringView may point to non-data memory. // - // For a non-canonical extension, its schema is known but not - // the one expected by the message, so it should be treated like an unknown - // field, but is stored as an extension to lazily defer serialization. + // Canonical extensions (tag 011) are extensions that are known to the + // message's schema: // - // Invariants that hold for all extensions, canonical or non-canonical: + // - Described by a complete upb_MiniTableExtension whose field is flagged + // as an extension. upb_MiniTableExtension_Extendee() returns the message + // it extends. + // - Never a map, since protobuf does not allow map extensions. // - // - The extension is described by a upb_MiniTableExtension whose field is - // flagged as an extension. - // - The field is never a map, since protobuf does not allow map extensions. + // Non-canonical extensions (tag 001) hold a field whose schema is known, but + // is not the one expected by the message. They are semantically unknown + // fields, but are stored as extensions to lazily defer serialization. A + // non-canonical extension is described by one of: + // + // - An actual extension of some message: a complete upb_MiniTableExtension + // whose field is flagged as an extension. + // upb_MiniTableExtension_Extendee() returns the message it extends. + // - A regular upb_MiniTableField, typically a copy of a field of another + // message (see upb_Message_Convert()), cast to upb_MiniTableExtension*. + // The field is not flagged as an extension and is not followed by an + // `extendee`. upb_MiniTableExtension_Extendee() returns NULL. + // + // Either way, a non-canonical extension is never a map (map fields are never + // stored as non-canonical extensions), and like for every extension, its + // field's `offset` and `presence` are 0: the value is stored in + // upb_Extension.data, and the extension is present iff it is in `aux_data`. + // + // Since a non-canonical extension may be a regular field, code that handles + // non-canonical extensions must only use field-level accessors + // (upb_MiniTableExtension_ToField(), _Number(), _CType(), _GetSubMessage(), + // _GetSubEnum()) and must handle a NULL upb_MiniTableExtension_Extendee(). + // Since such a field is not flagged as an extension, never pass it together + // with this message to the upb_Message_*Field() accessors (e.g. + // upb_Message_GetField()), which would treat it as a regular field of this + // message. uintptr_t ptr; } upb_TaggedAuxPtr;
diff --git a/upb/message/test.cc b/upb/message/test.cc index 252f479..2e8baee 100644 --- a/upb/message/test.cc +++ b/upb/message/test.cc
@@ -51,7 +51,9 @@ #include "upb/mini_table/extension.h" #include "upb/mini_table/extension_registry.h" #include "upb/mini_table/field.h" +#include "upb/mini_table/internal/field.h" #include "upb/mini_table/internal/message.h" +#include "upb/mini_table/internal/sub.h" #include "upb/mini_table/message.h" #include "upb/reflection/def.h" #include "upb/reflection/def.hpp" @@ -295,6 +297,8 @@ upb_MiniTableExtension ext; memset(&ext, 0, sizeof(ext)); ext.UPB_PRIVATE(field).UPB_ONLYBITS(number) = (1 << 29) + 5; + ext.UPB_PRIVATE(field).UPB_ONLYBITS(mode) = + kUpb_FieldMode_Scalar | kUpb_LabelFlags_IsExtension; ext.UPB_PRIVATE(extendee) = &upb_0test__TestMessageSet_msg_init; EXPECT_EQ(upb_ExtensionRegistry_Add(reg, &ext), @@ -1072,6 +1076,139 @@ EXPECT_EQ(count, 2); } +// A copy of a regular field of another message with its offset and presence +// cleared, followed by a copy of its sub (if any) that `submsg_ofs` points at. +struct RegularFieldCopy { + upb_MiniTableField field; + upb_MiniTableSubInternal sub; +}; + +static RegularFieldCopy CopyRegularField(const upb_MiniTableField* src) { + RegularFieldCopy copy; + memset(©, 0, sizeof(copy)); + copy.field = *src; + copy.field.UPB_PRIVATE(offset) = 0; + copy.field.presence = 0; + if (src->UPB_PRIVATE(submsg_ofs) != kUpb_NoSub) { + copy.sub = *UPB_PRIVATE(_upb_MiniTableField_Sub)(src); + copy.field.UPB_PRIVATE(submsg_ofs) = + offsetof(RegularFieldCopy, sub) / kUpb_SubmsgOffsetBytes; + } + return copy; +} + +// A non-canonical extension may be described by a regular field, i.e. a field +// that is not flagged as an extension and is not followed by an `extendee`. +// Like for every extension, its offset and presence are 0. +TEST(MessageTest, NonCanonicalExtensionFromRegularField) { + upb::Arena arena; + const upb_MiniTable* src_mt = + &protobuf_0test_0messages__proto3__TestAllTypesProto3_msg_init; + const upb_MiniTable* dst_mt = &upb_0test__TestExtensions_msg_init; + upb_Message* dst = upb_Message_New(dst_mt, arena.ptr()); + ASSERT_NE(dst, nullptr); + + const upb_MiniTableField* src_str_f = + upb_MiniTable_FindFieldByNumber(src_mt, 14); + const upb_MiniTableField* src_msg_f = + upb_MiniTable_FindFieldByNumber(src_mt, 18); + const upb_MiniTableField* src_arr_f = + upb_MiniTable_FindFieldByNumber(src_mt, 31); + ASSERT_NE(src_str_f, nullptr); + ASSERT_NE(src_msg_f, nullptr); + ASSERT_NE(src_arr_f, nullptr); + const RegularFieldCopy str_copy = CopyRegularField(src_str_f); + const RegularFieldCopy msg_copy = CopyRegularField(src_msg_f); + const RegularFieldCopy arr_copy = CopyRegularField(src_arr_f); + const upb_MiniTableField* str_f = &str_copy.field; + const upb_MiniTableField* msg_f = &msg_copy.field; + const upb_MiniTableField* arr_f = &arr_copy.field; + for (const upb_MiniTableField* f : {str_f, msg_f, arr_f}) { + ASSERT_FALSE(upb_MiniTableField_IsExtension(f)); + } + + upb_StringView str = upb_StringView_FromString("longer than twelve bytes"); + protobuf_test_messages_proto3_TestAllTypesProto3_NestedMessage* sub = + protobuf_test_messages_proto3_TestAllTypesProto3_NestedMessage_new( + arena.ptr()); + protobuf_test_messages_proto3_TestAllTypesProto3_NestedMessage_set_a(sub, + 123); + upb_Array* arr = upb_Array_New(arena.ptr(), kUpb_CType_Int32); + ASSERT_NE(arr, nullptr); + upb_MessageValue elem; + elem.int32_val = 7; + ASSERT_TRUE(upb_Array_Append(arr, elem, arena.ptr())); + elem.int32_val = 8; + ASSERT_TRUE(upb_Array_Append(arr, elem, arena.ptr())); + + ASSERT_TRUE(UPB_PRIVATE(_upb_Message_SetNonCanonicalExtension)( + dst, (const upb_MiniTableExtension*)str_f, &str, arena.ptr())); + ASSERT_TRUE(UPB_PRIVATE(_upb_Message_SetNonCanonicalExtension)( + dst, (const upb_MiniTableExtension*)msg_f, &sub, arena.ptr())); + ASSERT_TRUE(UPB_PRIVATE(_upb_Message_SetNonCanonicalExtension)( + dst, (const upb_MiniTableExtension*)arr_f, &arr, arena.ptr())); + + // Field-level accessors work, and there is no extendee. + uintptr_t iter = kUpb_Message_UnknownBegin; + upb_MessageUnknown unknown; + int count = 0; + while (upb_Message_NextUnknown2(dst, &unknown, &iter)) { + ASSERT_EQ(unknown.type, kUpb_MessageUnknownType_NonCanonicalExtension); + const upb_MiniTableExtension* e = + upb_Extension_MiniTableExtension(unknown.value.extension); + EXPECT_EQ(upb_MiniTableExtension_Extendee(e), nullptr); + if (upb_MiniTableExtension_Number(e) == 18) { + EXPECT_EQ(upb_MiniTableExtension_GetSubMessage(e), + upb_MiniTable_SubMessage(src_msg_f)); + } + count++; + } + EXPECT_EQ(count, 3); + + // Wire encoding round-trips through the original schema. + char* buf; + size_t size; + ASSERT_EQ(upb_Encode(dst, dst_mt, 0, arena.ptr(), &buf, &size), + kUpb_EncodeStatus_Ok); + protobuf_test_messages_proto3_TestAllTypesProto3* parsed = + protobuf_test_messages_proto3_TestAllTypesProto3_parse(buf, size, + arena.ptr()); + ASSERT_NE(parsed, nullptr); + upb_StringView parsed_str = + protobuf_test_messages_proto3_TestAllTypesProto3_optional_string(parsed); + EXPECT_EQ(absl::string_view(parsed_str.data, parsed_str.size), + "longer than twelve bytes"); + const protobuf_test_messages_proto3_TestAllTypesProto3_NestedMessage* + parsed_sub = + protobuf_test_messages_proto3_TestAllTypesProto3_optional_nested_message( + parsed); + ASSERT_NE(parsed_sub, nullptr); + EXPECT_EQ(protobuf_test_messages_proto3_TestAllTypesProto3_NestedMessage_a( + parsed_sub), + 123); + size_t arr_size; + const int32_t* parsed_arr = + protobuf_test_messages_proto3_TestAllTypesProto3_repeated_int32( + parsed, &arr_size); + ASSERT_EQ(arr_size, 2); + EXPECT_EQ(parsed_arr[0], 7); + EXPECT_EQ(parsed_arr[1], 8); + + // Unknown field comparison encodes each non-canonical extension. + upb_Message* reparsed = upb_Message_New(dst_mt, arena.ptr()); + ASSERT_NE(reparsed, nullptr); + ASSERT_EQ(upb_Decode(buf, size, reparsed, dst_mt, nullptr, 0, arena.ptr()), + kUpb_DecodeStatus_Ok); + EXPECT_TRUE(upb_Message_IsEqual(dst, reparsed, dst_mt, + kUpb_CompareOption_IncludeUnknownFields)); + + // Text encoding resolves the sub-message through the field. + char text[512]; + size_t text_size = upb_DebugString(dst, dst_mt, 0, text, sizeof(text)); + ASSERT_LT(text_size, sizeof(text)); + EXPECT_NE(std::string(text, text_size).find("123"), std::string::npos); +} + TEST(MessageTest, CoexistCanonicalAndNonCanonicalExtension) { upb::Arena arena; upb_test_TestExtensions* ext_msg = upb_test_TestExtensions_new(arena.ptr());
diff --git a/upb/message/unknown_fields.h b/upb/message/unknown_fields.h index 5a0b8bc..5b4bac9 100644 --- a/upb/message/unknown_fields.h +++ b/upb/message/unknown_fields.h
@@ -31,6 +31,27 @@ // Represents an unknown field in a message, whether it's in a serialized // (upb_StringView) or parsed non-canonical extension (upb_Extension*) format. +// +// A non-canonical extension is described by one of: +// +// - An actual extension of some message. upb_Extension_MiniTableExtension() +// returns a complete upb_MiniTableExtension, and +// upb_MiniTableExtension_Extendee() returns the message it extends. +// - A regular field, typically a copy of a field of another message. +// upb_Extension_MiniTableExtension() returns that upb_MiniTableField cast +// to upb_MiniTableExtension*, and upb_MiniTableExtension_Extendee() +// returns NULL. Like for an actual extension, the field's offset and +// presence are 0. +// +// Since either is possible, only use field-level accessors on a non-canonical +// extension (e.g. upb_Extension_MiniTableField(), upb_MiniTableField_Number(), +// upb_MiniTableField_CType(), upb_MiniTable_SubMessage()), and handle a NULL +// result from upb_MiniTableExtension_Extendee(). +// +// Since the field may not be flagged as an extension, a non-canonical +// extension's field must not be used with the containing message (e.g. +// upb_Message_GetField()). Read its value with upb_Extension_GetValue() +// instead. typedef struct upb_MessageUnknown { uint8_t type; union {
diff --git a/upb/mini_table/BUILD b/upb/mini_table/BUILD index abde510..dcbdd19 100644 --- a/upb/mini_table/BUILD +++ b/upb/mini_table/BUILD
@@ -139,6 +139,23 @@ ) cc_test( + name = "extension_test", + srcs = ["extension_test.cc"], + deps = [ + ":mini_table", + "//upb/base", + "//upb/mem", + "//upb/mini_descriptor", + "//upb/mini_descriptor:internal", + "//upb/test:test_messages_proto2_upb_minitable", + "//upb/test:test_proto_upb_minitable", + "//upb/test:test_upb_proto", + "@googletest//:gtest", + "@googletest//:gtest_main", + ], +) + +cc_test( name = "generated_registry_test", srcs = ["generated_registry_test.cc"], deps = [
diff --git a/upb/mini_table/extension_registry.c b/upb/mini_table/extension_registry.c index a2f23ea..7091f29 100644 --- a/upb/mini_table/extension_registry.c +++ b/upb/mini_table/extension_registry.c
@@ -14,6 +14,7 @@ #include "upb/hash/ext_table.h" #include "upb/mem/arena.h" #include "upb/mini_table/extension.h" +#include "upb/mini_table/field.h" #include "upb/mini_table/message.h" // Must be last. @@ -38,6 +39,8 @@ offsetof(upb_MiniTableExtension, UPB_PRIVATE(field).UPB_PRIVATE(number)) == 0, "Extension must be first-member-of-struct convertable with uint32_t"); + // Only actual extension fields can be registered. + UPB_ASSERT(upb_MiniTableField_IsExtension(upb_MiniTableExtension_ToField(e))); uint32_t fieldnum = upb_MiniTableExtension_Number(e); const upb_MiniTable* extendee = upb_MiniTableExtension_Extendee(e);
diff --git a/upb/mini_table/extension_test.cc b/upb/mini_table/extension_test.cc new file mode 100644 index 0000000..f90bce6 --- /dev/null +++ b/upb/mini_table/extension_test.cc
@@ -0,0 +1,134 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2026 Google LLC. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#include "upb/mini_table/extension.h" + +#include <stdint.h> + +#include <gtest/gtest.h> +#include "google/protobuf/test_messages_proto2.upb_minitable.h" +#include "upb/base/descriptor_constants.h" +#include "upb/base/status.h" +#include "upb/mem/arena.hpp" +#include "upb/mini_descriptor/decode.h" +#include "upb/mini_descriptor/internal/encode.hpp" +#include "upb/mini_descriptor/internal/modifiers.h" +#include "upb/mini_table/enum.h" +#include "upb/mini_table/field.h" +#include "upb/mini_table/message.h" +#include "upb/test/test.upb_minitable.h" + +namespace { + +const upb_MiniTable* Proto2Mt() { + return &protobuf_0test_0messages__proto2__TestAllTypesProto2_msg_init; +} + +// Proto2 `TestAllTypesProto2` field numbers. +constexpr uint32_t kOptionalNestedMessage = 18; +constexpr uint32_t kOptionalNestedEnum = 21; +constexpr uint32_t kOptionalForeignEnum = 22; + +TEST(MiniTableExtensionTest, GeneratedMessageExtension) { + const upb_MiniTableExtension* e = upb_test_ModelExtension1_model_ext_ext; + const upb_MiniTableField* f = upb_MiniTableExtension_ToField(e); + + EXPECT_EQ(static_cast<const void*>(f), static_cast<const void*>(e)); + EXPECT_TRUE(upb_MiniTableField_IsExtension(f)); + EXPECT_EQ(upb_MiniTableExtension_Number(e), 1547); + EXPECT_EQ(upb_MiniTableExtension_CType(e), kUpb_CType_Message); + EXPECT_EQ(upb_MiniTableExtension_Extendee(e), + &upb_0test__ModelWithExtensions_msg_init); + EXPECT_EQ(upb_MiniTableExtension_GetSubMessage(e), + &upb_0test__ModelExtension1_msg_init); + EXPECT_EQ(upb_MiniTableExtension_GetSubEnum(e), nullptr); +} + +// The accessors only read field-level data unless the field is flagged as an +// extension, so they also work on a regular field of a message. +TEST(MiniTableExtensionTest, AccessorsOnRegularField) { + const upb_MiniTableField* msg_f = + upb_MiniTable_FindFieldByNumber(Proto2Mt(), kOptionalNestedMessage); + const upb_MiniTableField* enum_f = + upb_MiniTable_FindFieldByNumber(Proto2Mt(), kOptionalNestedEnum); + ASSERT_NE(msg_f, nullptr); + ASSERT_NE(enum_f, nullptr); + ASSERT_FALSE(upb_MiniTableField_IsExtension(msg_f)); + ASSERT_FALSE(upb_MiniTableField_IsExtension(enum_f)); + + const upb_MiniTableExtension* msg_e = + reinterpret_cast<const upb_MiniTableExtension*>(msg_f); + EXPECT_EQ(upb_MiniTableExtension_ToField(msg_e), msg_f); + EXPECT_EQ(upb_MiniTableExtension_Number(msg_e), kOptionalNestedMessage); + EXPECT_EQ(upb_MiniTableExtension_CType(msg_e), kUpb_CType_Message); + EXPECT_EQ(upb_MiniTableExtension_Extendee(msg_e), nullptr); + EXPECT_EQ(upb_MiniTableExtension_GetSubMessage(msg_e), + upb_MiniTable_SubMessage(msg_f)); + EXPECT_NE(upb_MiniTableExtension_GetSubMessage(msg_e), nullptr); + EXPECT_EQ(upb_MiniTableExtension_GetSubEnum(msg_e), nullptr); + + const upb_MiniTableExtension* enum_e = + reinterpret_cast<const upb_MiniTableExtension*>(enum_f); + EXPECT_EQ(upb_MiniTableExtension_ToField(enum_e), enum_f); + EXPECT_EQ(upb_MiniTableExtension_Number(enum_e), kOptionalNestedEnum); + EXPECT_EQ(upb_MiniTableExtension_CType(enum_e), kUpb_CType_Enum); + EXPECT_EQ(upb_MiniTableExtension_Extendee(enum_e), nullptr); + EXPECT_EQ(upb_MiniTableExtension_GetSubMessage(enum_e), nullptr); + EXPECT_EQ(upb_MiniTableExtension_GetSubEnum(enum_e), + upb_MiniTable_GetSubEnumTable(enum_f)); + EXPECT_NE(upb_MiniTableExtension_GetSubEnum(enum_e), nullptr); +} + +TEST(MiniTableExtensionTest, SetSubEnum) { + upb::Arena arena; + const upb_MiniTableEnum* nested_enum = upb_MiniTable_GetSubEnumTable( + upb_MiniTable_FindFieldByNumber(Proto2Mt(), kOptionalNestedEnum)); + const upb_MiniTableEnum* foreign_enum = upb_MiniTable_GetSubEnumTable( + upb_MiniTable_FindFieldByNumber(Proto2Mt(), kOptionalForeignEnum)); + ASSERT_NE(nested_enum, nullptr); + ASSERT_NE(foreign_enum, nullptr); + ASSERT_NE(nested_enum, foreign_enum); + + upb::MtDataEncoder enc; + ASSERT_TRUE(enc.EncodeExtension(kUpb_FieldType_Enum, 199, + kUpb_FieldModifier_IsClosedEnum)); + upb_Status status; + upb_Status_Clear(&status); + upb_MiniTableExtension* e = upb_MiniTableExtension_BuildEnum( + enc.data().data(), enc.data().size(), Proto2Mt(), nested_enum, + arena.ptr(), &status); + ASSERT_NE(e, nullptr) << upb_Status_ErrorMessage(&status); + + EXPECT_EQ(upb_MiniTableExtension_Extendee(e), Proto2Mt()); + EXPECT_EQ(upb_MiniTableExtension_GetSubEnum(e), nested_enum); + EXPECT_TRUE(upb_MiniTableExtension_SetSubEnum(e, foreign_enum)); + EXPECT_EQ(upb_MiniTableExtension_GetSubEnum(e), foreign_enum); + EXPECT_FALSE(upb_MiniTableExtension_SetSubMessage( + e, &upb_0test__ModelExtension1_msg_init)); + EXPECT_EQ(upb_MiniTableExtension_GetSubEnum(e), foreign_enum); +} + +TEST(MiniTableExtensionTest, SetSubOnScalarExtensionFails) { + upb::Arena arena; + upb::MtDataEncoder enc; + ASSERT_TRUE(enc.EncodeExtension(kUpb_FieldType_Int32, 198, 0)); + upb_Status status; + upb_Status_Clear(&status); + upb_MiniTableExtension* e = upb_MiniTableExtension_Build( + enc.data().data(), enc.data().size(), Proto2Mt(), arena.ptr(), &status); + ASSERT_NE(e, nullptr) << upb_Status_ErrorMessage(&status); + + EXPECT_EQ(upb_MiniTableExtension_CType(e), kUpb_CType_Int32); + EXPECT_EQ(upb_MiniTableExtension_Number(e), 198); + EXPECT_EQ(upb_MiniTableExtension_Extendee(e), Proto2Mt()); + EXPECT_FALSE(upb_MiniTableExtension_SetSubMessage( + e, &upb_0test__ModelExtension1_msg_init)); + EXPECT_EQ(upb_MiniTableExtension_GetSubMessage(e), nullptr); + EXPECT_EQ(upb_MiniTableExtension_GetSubEnum(e), nullptr); +} + +} // namespace
diff --git a/upb/mini_table/internal/extension.h b/upb/mini_table/internal/extension.h index cb2f809..74f9833 100644 --- a/upb/mini_table/internal/extension.h +++ b/upb/mini_table/internal/extension.h
@@ -27,7 +27,12 @@ // extension, it's typically converted from a canonical extension via the // upb_Message_Convert() API, but is not registered on the extension // registry of the target message when it gets converted. In this case, the - // `extendee` info is present on the source message before conversion. + // `extendee` info is present on the source message before conversion. A + // non-canonical extension that is described by a regular field (e.g. a copy + // of a field of another message) has no `extendee` at all. + // + // Only read this through upb_MiniTableExtension_Extendee(), which checks that + // the field is flagged as an extension first and returns NULL otherwise. const struct upb_MiniTable* UPB_PRIVATE(extendee); }; @@ -44,18 +49,39 @@ extern "C" { #endif +// Returns the field of `e`. +// +// Accessors below only read field-level data through this function, unless +// they first check upb_MiniTableField_IsExtension(). +UPB_API_INLINE const struct upb_MiniTableField* upb_MiniTableExtension_ToField( + const struct upb_MiniTableExtension* e) { + UPB_STATIC_ASSERT( + offsetof(struct upb_MiniTableExtension, UPB_PRIVATE(field)) == 0, + "field must be the first member of upb_MiniTableExtension"); + return (const struct upb_MiniTableField*)e; +} + +UPB_INLINE struct upb_MiniTableField* UPB_PRIVATE( + _upb_MiniTableExtension_MutableField)(struct upb_MiniTableExtension* e) { + return (struct upb_MiniTableField*)e; +} + UPB_API_INLINE upb_CType upb_MiniTableExtension_CType(const struct upb_MiniTableExtension* e) { - return upb_MiniTableField_CType(&e->UPB_PRIVATE(field)); + return upb_MiniTableField_CType(upb_MiniTableExtension_ToField(e)); } UPB_API_INLINE uint32_t upb_MiniTableExtension_Number(const struct upb_MiniTableExtension* e) { - return e->UPB_PRIVATE(field).UPB_ONLYBITS(number); + return upb_MiniTableField_Number(upb_MiniTableExtension_ToField(e)); } +// Returns NULL if `e` does not describe an actual extension field. UPB_API_INLINE const struct upb_MiniTable* upb_MiniTableExtension_Extendee( const struct upb_MiniTableExtension* e) { + if (!upb_MiniTableField_IsExtension(upb_MiniTableExtension_ToField(e))) { + return NULL; + } return e->UPB_PRIVATE(extendee); } @@ -64,7 +90,7 @@ if (upb_MiniTableExtension_CType(e) != kUpb_CType_Message) { return NULL; } - return upb_MiniTable_GetSubMessageTable(&e->UPB_PRIVATE(field)); + return upb_MiniTable_GetSubMessageTable(upb_MiniTableExtension_ToField(e)); } UPB_API_INLINE const struct upb_MiniTableEnum* @@ -72,44 +98,39 @@ if (upb_MiniTableExtension_CType(e) != kUpb_CType_Enum) { return NULL; } - return upb_MiniTable_GetSubEnumTable(&e->UPB_PRIVATE(field)); + return upb_MiniTable_GetSubEnumTable(upb_MiniTableExtension_ToField(e)); } UPB_API_INLINE bool upb_MiniTableExtension_SetSubMessage( struct upb_MiniTableExtension* e, const struct upb_MiniTable* m) { - if (e->UPB_PRIVATE(field).UPB_PRIVATE(descriptortype) != - kUpb_FieldType_Message && - e->UPB_PRIVATE(field).UPB_PRIVATE(descriptortype) != - kUpb_FieldType_Group) { + struct upb_MiniTableField* f = + UPB_PRIVATE(_upb_MiniTableExtension_MutableField)(e); + if (f->UPB_PRIVATE(descriptortype) != kUpb_FieldType_Message && + f->UPB_PRIVATE(descriptortype) != kUpb_FieldType_Group) { return false; } if (m->UPB_PRIVATE(ext) & kUpb_ExtMode_IsMapEntry) { return false; } - UPB_PRIVATE(_upb_MiniTableField_Sub)(&e->UPB_PRIVATE(field)) - ->UPB_PRIVATE(submsg) = m; + UPB_PRIVATE(_upb_MiniTableField_Sub)(f)->UPB_PRIVATE(submsg) = m; return true; } UPB_API_INLINE bool upb_MiniTableExtension_SetSubEnum( struct upb_MiniTableExtension* e, const struct upb_MiniTableEnum* en) { - if (e->UPB_PRIVATE(field).UPB_PRIVATE(descriptortype) != - kUpb_FieldType_Enum) { + struct upb_MiniTableField* f = + UPB_PRIVATE(_upb_MiniTableExtension_MutableField)(e); + if (f->UPB_PRIVATE(descriptortype) != kUpb_FieldType_Enum) { return false; } - UPB_PRIVATE(_upb_MiniTableField_Sub)(&e->UPB_PRIVATE(field)) - ->UPB_PRIVATE(subenum) = en; + UPB_PRIVATE(_upb_MiniTableField_Sub)(f)->UPB_PRIVATE(subenum) = en; return true; } -UPB_API_INLINE const struct upb_MiniTableField* upb_MiniTableExtension_ToField( - const struct upb_MiniTableExtension* e) { - return &e->UPB_PRIVATE(field); -} - UPB_INLINE upb_FieldRep UPB_PRIVATE(_upb_MiniTableExtension_GetRep)( const struct upb_MiniTableExtension* e) { - return UPB_PRIVATE(_upb_MiniTableField_GetRep)(&e->UPB_PRIVATE(field)); + return UPB_PRIVATE(_upb_MiniTableField_GetRep)( + upb_MiniTableExtension_ToField(e)); } #ifdef __cplusplus
diff --git a/upb/test/fuzz_util.cc b/upb/test/fuzz_util.cc index ed39d18..1c9976f 100644 --- a/upb/test/fuzz_util.cc +++ b/upb/test/fuzz_util.cc
@@ -152,7 +152,8 @@ extendee, upb_MiniTableExtension_Number(ext)) != nullptr) { continue; } - if (upb_ExtensionRegistry_Lookup(*exts, ext->UPB_PRIVATE(extendee), + if (upb_ExtensionRegistry_Lookup(*exts, + upb_MiniTableExtension_Extendee(ext), upb_MiniTableExtension_Number(ext))) continue; auto status = upb_ExtensionRegistry_AddArray(
diff --git a/upb/text/internal/encode.c b/upb/text/internal/encode.c index e6eb3e5..83b8461 100644 --- a/upb/text/internal/encode.c +++ b/upb/text/internal/encode.c
@@ -387,7 +387,7 @@ upb_MessageValue val_ext; iter = kUpb_Message_ExtensionBegin; while (upb_Message_NextExtension(msg, &ext, &val_ext, &iter)) { - const upb_MiniTableField* f = &ext->UPB_PRIVATE(field); + const upb_MiniTableField* f = upb_MiniTableExtension_ToField(ext); // It is not sufficient to only pass |f| as we lose valuable information // about sub-messages. It is required that we pass |ext|. if (upb_MiniTableField_IsMap(f)) {
diff --git a/upb/wire/decode.c b/upb/wire/decode.c index f24edbf..90ae322 100644 --- a/upb/wire/decode.c +++ b/upb/wire/decode.c
@@ -612,7 +612,7 @@ upb_Message* submsg = upb_Extension_GetMutableMessage(ext); submsg = _upb_Decoder_NewSubMessage2( d, upb_MiniTableExtension_GetSubMessage(item_mt), - &item_mt->UPB_PRIVATE(field), &submsg); + upb_MiniTableExtension_ToField(item_mt), &submsg); upb_Extension_SetMessage(ext, submsg); // upb_Decode_LimitDepth() takes uint32_t, d->depth - 1 can not be negative. if (d->depth <= 1) { @@ -736,7 +736,7 @@ wire_type == kUpb_WireType_Delimited)) { const upb_MiniTableExtension* ext = upb_ExtensionRegistry_Lookup(d->extreg, t, field_number); - if (ext) return &ext->UPB_PRIVATE(field); + if (ext) return upb_MiniTableExtension_ToField(ext); } else if (ext_mode == kUpb_ExtMode_IsMessageSet) { if (field_number == kUpb_MsgSet_Item) { static upb_MiniTableField item = {
diff --git a/upb/wire/internal/encoder.c b/upb/wire/internal/encoder.c index 316909f..e340703 100644 --- a/upb/wire/internal/encoder.c +++ b/upb/wire/internal/encoder.c
@@ -669,6 +669,8 @@ // MessageSet items are always singular submessages. This holds for canonical // extensions of a MessageSet, but must also hold for any non-canonical // extension that is encoded as a MessageSet item. + UPB_ASSERT( + upb_MiniTableField_IsExtension(upb_MiniTableExtension_ToField(ext))); UPB_ASSERT(upb_MiniTableField_IsScalar(upb_MiniTableExtension_ToField(ext))); UPB_ASSERT(upb_MiniTableExtension_CType(ext) == kUpb_CType_Message); size_t size; @@ -690,7 +692,7 @@ ptr = encode_msgset_item(ptr, e, ext, ext_val); } else { ptr = encode_field(ptr, e, &ext_val.UPB_PRIVATE(ext_msg_val), - &ext->UPB_PRIVATE(field)); + upb_MiniTableExtension_ToField(ext)); } return ptr; }