diff --git a/.changes/unreleased/fixed-20261003-211406.yaml b/.changes/unreleased/fixed-20261003-211406.yaml new file mode 100644 index 00000000..451907c5 --- /dev/null +++ b/.changes/unreleased/fixed-20261003-211406.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: Reject reflective Any type URLs without a slash-delimited message name when unpacking or converting to and from JSON. +time: 2026-10-03T21:14:06.552245+02:00 diff --git a/buffa-descriptor/src/reflect/dynamic.rs b/buffa-descriptor/src/reflect/dynamic.rs index edee5ce6..82c995ce 100644 --- a/buffa-descriptor/src/reflect/dynamic.rs +++ b/buffa-descriptor/src/reflect/dynamic.rs @@ -1519,8 +1519,8 @@ impl DynamicMessage { /// /// - [`AnyError::NotAny`] if this message is not a `google.protobuf.Any`. /// - [`AnyError::MissingTypeUrl`] if the `Any` has no `type_url`. - /// - [`AnyError::UnknownType`] if the `type_url` names a type the pool - /// doesn't carry. + /// - [`AnyError::UnknownType`] if the `type_url` has no non-empty name + /// after a `/`, or names a type the pool doesn't carry. /// - [`AnyError::Decode`] if the wrapped bytes are malformed. Note that /// an `Any` whose `value` is absent or empty is **not** an error: it /// decodes to a default-valued message of the resolved type, per the @@ -1541,13 +1541,9 @@ impl DynamicMessage { Some(Value::Bytes(b)) => b, _ => &[], }; - let type_name = type_url.rsplit('/').next().unwrap_or(type_url); - let idx = self - .pool - .message_index(type_name) - .ok_or_else(|| AnyError::UnknownType { - type_url: type_url.to_owned(), - })?; + let idx = resolve_any_type(&self.pool, type_url).ok_or_else(|| AnyError::UnknownType { + type_url: type_url.to_owned(), + })?; DynamicMessage::decode(Arc::clone(&self.pool), idx, value).map_err(|source| { AnyError::Decode { type_url: type_url.to_owned(), @@ -1633,6 +1629,15 @@ impl DynamicMessage { /// The fully-qualified name of `google.protobuf.Any`. const ANY_FULL_NAME: &str = "google.protobuf.Any"; +/// Resolve an Any URL whose last `/` is followed by a non-empty type name. +pub(super) fn resolve_any_type(pool: &DescriptorPool, type_url: &str) -> Option { + let (_, name) = type_url.rsplit_once('/')?; + if name.is_empty() { + return None; + } + pool.message_index(name) +} + /// An error from [`DynamicMessage::unpack_any`] or /// [`DynamicMessage::pack_any`]. #[derive(Clone, Debug)] @@ -1649,7 +1654,8 @@ pub enum AnyError { AnyNotRegistered, /// The `Any` has no `type_url` (field 1 absent, empty, or not a string). MissingTypeUrl, - /// The `Any`'s `type_url` names a type the pool does not carry. + /// The `Any`'s `type_url` has no non-empty name after a `/`, or names a + /// type the pool does not carry. UnknownType { /// The unresolvable type URL. type_url: String, diff --git a/buffa-descriptor/src/reflect/json_wkt.rs b/buffa-descriptor/src/reflect/json_wkt.rs index f1b9f9bc..8a8c5219 100644 --- a/buffa-descriptor/src/reflect/json_wkt.rs +++ b/buffa-descriptor/src/reflect/json_wkt.rs @@ -505,7 +505,7 @@ fn serialize_any( // Empty Any → empty object. return s.serialize_map(Some(0))?.end(); } - let Some(inner_idx) = resolve_any_type(pool, type_url) else { + let Some(inner_idx) = super::dynamic::resolve_any_type(pool, type_url) else { return Err(S::Error::custom(format!( "Any type_url {type_url:?} not registered in the descriptor pool" ))); @@ -577,7 +577,7 @@ fn deserialize_any<'de, D: Deserializer<'de>>( let Some(serde_json::Value::String(type_url)) = obj.remove("@type") else { return Err(D::Error::custom("Any object missing string \"@type\"")); }; - let Some(inner_idx) = resolve_any_type(&pool, &type_url) else { + let Some(inner_idx) = super::dynamic::resolve_any_type(&pool, &type_url) else { return Err(D::Error::custom(format!( "Any type_url {type_url:?} not registered in the descriptor pool" ))); @@ -650,14 +650,6 @@ fn deserialize_any<'de, D: Deserializer<'de>>( )) } -/// Resolve a `type_url` to a [`MessageIndex`]. Accepts `type.googleapis.com/` -/// and any other prefix; the type name is the segment after the last `/`. -fn resolve_any_type(pool: &DescriptorPool, type_url: &str) -> Option { - let name = type_url.rsplit('/').next()?; - pool.message_index(name) -} - - // ── Timestamp / Duration / FieldMask formatting ───────────────────────────── // // The shared formatting and parsing primitives live in diff --git a/buffa-descriptor/tests/options_any_symbol_e2e.rs b/buffa-descriptor/tests/options_any_symbol_e2e.rs index 63eb6123..06e9c41b 100644 --- a/buffa-descriptor/tests/options_any_symbol_e2e.rs +++ b/buffa-descriptor/tests/options_any_symbol_e2e.rs @@ -132,6 +132,80 @@ fn any_json_spreads_payload_extensions() { assert_eq!(parsed.unpack_any().unwrap(), opts); } +#[cfg(feature = "json")] +#[test] +fn empty_any_json_preserves_empty_object() { + let p = pool(); + let any_idx = p.message_index("google.protobuf.Any").unwrap(); + let any = DynamicMessage::new(Arc::clone(&p), any_idx); + assert_eq!(any.to_json().unwrap(), "{}"); + for input in ["{}", r#"{"@type":""}"#] { + let strict = DynamicMessage::from_json(Arc::clone(&p), any_idx, input); + let lenient = DynamicMessage::from_json_ignoring_unknown(Arc::clone(&p), any_idx, input); + if input == "{}" { + assert_eq!(strict.unwrap(), any); + assert_eq!(lenient.unwrap(), any); + } else { + assert!(strict.is_err()); + assert!(lenient.is_err()); + } + } +} + +#[cfg(feature = "json")] +#[test] +fn any_json_requires_a_slash_and_non_empty_type_name() { + let p = pool(); + let any_idx = p.message_index("google.protobuf.Any").unwrap(); + + for type_url in [ + "type.googleapis.com/reflect.opt.Annotated", + "company.example/v1/reflect.opt.Annotated", + "/reflect.opt.Annotated", + ] { + let input = format!(r#"{{"@type":"{type_url}"}}"#); + let parsed = DynamicMessage::from_json(Arc::clone(&p), any_idx, &input) + .unwrap_or_else(|err| panic!("{type_url}: {err}")); + assert_eq!(parsed.to_json().unwrap(), input); + assert_eq!( + DynamicMessage::from_json_ignoring_unknown(Arc::clone(&p), any_idx, &input) + .unwrap() + .to_json() + .unwrap(), + input + ); + } + + for type_url in [ + "reflect.opt.Annotated", + "type.googleapis.com/", + "company.example/v1/reflect.opt.Annotated/", + "/", + "type.googleapis.com/no.Such", + ] { + let input = format!(r#"{{"@type":"{type_url}"}}"#); + assert!( + DynamicMessage::from_json(Arc::clone(&p), any_idx, &input).is_err(), + "unexpectedly parsed {type_url:?}" + ); + + assert!( + DynamicMessage::from_json_ignoring_unknown(Arc::clone(&p), any_idx, &input).is_err(), + "unexpectedly parsed {type_url:?} in lenient mode" + ); + + let mut any = DynamicMessage::new(Arc::clone(&p), any_idx); + any.set( + p.message(any_idx).field(1).unwrap(), + Value::String(type_url.to_owned()), + ); + assert!( + any.to_json().is_err(), + "unexpectedly serialized {type_url:?}" + ); + } +} + /// Read a custom option off a re-encoded options message: decode it as a /// `DynamicMessage` of `options_type` and pull the extension's value. This /// is the documented generic flow for reading a custom option by name when @@ -224,6 +298,44 @@ fn any_pack_unpack_round_trip() { assert_eq!(back, ann); } +#[test] +fn any_unpack_requires_a_slash_and_non_empty_type_name() { + let p = pool(); + let any_idx = p.message_index("google.protobuf.Any").unwrap(); + let any_md = p.message(any_idx); + + for type_url in [ + "type.googleapis.com/reflect.opt.Annotated", + "company.example/v1/reflect.opt.Annotated", + "/reflect.opt.Annotated", + ] { + let mut any = DynamicMessage::new(Arc::clone(&p), any_idx); + any.set(any_md.field(1).unwrap(), Value::String(type_url.to_owned())); + let unpacked = any + .unpack_any() + .unwrap_or_else(|err| panic!("{type_url}: {err}")); + assert_eq!( + unpacked.message_descriptor().full_name(), + "reflect.opt.Annotated" + ); + } + + for type_url in [ + "reflect.opt.Annotated", + "type.googleapis.com/", + "company.example/v1/reflect.opt.Annotated/", + "/", + "type.googleapis.com/no.Such", + ] { + let mut any = DynamicMessage::new(Arc::clone(&p), any_idx); + any.set(any_md.field(1).unwrap(), Value::String(type_url.to_owned())); + match any.unpack_any().unwrap_err() { + AnyError::UnknownType { type_url: actual } => assert_eq!(actual, type_url), + other => panic!("unexpected error for {type_url:?}: {other}"), + } + } +} + #[test] fn any_errors() { let p = pool();