From 8cda375ea81f22410e1d715c7f592bafb4eb937f Mon Sep 17 00:00:00 2001 From: Divyansh <2023csb1119@iitrpr.ac.in> Date: Thu, 29 Oct 2026 20:30:59 +0530 Subject: [PATCH] fix(descriptor,codegen): allow MessageSet extension numbers up to i32::MAX Match protoc's relaxed field-number bound for extensions of message_set_wire_format messages in the pool linker and extension codegen. --- ...28-message-set-extension-field-number.yaml | 4 + buffa-codegen/src/extension.rs | 3 +- buffa-codegen/src/impl_message.rs | 86 ++++++++++++++++++- .../src/tests/message_set_extension.rs | 60 +++++++++++++ buffa-codegen/src/tests/mod.rs | 1 + buffa-descriptor/src/pool.rs | 40 +++++++-- .../message_set_extension_field_number.rs | 74 ++++++++++++++++ 7 files changed, 257 insertions(+), 11 deletions(-) create mode 100644 .changes/unreleased/fixed-20261028-message-set-extension-field-number.yaml create mode 100644 buffa-codegen/src/tests/message_set_extension.rs create mode 100644 buffa-descriptor/tests/message_set_extension_field_number.rs diff --git a/.changes/unreleased/fixed-20261028-message-set-extension-field-number.yaml b/.changes/unreleased/fixed-20261028-message-set-extension-field-number.yaml new file mode 100644 index 000000000..09e5b7010 --- /dev/null +++ b/.changes/unreleased/fixed-20261028-message-set-extension-field-number.yaml @@ -0,0 +1,4 @@ +kind: Fixed +body: |- + **MessageSet extensions accept field numbers up to `i32::MAX`** (#702). `DescriptorPool::link_extension` and codegen for `extend` now match protoc's relaxed bound when the extendee has `message_set_wire_format = true`, instead of rejecting numbers above `(1 << 29) - 1`. +time: 2026-10-28T23:20:00+05:30 diff --git a/buffa-codegen/src/extension.rs b/buffa-codegen/src/extension.rs index 526a20e4a..09d9d0e73 100644 --- a/buffa-codegen/src/extension.rs +++ b/buffa-codegen/src/extension.rs @@ -71,7 +71,8 @@ fn generate_one( .name .as_deref() .ok_or(CodeGenError::MissingField("extension.name"))?; - let number = crate::impl_message::validated_field_number(field)?; + let number = + crate::impl_message::validated_extension_field_number(ctx, field, current_package)?; let ty = crate::impl_message::effective_type(ctx, field, features); let extendee = field.extendee.as_deref().unwrap_or(""); // Strip leading dot for both the runtime extendee check (PROTO_FQN is diff --git a/buffa-codegen/src/impl_message.rs b/buffa-codegen/src/impl_message.rs index 82beeb62c..5d0556217 100644 --- a/buffa-codegen/src/impl_message.rs +++ b/buffa-codegen/src/impl_message.rs @@ -17,7 +17,7 @@ use crate::context::CodeGenContext; use crate::generated::descriptor::field_descriptor_proto::{Label, Type}; -use crate::generated::descriptor::{DescriptorProto, FieldDescriptorProto}; +use crate::generated::descriptor::{DescriptorProto, FieldDescriptorProto, FileDescriptorProto}; use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote}; @@ -30,17 +30,99 @@ use buffa::encoding::MAX_FIELD_NUMBER; /// /// Protobuf field numbers must be in `[1, MAX_FIELD_NUMBER]`. pub(crate) fn validated_field_number(field: &FieldDescriptorProto) -> Result { + validated_field_number_with_upper(field, MAX_FIELD_NUMBER) +} + +fn validated_field_number_with_upper( + field: &FieldDescriptorProto, + upper: u32, +) -> Result { let n = field .number .ok_or(CodeGenError::MissingField("field.number"))?; // FieldDescriptorProto.number is int32 in the descriptor schema, hence // the cast for the range check (2^29 − 1 fits comfortably in i32). - if !(1..=MAX_FIELD_NUMBER as i32).contains(&n) { + if !(1..=upper as i32).contains(&n) { return Err(CodeGenError::Other(format!("invalid field number: {n}"))); } Ok(n as u32) } +/// Validate an extension field number, using protoc's relaxed `i32::MAX` bound +/// when the extendee is a MessageSet (`message_set_wire_format = true`). +pub(crate) fn validated_extension_field_number( + ctx: &CodeGenContext, + field: &FieldDescriptorProto, + current_package: &str, +) -> Result { + let upper = if extension_extendee_is_message_set(ctx.files, field, current_package)? { + i32::MAX as u32 + } else { + MAX_FIELD_NUMBER + }; + validated_field_number_with_upper(field, upper) +} + +fn extension_extendee_is_message_set( + files: &[FileDescriptorProto], + field: &FieldDescriptorProto, + current_package: &str, +) -> Result { + let Some(extendee) = field.extendee.as_deref() else { + return Ok(false); + }; + let fqn = resolve_proto_type_fqn(extendee, current_package); + let Some(msg) = find_message_by_fqn(files, &fqn) else { + return Ok(false); + }; + Ok(msg + .options + .as_option() + .and_then(|o| o.message_set_wire_format) + .unwrap_or(false)) +} + +fn resolve_proto_type_fqn(type_name: &str, current_package: &str) -> String { + let lookup = type_name.strip_prefix('.').unwrap_or(type_name); + if lookup.contains('.') || current_package.is_empty() { + lookup.to_string() + } else { + format!("{current_package}.{lookup}") + } +} + +fn find_message_by_fqn<'a>( + files: &'a [FileDescriptorProto], + fqn: &str, +) -> Option<&'a DescriptorProto> { + files.iter().find_map(|file| { + let pkg = file.package.as_deref().unwrap_or(""); + find_message_in_tree(&file.message_type, pkg, fqn) + }) +} + +fn find_message_in_tree<'a>( + messages: &'a [DescriptorProto], + scope: &str, + fqn: &str, +) -> Option<&'a DescriptorProto> { + for msg in messages { + let name = msg.name.as_deref().unwrap_or(""); + let msg_fqn = if scope.is_empty() { + name.to_string() + } else { + format!("{scope}.{name}") + }; + if msg_fqn == fqn { + return Some(msg); + } + if let Some(found) = find_message_in_tree(&msg.nested_type, &msg_fqn, fqn) { + return Some(found); + } + } + None +} + /// Returns `true` when a non-message field has *explicit* field presence. A /// singular field for which this holds is encoded as `Option`. /// diff --git a/buffa-codegen/src/tests/message_set_extension.rs b/buffa-codegen/src/tests/message_set_extension.rs new file mode 100644 index 000000000..0eb5aa2c8 --- /dev/null +++ b/buffa-codegen/src/tests/message_set_extension.rs @@ -0,0 +1,60 @@ +//! MessageSet extensions may use field numbers above `(1 << 29) - 1`. + +use super::*; +use crate::generated::descriptor::descriptor_proto::ExtensionRange; +use crate::generated::descriptor::field_descriptor_proto::{Label, Type}; +use crate::generated::descriptor::{DescriptorProto, FieldDescriptorProto, MessageOptions}; + +const TYPE_ID: i32 = 1_000_000_000; + +fn message_set_extension_file() -> FileDescriptorProto { + let mut file = proto3_file("message-set-ext.proto"); + file.package = Some("mset.codegen".into()); + file.syntax = Some("proto2".into()); + file.message_type = vec![ + DescriptorProto { + name: Some("Set".into()), + extension_range: vec![ExtensionRange { + start: Some(4), + end: Some(i32::MAX), + ..Default::default() + }], + options: MessageOptions { + message_set_wire_format: Some(true), + ..Default::default() + } + .into(), + ..Default::default() + }, + DescriptorProto { + name: Some("Item".into()), + ..Default::default() + }, + ]; + file.extension = vec![FieldDescriptorProto { + name: Some("ext".into()), + number: Some(TYPE_ID), + label: Some(Label::LABEL_OPTIONAL), + r#type: Some(Type::TYPE_MESSAGE), + type_name: Some(".mset.codegen.Item".into()), + extendee: Some(".mset.codegen.Set".into()), + ..Default::default() + }]; + file +} + +#[test] +fn message_set_extension_const_uses_large_field_number() { + let file = message_set_extension_file(); + let config = CodeGenConfig { + allow_message_set: true, + ..Default::default() + }; + let files = generate(&[file], &["message-set-ext.proto".into()], &config) + .expect("MessageSet extension should generate"); + let content = joined(&files); + assert!( + content.contains(&format!("::buffa::Extension::new({TYPE_ID}")), + "extension const must use the MessageSet type id: {content}" + ); +} diff --git a/buffa-codegen/src/tests/mod.rs b/buffa-codegen/src/tests/mod.rs index f56e3bb7d..e53742765 100644 --- a/buffa-codegen/src/tests/mod.rs +++ b/buffa-codegen/src/tests/mod.rs @@ -65,6 +65,7 @@ mod json_codegen; mod keyword_escaped_fields; mod lifetime_anchor; mod map_type; +mod message_set_extension; mod naming; mod owned_view_codegen; mod pointer_repr; diff --git a/buffa-descriptor/src/pool.rs b/buffa-descriptor/src/pool.rs index 3990d97cb..e3a36cc9d 100644 --- a/buffa-descriptor/src/pool.rs +++ b/buffa-descriptor/src/pool.rs @@ -2281,7 +2281,14 @@ impl DescriptorPool { }); } } - let fd = self.link_field(&fqn, f, &msg_features, Some(msg), scope)?; + let fd = self.link_field( + &fqn, + f, + &msg_features, + Some(msg), + scope, + buffa::encoding::MAX_FIELD_NUMBER, + )?; if reserved_names.contains(fd.name.as_str()) { return Err(PoolError::ReservedMessageFieldName { message: fqn.clone(), @@ -2707,9 +2714,25 @@ impl DescriptorPool { extendee: extendee_name.clone(), }); } + let extendee_message = &self.messages[extendee.0 as usize]; // The field links exactly like a declared field. `containing_msg` is // `None` because an extension never links as a map field. - let mut field = self.link_field(scope_fqn, ext, parent_features, None, scope)?; + let max_field_number = if extendee_message + .options() + .is_some_and(|options| options.message_set_wire_format == Some(true)) + { + i32::MAX as u32 + } else { + buffa::encoding::MAX_FIELD_NUMBER + }; + let mut field = self.link_field( + scope_fqn, + ext, + parent_features, + None, + scope, + max_field_number, + )?; // An extension whose type is a map entry does not link. protoc accepts // one shape, a repeated extension of the entry's containing message // that is named for the entry, and treats it as a map field. The pool @@ -2739,7 +2762,6 @@ impl DescriptorPool { // there is exactly one valid interpretation of an extension's oneof // membership, and it is "none". field.oneof_index = None; - let extendee_message = &self.messages[extendee.0 as usize]; if extendee_message .options() .is_some_and(|options| options.message_set_wire_format == Some(true)) @@ -2850,6 +2872,7 @@ impl DescriptorPool { parent_features: &ResolvedFeatures, containing_msg: Option<&DescriptorProto>, scope: LinkScope<'_>, + max_field_number: u32, ) -> Result { let name = f.name.clone().unwrap_or_default(); let field_fqn = if msg_fqn.is_empty() { @@ -3074,14 +3097,15 @@ impl DescriptorPool { }); } - // Validate the field number. The wire format reserves 0, and the - // upper bound is `(1 << 29) - 1`. Spec-compliant `protoc` never emits - // an invalid number, but the input is no longer trusted to come from - // `protoc` once consumers feed network-loaded descriptors. + // Validate the field number. The wire format reserves 0. The upper + // bound is `(1 << 29) - 1` for ordinary fields; MessageSet extensions + // use `type_id` values up to `i32::MAX` (see `link_extension`). Spec- + // compliant `protoc` never emits an invalid number, but the input is + // no longer trusted once consumers feed network-loaded descriptors. let raw_number = f.number.unwrap_or(0); let number = u32::try_from(raw_number) .ok() - .filter(|&n| (1..=buffa::encoding::MAX_FIELD_NUMBER).contains(&n)) + .filter(|&n| (1..=max_field_number).contains(&n)) .ok_or_else(|| PoolError::InvalidFieldNumber { field: field_fqn.clone(), number: raw_number, diff --git a/buffa-descriptor/tests/message_set_extension_field_number.rs b/buffa-descriptor/tests/message_set_extension_field_number.rs new file mode 100644 index 000000000..90c9c7b22 --- /dev/null +++ b/buffa-descriptor/tests/message_set_extension_field_number.rs @@ -0,0 +1,74 @@ +#![cfg(feature = "reflect")] + +use buffa::Message; +use buffa_descriptor::generated::descriptor::descriptor_proto::ExtensionRange; +use buffa_descriptor::generated::descriptor::field_descriptor_proto::{Label, Type}; +use buffa_descriptor::generated::descriptor::{ + DescriptorProto, FieldDescriptorProto, FileDescriptorProto, FileDescriptorSet, MessageOptions, +}; +use buffa_descriptor::DescriptorPool; + +const TYPE_ID: i32 = 1_000_000_000; + +fn message_set_with_extension() -> FileDescriptorSet { + FileDescriptorSet { + file: vec![FileDescriptorProto { + name: Some("message-set-ext.proto".into()), + package: Some("mset.test".into()), + syntax: Some("proto2".into()), + message_type: vec![ + DescriptorProto { + name: Some("Set".into()), + extension_range: vec![ExtensionRange { + start: Some(4), + end: Some(i32::MAX), + ..Default::default() + }], + options: MessageOptions { + message_set_wire_format: Some(true), + ..Default::default() + } + .into(), + ..Default::default() + }, + DescriptorProto { + name: Some("Item".into()), + field: vec![FieldDescriptorProto { + name: Some("payload".into()), + number: Some(1), + label: Some(Label::LABEL_OPTIONAL), + r#type: Some(Type::TYPE_STRING), + ..Default::default() + }], + ..Default::default() + }, + ], + extension: vec![FieldDescriptorProto { + name: Some("ext".into()), + number: Some(TYPE_ID), + label: Some(Label::LABEL_OPTIONAL), + r#type: Some(Type::TYPE_MESSAGE), + type_name: Some(".mset.test.Item".into()), + extendee: Some(".mset.test.Set".into()), + ..Default::default() + }], + ..Default::default() + }], + ..Default::default() + } +} + +#[test] +fn message_set_extension_above_max_field_number_links() { + let set = message_set_with_extension(); + for pool in [ + DescriptorPool::new(set.clone()).unwrap(), + DescriptorPool::decode(&set.encode_to_vec()).unwrap(), + ] { + let ext = pool + .extension_by_name("mset.test.ext") + .expect("extension should link"); + assert_eq!(ext.field().number(), TYPE_ID as u32); + assert_eq!(pool.message(ext.extendee()).full_name(), "mset.test.Set"); + } +}