descriptor: reject duplicate oneof names

.changes/unreleased/fixed-20260903-113000.yaml

@@ -0,0 +1,4 @@
+kind: Fixed
+body: |-
+    **`DescriptorPool` rejects duplicate oneof names within a message** (#415), as `protoc` does. Only hand-built or synthesized descriptor sets can reach this state; the rejection is the new `PoolError::DuplicateOneofName` variant.
+time: 2026-09-03T11:30:00+02:00

buffa-descriptor/src/pool.rs

@@ -206,6 +206,8 @@ pub enum PoolError {
         field: String,
         index: i32,
     },
+    /// Two oneof declarations in one message have the same name.
+    DuplicateOneofName { message: String, name: String },
     /// A field number is outside the valid range
     /// `[1, MAX_FIELD_NUMBER]` (`(1 << 29) - 1`).
     InvalidFieldNumber { field: String, number: i32 },
@@ -377,6 +379,12 @@ impl core::fmt::Display for PoolError {
                 f,
                 "field {field} in message {message} has invalid oneof index {index}"
             ),
+            Self::DuplicateOneofName { message, name } => {
+                write!(
+                    f,
+                    "message {message} declares oneof name {name:?} more than once"
+                )
+            }
             Self::InvalidFieldNumber { field, number } => {
                 write!(f, "field {field} has invalid field number {number}")
             }
@@ -1477,16 +1485,23 @@ impl DescriptorPool {
         }
 
         // Build oneof descriptors. Track member field indices as we go.
-        let mut oneofs: Vec<OneofDescriptor> = msg
-            .oneof_decl
-            .iter()
-            .map(|o| OneofDescriptor {
-                name: o.name.clone().unwrap_or_default(),
+        let mut oneof_names: BTreeSet<&str> = BTreeSet::new();
+        let mut oneofs = Vec::with_capacity(msg.oneof_decl.len());
+        for o in &msg.oneof_decl {
+            let oneof_name = o.name.as_deref().unwrap_or("");
+            if !oneof_names.insert(oneof_name) {
+                return Err(PoolError::DuplicateOneofName {
+                    message: fqn,
+                    name: oneof_name.to_string(),
+                });
+            }
+            oneofs.push(OneofDescriptor {
+                name: oneof_name.to_string(),
                 field_indices: Vec::new(),
                 synthetic: false,
                 options: clone_options(&o.options),
-            })
-            .collect();
+            });
+        }
 
         // Build field descriptors.
         let mut fields = Vec::with_capacity(field_count);

buffa-descriptor/tests/pool_e2e.rs

@@ -484,6 +484,94 @@ fn oneof_links() {
     assert_eq!(o.field_indices(), vec![0, 1, 2]);
 }
 
+#[test]
+fn duplicate_oneof_names_are_rejected_transactionally() {
+    use buffa_descriptor::generated::descriptor::field_descriptor_proto::Type;
+    use buffa_descriptor::generated::descriptor::{
+        DescriptorProto, FieldDescriptorProto, OneofDescriptorProto,
+    };
+
+    // Each oneof gets a member so the descriptor is invalid for the duplicate
+    // name alone (protoc separately rejects an empty oneof).
+    let member = |name: &str, number: i32, oneof: i32| FieldDescriptorProto {
+        oneof_index: Some(oneof),
+        ..scalar_field(name, number, Type::TYPE_INT32)
+    };
+    assert_rejected_without_mutating_pool(
+        "duplicate-oneof-name.proto",
+        "invalid.test.DuplicateOneof",
+        DescriptorProto {
+            name: Some("DuplicateOneof".into()),
+            field: vec![member("a", 1, 0), member("b", 2, 1)],
+            oneof_decl: vec![
+                OneofDescriptorProto {
+                    name: Some("choice".into()),
+                    ..Default::default()
+                },
+                OneofDescriptorProto {
+                    name: Some("choice".into()),
+                    ..Default::default()
+                },
+            ],
+            ..Default::default()
+        },
+        |err| {
+            assert!(matches!(
+                err,
+                PoolError::DuplicateOneofName { message, name }
+                    if message == "invalid.test.DuplicateOneof" && name == "choice"
+            ));
+            assert_eq!(
+                err.to_string(),
+                "message invalid.test.DuplicateOneof declares oneof name \"choice\" more than once"
+            );
+        },
+    );
+}
+
+#[test]
+fn distinct_oneof_names_are_accepted() {
+    use buffa_descriptor::generated::descriptor::{
+        DescriptorProto, FileDescriptorProto, FileDescriptorSet, OneofDescriptorProto,
+    };
+
+    let mut p = DescriptorPool::decode(FDS_BYTES).unwrap();
+    p.add_file_descriptor_set(FileDescriptorSet {
+        file: vec![FileDescriptorProto {
+            name: Some("distinct-oneof-names.proto".into()),
+            package: Some("valid.test".into()),
+            syntax: Some("proto3".into()),
+            message_type: vec![DescriptorProto {
+                name: Some("DistinctOneofs".into()),
+                oneof_decl: vec![
+                    OneofDescriptorProto {
+                        name: Some("first".into()),
+                        ..Default::default()
+                    },
+                    OneofDescriptorProto {
+                        name: Some("second".into()),
+                        ..Default::default()
+                    },
+                ],
+                ..Default::default()
+            }],
+            ..Default::default()
+        }],
+        ..Default::default()
+    })
+    .unwrap();
+
+    let message = p.message_by_name("valid.test.DistinctOneofs").unwrap();
+    assert_eq!(
+        message
+            .oneofs()
+            .iter()
+            .map(|oneof| oneof.name())
+            .collect::<Vec<_>>(),
+        ["first", "second"]
+    );
+}
+
 #[test]
 fn editions_feature_resolution() {
     let p = pool();