From 9253c428438d2b92eceb36c6ab9571afb57b0378 Mon Sep 17 00:00:00 2001
From: Eric Salo <salo@google.com>
Date: Fri, 24 Feb 2023 03:35:42 -0800
Subject: [PATCH] add error check for duplicate oneof field names

PiperOrigin-RevId: 512029347
---
 upb/reflection/oneof_def.c    | 10 ++++++++--
 upb/util/def_to_proto_test.cc | 15 +++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/upb/reflection/oneof_def.c b/upb/reflection/oneof_def.c
index ba198924cd..e33dba953f 100644
--- a/upb/reflection/oneof_def.c
+++ b/upb/reflection/oneof_def.c
@@ -128,11 +128,17 @@ void _upb_OneofDef_Insert(upb_DefBuilder* ctx, upb_OneofDef* o,
   // 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)) {
+  const bool number_exists = upb_inttable_lookup(&o->itof, number, NULL);
+  if (UPB_UNLIKELY(number_exists)) {
     _upb_DefBuilder_Errf(ctx, "oneof fields have the same number (%d)", number);
   }
 
+  // TODO(salo): More redundant work happening here.
+  const bool name_exists = upb_strtable_lookup2(&o->ntof, name, size, NULL);
+  if (UPB_UNLIKELY(name_exists)) {
+    _upb_DefBuilder_Errf(ctx, "oneof fields have the same name (%s)", name);
+  }
+
   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)) {
diff --git a/upb/util/def_to_proto_test.cc b/upb/util/def_to_proto_test.cc
index 1fa88a72ae..5bf6ddffb3 100644
--- a/upb/util/def_to_proto_test.cc
+++ b/upb/util/def_to_proto_test.cc
@@ -316,4 +316,19 @@ TEST(FuzzTest, RoundTripDescriptorRegression) {
                                                })pb"));
 }
 
+// Multiple oneof fields which have the same name.
+TEST(FuzzTest, RoundTripDescriptorRegressionOneofSameName) {
+  RoundTripDescriptor(ParseTextProtoOrDie(
+      R"pb(file {
+             name: "N"
+             package: ""
+             message_type {
+               name: "b"
+               field { name: "W" number: 1 type: TYPE_BYTES oneof_index: 0 }
+               field { name: "W" number: 17 type: TYPE_UINT32 oneof_index: 0 }
+               oneof_decl { name: "k" }
+             }
+           })pb"));
+}
+
 }  // namespace upb_test