detect and reject repeated field numbers within a oneof

PiperOrigin-RevId: 509707627

upb/reflection/field_def.c

@@ -638,8 +638,7 @@ static void _upb_FieldDef_Create(upb_DefBuilder* ctx, const char* prefix,
     upb_OneofDef* oneof = (upb_OneofDef*)upb_MessageDef_Oneof(m, oneof_index);
     f->scope.oneof = oneof;
 
-    bool ok = _upb_OneofDef_Insert(oneof, f, name.data, name.size, ctx->arena);
-    if (!ok) _upb_DefBuilder_OomErr(ctx);
+    _upb_OneofDef_Insert(ctx, oneof, f, name.data, name.size);
   }
 
   UPB_DEF_SET_OPTIONS(f->opts, FieldDescriptorProto, FieldOptions, field_proto);

upb/reflection/oneof_def.c

@@ -115,15 +115,29 @@ const upb_FieldDef* upb_OneofDef_LookupNumber(const upb_OneofDef* o,
                                                   : NULL;
 }
 
-bool _upb_OneofDef_Insert(upb_OneofDef* o, const upb_FieldDef* f,
-                          const char* name, size_t size, upb_Arena* a) {
+void _upb_OneofDef_Insert(upb_DefBuilder* ctx, upb_OneofDef* o,
+                          const upb_FieldDef* f, const char* name,
+                          size_t size) {
   o->field_count++;
   if (_upb_FieldDef_IsProto3Optional(f)) o->synthetic = true;
 
   const int number = upb_FieldDef_Number(f);
   const upb_value v = upb_value_constptr(f);
-  return upb_inttable_insert(&o->itof, number, v, a) &&
-         upb_strtable_insert(&o->ntof, name, size, v, a);
+
+  // TODO(salo): This lookup is unfortunate because we also perform it when
+  // inserting into the message's table. Unfortunately that step occurs after
+  // this one and moving things around could be tricky so let's leave it for
+  // a future refactoring.
+  const bool exists = upb_inttable_lookup(&o->itof, number, NULL);
+  if (UPB_UNLIKELY(exists)) {
+    _upb_DefBuilder_Errf(ctx, "oneof fields have the same number (%d)", number);
+  }
+
+  const bool ok = upb_inttable_insert(&o->itof, number, v, ctx->arena) &&
+                  upb_strtable_insert(&o->ntof, name, size, v, ctx->arena);
+  if (UPB_UNLIKELY(!ok)) {
+    _upb_DefBuilder_OomErr(ctx);
+  }
 }
 
 // Returns the synthetic count.

upb/reflection/oneof_def_internal.h

@@ -38,8 +38,8 @@ extern "C" {
 #endif
 
 upb_OneofDef* _upb_OneofDef_At(const upb_OneofDef* o, int i);
-bool _upb_OneofDef_Insert(upb_OneofDef* o, const upb_FieldDef* f,
-                          const char* name, size_t size, upb_Arena* a);
+void _upb_OneofDef_Insert(upb_DefBuilder* ctx, upb_OneofDef* o,
+                          const upb_FieldDef* f, const char* name, size_t size);
 
 // Allocate and initialize an array of |n| oneof defs owned by |m|.
 upb_OneofDef* _upb_OneofDefs_New(

upb/util/def_to_proto_test.cc

@@ -176,6 +176,19 @@ TEST(FuzzTest, EditionEmbeddedNull) {
       ParseTextProtoOrDie(R"pb(file { name: "n" edition: "\000" })pb"));
 }
 
+TEST(FuzzTest, DuplicateOneofIndex) {
+  RoundTripDescriptor(ParseTextProtoOrDie(
+      R"pb(file {
+             name: "F"
+             message_type {
+               name: "M"
+               oneof_decl { name: "O" }
+               field { name: "f1" number: 1 type: TYPE_INT32 oneof_index: 0 }
+               field { name: "f2" number: 1 type: TYPE_INT32 oneof_index: 0 }
+             }
+           })pb"));
+}
+
 TEST(FuzzTest, NanValue) {
   RoundTripDescriptor(ParseTextProtoOrDie(
       R"pb(file {