From 3a4f9f36177b78f8987a3dcef96c7104877db62c Mon Sep 17 00:00:00 2001
From: Andrew Hickman <andrew.hickman1@sky.com>
Date: Sat, 1 Mar 2025 13:20:32 +0000
Subject: [PATCH] Fix error messages when Any is missing a value field (#146)

---
 prost-reflect-tests/src/json.rs           | 11 ++++
 prost-reflect/src/dynamic/serde/de/wkt.rs | 65 ++++++++++++-----------
 2 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/prost-reflect-tests/src/json.rs b/prost-reflect-tests/src/json.rs
index 7ffc4a7e..83338f8c 100644
--- a/prost-reflect-tests/src/json.rs
+++ b/prost-reflect-tests/src/json.rs
@@ -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() {
diff --git a/prost-reflect/src/dynamic/serde/de/wkt.rs b/prost-reflect/src/dynamic/serde/de/wkt.rs
index 58edd13c..aa7eab7c 100644
--- a/prost-reflect/src/dynamic/serde/de/wkt.rs
+++ b/prost-reflect/src/dynamic/serde/de/wkt.rs
@@ -2,6 +2,7 @@ use std::{
     borrow::Cow,
     collections::{BTreeMap, HashMap},
     fmt,
+    marker::PhantomData,
 };
 
 use prost::Message;
@@ -46,21 +47,14 @@ impl<'de> Visitor<'de> for GoogleProtobufAnyVisitor<'_> {
     {
         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,
+            "@type",
+            PhantomData::<String>,
+        )?;
 
         let message_name = get_message_name(&type_url).map_err(Error::custom)?;
-
         let message_desc = self
             .0
             .get_message_by_name(message_name)
@@ -71,24 +65,12 @@ impl<'de> Visitor<'de> for GoogleProtobufAnyVisitor<'_> {
                 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,
+                    "value",
+                    MessageSeed(&message_desc, self.1),
+                )?,
             };
 
             if self.1.deny_unknown_fields {
@@ -381,6 +363,27 @@ impl<'de> Visitor<'de> for GoogleProtobufEmptyVisitor {
     }
 }
 
+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> {