From 078a7b8a185cb397b25af79c96e7ffe213aec1da Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 3 Sep 2026 13:39:21 +0200 Subject: [PATCH 1/2] descriptor: reject empty enums --- .../unreleased/fixed-20260903-134500.yaml | 4 +++ buffa-descriptor/src/pool.rs | 22 +++++++++---- buffa-descriptor/tests/pool_e2e.rs | 33 +++++++++++++++++++ 3 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 .changes/unreleased/fixed-20260903-134500.yaml diff --git a/.changes/unreleased/fixed-20260903-134500.yaml b/.changes/unreleased/fixed-20260903-134500.yaml new file mode 100644 index 00000000..4444ca34 --- /dev/null +++ b/.changes/unreleased/fixed-20260903-134500.yaml @@ -0,0 +1,4 @@ +kind: Fixed +body: |- + **`DescriptorPool` rejects empty enum declarations** as `protoc` does. Only hand-built or synthesized descriptor sets can reach this state; the rejection is the new `PoolError::EmptyEnum` variant. +time: 2026-09-03T13:45:00+02:00 diff --git a/buffa-descriptor/src/pool.rs b/buffa-descriptor/src/pool.rs index c97d19d1..3e4bde08 100644 --- a/buffa-descriptor/src/pool.rs +++ b/buffa-descriptor/src/pool.rs @@ -225,6 +225,8 @@ pub enum PoolError { name: String, number: i32, }, + /// An enum declares no values. + EmptyEnum { enum_name: String }, } impl core::fmt::Display for PoolError { @@ -345,6 +347,9 @@ impl core::fmt::Display for PoolError { f, "enum {enum_name} value {name:?} reuses number {number} without allow_alias" ), + Self::EmptyEnum { enum_name } => { + write!(f, "enum {enum_name} declares no values") + } } } } @@ -434,10 +439,10 @@ impl DescriptorPool { /// field identity is declared twice, a field number is out of range or in /// the implementation-reserved band (19000-19999), a field uses a name or /// number its message reserved, an extension range overlaps a reserved - /// range, an open enum's first value is non-zero, an enum value reuses a - /// reserved name or number or a duplicate number without `allow_alias`, a - /// oneof index is invalid, a message exceeds 65 535 fields, or a map entry - /// is malformed. + /// range, an enum declares no values, an open enum's first value is + /// non-zero, an enum value reuses a reserved name or number or a duplicate + /// number without `allow_alias`, a oneof index is invalid, a message + /// exceeds 65 535 fields, or a map entry is malformed. pub fn new(set: FileDescriptorSet) -> Result { let mut pool = Self::default(); pool.add_file_descriptor_set(set)?; @@ -458,9 +463,9 @@ impl DescriptorPool { /// validation failure (dangling type names, out-of-range or /// implementation-reserved field numbers, reserved message fields, an /// overlapping extension range, duplicate symbols or field identities, - /// an open enum whose first value is non-zero, reserved enum values, - /// duplicate enum numbers without `allow_alias`, invalid oneof indices, - /// or malformed map entries). + /// an empty enum, an open enum whose first value is non-zero, reserved enum + /// values, duplicate enum numbers without `allow_alias`, invalid oneof + /// indices, or malformed map entries). /// /// A large descriptor set can exceed the default element-memory bound — /// the descriptor types are wide structs, so the element footprint runs @@ -1223,6 +1228,9 @@ impl DescriptorPool { format!("{parent_fqn}.{name}") }; let enum_features = features::resolve_child(parent_features, features::enum_features(e)); + if e.value.is_empty() { + return Err(PoolError::EmptyEnum { enum_name: fqn }); + } if enum_features.enum_type == EnumType::Open { if let Some(first) = e.value.first() { let number = first.number.unwrap_or(0); diff --git a/buffa-descriptor/tests/pool_e2e.rs b/buffa-descriptor/tests/pool_e2e.rs index 4f4b4906..c16a7514 100644 --- a/buffa-descriptor/tests/pool_e2e.rs +++ b/buffa-descriptor/tests/pool_e2e.rs @@ -462,6 +462,39 @@ fn open_enum_with_no_values_is_not_rejected_by_the_first_value_rule() { ); } +#[test] +fn empty_enums_are_rejected_transactionally() { + use buffa_descriptor::generated::descriptor::{ + EnumDescriptorProto, FileDescriptorProto, FileDescriptorSet, + }; + + let set = FileDescriptorSet { + file: vec![FileDescriptorProto { + name: Some("empty-enum.proto".into()), + package: Some("invalid.test".into()), + syntax: Some("proto3".into()), + enum_type: vec![EnumDescriptorProto { + name: Some("Empty".into()), + ..Default::default() + }], + ..Default::default() + }], + ..Default::default() + }; + + assert_set_rejected_without_mutating_pool( + "empty-enum.proto", + "invalid.test.Empty", + set, + |err| { + assert!(matches!( + err, + PoolError::EmptyEnum { enum_name } if enum_name == "invalid.test.Empty" + )); + }, + ); +} + #[test] fn oneof_links() { let p = pool(); From c0363531f17668e71e453828fbccc97a0ca86ce0 Mon Sep 17 00:00:00 2001 From: Iain McGinniss <309153+iainmcgin@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:31:13 -0700 Subject: [PATCH 2/2] descriptor: assert EmptyEnum in the empty-open-enum test; cite the PR in the fragment The test guarding the first-value rule against an empty enum became vacuous once the pool rejects empty enums outright; it now asserts the EmptyEnum error (and still that the first-value rule is not what fires). Merged main; the DescriptorPool::new/decode error summaries keep main's wording pending a single rewrite after this series. --- .changes/unreleased/fixed-20260903-134500.yaml | 2 +- buffa-descriptor/tests/pool_e2e.rs | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.changes/unreleased/fixed-20260903-134500.yaml b/.changes/unreleased/fixed-20260903-134500.yaml index 4444ca34..5eba058c 100644 --- a/.changes/unreleased/fixed-20260903-134500.yaml +++ b/.changes/unreleased/fixed-20260903-134500.yaml @@ -1,4 +1,4 @@ kind: Fixed body: |- - **`DescriptorPool` rejects empty enum declarations** as `protoc` does. Only hand-built or synthesized descriptor sets can reach this state; the rejection is the new `PoolError::EmptyEnum` variant. + **`DescriptorPool` rejects empty enum declarations** (#421), as `protoc` does ("Enums must contain at least one value"). Only hand-built or synthesized descriptor sets can reach this state; the rejection is the new `PoolError::EmptyEnum` variant. time: 2026-09-03T13:45:00+02:00 diff --git a/buffa-descriptor/tests/pool_e2e.rs b/buffa-descriptor/tests/pool_e2e.rs index eaaf25e2..c1381cef 100644 --- a/buffa-descriptor/tests/pool_e2e.rs +++ b/buffa-descriptor/tests/pool_e2e.rs @@ -444,13 +444,14 @@ fn proto2_enum_first_value_can_be_nonzero() { } #[test] -fn open_enum_with_no_values_is_not_rejected_by_the_first_value_rule() { +fn empty_open_enum_reports_empty_enum_not_the_first_value_rule() { use buffa_descriptor::generated::descriptor::{ EnumDescriptorProto, FileDescriptorProto, FileDescriptorSet, }; - // protoc rejects an empty enum for a different reason; this rule must - // not panic or misfire on `value.first()` being `None`. + // An empty enum is rejected as `EmptyEnum` (checked first, as protoc + // does); the open-enum first-value rule must not be the one that fires, + // nor panic on `value.first()` being `None`. let result = DescriptorPool::new(FileDescriptorSet { file: vec![FileDescriptorProto { name: Some("proto3-empty-enum.proto".into()), @@ -465,9 +466,9 @@ fn open_enum_with_no_values_is_not_rejected_by_the_first_value_rule() { ..Default::default() }); assert!( - !matches!( - result, - Err(buffa_descriptor::PoolError::OpenEnumFirstValueNotZero { .. }) + matches!( + &result, + Err(buffa_descriptor::PoolError::EmptyEnum { enum_name }) if enum_name == "valid.test.Empty" ), "{result:?}" );