Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error messages when Any is missing a value field #146

Merged
merged 1 commit into from
Mar 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions prost-reflect-tests/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,17 @@ fn deserialize_any_wkt() {
);
}

#[test]
#[should_panic(expected = "expected 'value' field")]
fn deserialize_any_wkt_missing_value() {
from_json::<prost_types::Any>(
json!({
"@type": "type.googleapis.com/google.protobuf.Int32Value"
}),
"google.protobuf.Any",
);
}

#[test]
#[should_panic(expected = "unrecognized field name 'unknown'")]
fn deserialize_any_deny_unknown_fields() {
Expand Down
65 changes: 34 additions & 31 deletions prost-reflect/src/dynamic/serde/de/wkt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
borrow::Cow,
collections::{BTreeMap, HashMap},
fmt,
marker::PhantomData,
};

use prost::Message;
Expand Down Expand Up @@ -46,21 +47,14 @@
{
let mut buffered_entries = HashMap::new();

let type_url = loop {
match map.next_key::<Cow<str>>()? {
Some(key) if key == "@type" => {
break map.next_value::<String>()?;
}
Some(key) => {
let value: serde_value::Value = map.next_value()?;
buffered_entries.insert(key, value);
}
None => return Err(Error::custom("expected '@type' field")),
}
};
let type_url = find_field(
&mut map,
&mut buffered_entries,

Check warning on line 52 in prost-reflect/src/dynamic/serde/de/wkt.rs

View check run for this annotation

Codecov / codecov/patch

prost-reflect/src/dynamic/serde/de/wkt.rs#L51-L52

Added lines #L51 - L52 were not covered by tests
"@type",
PhantomData::<String>,

Check warning on line 54 in prost-reflect/src/dynamic/serde/de/wkt.rs

View check run for this annotation

Codecov / codecov/patch

prost-reflect/src/dynamic/serde/de/wkt.rs#L54

Added line #L54 was not covered by tests
)?;

let message_name = get_message_name(&type_url).map_err(Error::custom)?;

let message_desc = self
.0
.get_message_by_name(message_name)
Expand All @@ -71,24 +65,12 @@
Some(value) => {
deserialize_message(&message_desc, value, self.1).map_err(Error::custom)?
}
None => loop {
match map.next_key::<Cow<str>>()? {
Some(key) if key == "value" => {
break map.next_value_seed(MessageSeed(&message_desc, self.1))?
}
Some(key) => {
if self.1.deny_unknown_fields {
return Err(Error::custom(format!(
"unrecognized field name '{}'",
key
)));
} else {
let _ = map.next_value::<IgnoredAny>()?;
}
}
None => return Err(Error::custom("expected '@type' field")),
}
},
None => find_field(
&mut map,
&mut buffered_entries,

Check warning on line 70 in prost-reflect/src/dynamic/serde/de/wkt.rs

View check run for this annotation

Codecov / codecov/patch

prost-reflect/src/dynamic/serde/de/wkt.rs#L69-L70

Added lines #L69 - L70 were not covered by tests
"value",
MessageSeed(&message_desc, self.1),
)?,
};

if self.1.deny_unknown_fields {
Expand Down Expand Up @@ -381,6 +363,27 @@
}
}

fn find_field<'de, A, D>(
map: &mut A,
buffered_entries: &mut HashMap<Cow<str>, serde_value::Value>,
expected: &str,
value_seed: D,
) -> Result<D::Value, A::Error>
where
A: MapAccess<'de>,
D: DeserializeSeed<'de>,
{
loop {
match map.next_key::<Cow<str>>()? {
Some(key) if key == expected => return map.next_value_seed(value_seed),
Some(key) => {
buffered_entries.insert(key, map.next_value()?);
}
None => return Err(Error::custom(format!("expected '{expected}' field"))),
}
}
}

/// Validates the string is a valid RFC3339 timestamp, requiring upper-case
/// 'T' and 'Z' characters as recommended by the conformance tests.
fn validate_strict_rfc3339(v: &str) -> Result<(), String> {
Expand Down