Skip to content

Commit

Permalink
Update text format type url handling to match JSON (#147)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewhickman authored Mar 1, 2025
1 parent 3a4f9f3 commit c91072d
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 60 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- All type name domains are now permitted when deserializing `google.protobuf.Any` types in the JSON format and text format. ([#143], [#147]).

## [0.14.6] - 2025-02-06

### Fixed
Expand Down Expand Up @@ -466,5 +470,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#138]: https://github.com/andrewhickman/prost-reflect/issues/138
[#140]: https://github.com/andrewhickman/prost-reflect/pull/140
[#143]: https://github.com/andrewhickman/prost-reflect/pull/143
[#145]: https://github.com/andrewhickman/prost-reflect/pull/143
[#147]: https://github.com/andrewhickman/prost-reflect/pull/143
[protox#82]: https://github.com/andrewhickman/protox/issues/82
[protox#86]: https://github.com/andrewhickman/protox/issues/86
4 changes: 0 additions & 4 deletions prost-reflect-tests/src/text_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -754,10 +754,6 @@ fn parse_error() {
"extension 'my.ext' not found for message 'test.Scalars'"
);

assert_eq!(
any_error("[example.com/my.message] <>"),
"unknown domain 'example.com' for type url"
);
assert_eq!(
any_error("[type.googleapis.com/namespace.NotFound] {}"),
"message type 'namespace.NotFound' not found"
Expand Down
3 changes: 0 additions & 3 deletions prost-reflect/src/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ use crate::{descriptor::types::FileDescriptorProto, Value};
pub(crate) const MAP_ENTRY_KEY_NUMBER: u32 = 1;
pub(crate) const MAP_ENTRY_VALUE_NUMBER: u32 = 2;

pub(crate) const GOOGLE_APIS_DOMAIN: &str = "type.googleapis.com/";
pub(crate) const GOOGLE_PROD_DOMAIN: &str = "type.googleprod.com/";

pub(crate) const RESERVED_MESSAGE_FIELD_NUMBERS: Range<i32> = 19_000..20_000;
pub(crate) const VALID_MESSAGE_FIELD_NUMBERS: Range<i32> = 1..536_870_912;

Expand Down
40 changes: 40 additions & 0 deletions prost-reflect/src/dynamic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,3 +1216,43 @@ impl<L: Iterator, R: Iterator<Item = L::Item>> Iterator for Either<L, R> {
}
}
}

fn get_type_url_message_name(type_url: &str) -> Result<&str, String> {
let (_type_domain_name, message_name) = type_url
.rsplit_once('/')
.ok_or_else(|| format!("unsupported type url '{type_url}': missing at least one '/'",))?;

Ok(message_name)
}

#[test]
fn test_get_type_url_message_name() {
assert_eq!(
get_type_url_message_name("type.googleapis.com/my.messages.Message"),
Ok("my.messages.Message")
);
assert_eq!(
get_type_url_message_name("type.googleprod.com/my.messages.Message"),
Ok("my.messages.Message")
);
assert_eq!(
get_type_url_message_name("/my.messages.Message"),
Ok("my.messages.Message")
);
assert_eq!(
get_type_url_message_name("any.url.com/my.messages.Message"),
Ok("my.messages.Message")
);
assert_eq!(
get_type_url_message_name("http://even.multiple/slashes/my.messages.Message"),
Ok("my.messages.Message")
);
assert_eq!(
get_type_url_message_name("/any.type.isAlsoValid"),
Ok("any.type.isAlsoValid")
);
assert_eq!(
get_type_url_message_name("my.messages.Message"),
Err("unsupported type url 'my.messages.Message': missing at least one '/'".to_owned())
);
}
31 changes: 2 additions & 29 deletions prost-reflect/src/dynamic/serde/de/wkt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use serde::de::{

use crate::{
dynamic::{
get_type_url_message_name,
serde::{
case::camel_case_to_snake_case, check_duration, check_timestamp, is_well_known_type,
DeserializeOptions,
Expand Down Expand Up @@ -54,7 +55,7 @@ impl<'de> Visitor<'de> for GoogleProtobufAnyVisitor<'_> {
PhantomData::<String>,
)?;

let message_name = get_message_name(&type_url).map_err(Error::custom)?;
let message_name = get_type_url_message_name(&type_url).map_err(Error::custom)?;
let message_desc = self
.0
.get_message_by_name(message_name)
Expand Down Expand Up @@ -463,14 +464,6 @@ fn validate_strict_rfc3339(v: &str) -> Result<(), String> {
Ok(())
}

fn get_message_name(type_url: &str) -> Result<&str, String> {
let (_type_domain_name, message_name) = type_url
.rsplit_once('/')
.ok_or_else(|| format!("unsupported type url '{type_url}': missing at least one '/'",))?;

Ok(message_name)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -514,24 +507,4 @@ mod tests {
case!("2019-03-26 14:00Z" => Err("invalid rfc3339 timestamp: expected 'T' but found ' '"));
case!("2019-03-26T14:00:00." => Err("invalid rfc3339 timestamp: empty fractional seconds"));
}

#[test]
fn test_get_message_name_type_url() {
macro_rules! case {
($s:expr => Ok($e:expr)) => {
assert_eq!(get_message_name($s).unwrap(), $e)
};
($s:expr => Err($e:expr)) => {
assert_eq!(get_message_name($s).unwrap_err(), $e)
};
}

case!("type.googleapis.com/my.messages.Message" => Ok("my.messages.Message"));
case!("type.googleprod.com/my.messages.Message" => Ok("my.messages.Message"));
case!("/my.messages.Message" => Ok("my.messages.Message"));
case!("any.url.com/my.messages.Message" => Ok("my.messages.Message"));
case!("http://even.multiple/slashes/my.messages.Message" => Ok("my.messages.Message"));
case!("/any.type.isAlsoValid" => Ok("any.type.isAlsoValid"));
case!("my.messages.Message" => Err("unsupported type url 'my.messages.Message': missing at least one '/'"));
}
}
8 changes: 2 additions & 6 deletions prost-reflect/src/dynamic/text_format/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ use std::fmt::{self, Write};
use prost::Message;

use crate::{
descriptor::{GOOGLE_APIS_DOMAIN, GOOGLE_PROD_DOMAIN},
dynamic::{
fields::ValueAndDescriptor,
fmt_string,
fmt_string, get_type_url_message_name,
text_format::FormatOptions,
unknown::{UnknownField, UnknownFieldSet, UnknownFieldValue},
},
Expand Down Expand Up @@ -303,10 +302,7 @@ fn as_any(message: &DynamicMessage) -> Option<(String, DynamicMessage)> {
}

let any = message.transcode_to::<prost_types::Any>().ok()?;
let message_name = any
.type_url
.strip_prefix(GOOGLE_APIS_DOMAIN)
.or_else(|| any.type_url.strip_prefix(GOOGLE_PROD_DOMAIN))?;
let message_name = get_type_url_message_name(&any.type_url).ok()?;

let desc = message
.desc
Expand Down
8 changes: 0 additions & 8 deletions prost-reflect/src/dynamic/text_format/parse/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,6 @@ pub(crate) enum ParseErrorKind {
#[cfg_attr(feature = "miette", label("set here"))]
span: Span,
},
UnknownTypeUrlDomain {
domain: String,
#[cfg_attr(feature = "miette", label("used here"))]
span: Span,
},
MessageNotFound {
message_name: String,
#[cfg_attr(feature = "miette", label("used here"))]
Expand Down Expand Up @@ -169,9 +164,6 @@ impl Display for ParseErrorKind {
extension_name, message_name
)
}
ParseErrorKind::UnknownTypeUrlDomain { domain, .. } => {
write!(f, "unknown domain '{}' for type url", domain)
}
ParseErrorKind::MessageNotFound { message_name, .. } => {
write!(f, "message type '{}' not found", message_name)
}
Expand Down
12 changes: 2 additions & 10 deletions prost-reflect/src/dynamic/text_format/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ use self::{
lex::{Int, Token},
};
use crate::{
descriptor::{
GOOGLE_APIS_DOMAIN, GOOGLE_PROD_DOMAIN, MAP_ENTRY_KEY_NUMBER, MAP_ENTRY_VALUE_NUMBER,
},
descriptor::{MAP_ENTRY_KEY_NUMBER, MAP_ENTRY_VALUE_NUMBER},
dynamic::fields::FieldDescriptorLike,
DynamicMessage, EnumDescriptor, FieldDescriptor, Kind, MapKey, MessageDescriptor, Value,
};
Expand Down Expand Up @@ -103,10 +101,7 @@ impl<'a> Parser<'a> {

self.parse_field_value(message, &extension)?;
}
FieldName::Any(domain, message_name)
if domain == GOOGLE_APIS_DOMAIN.trim_end_matches('/')
|| domain == GOOGLE_PROD_DOMAIN.trim_end_matches('/') =>
{
FieldName::Any(domain, message_name) => {
let value_message = match message
.desc
.parent_pool()
Expand All @@ -133,9 +128,6 @@ impl<'a> Parser<'a> {
return Err(ParseErrorKind::InvalidTypeForAny { span });
}
}
FieldName::Any(domain, _) => {
return Err(ParseErrorKind::UnknownTypeUrlDomain { domain, span })
}
}

if matches!(self.peek()?, Some((Token::Comma | Token::Semicolon, _))) {
Expand Down

0 comments on commit c91072d

Please sign in to comment.