Skip to content

Commit e02efbf

Browse files
authored
fix(model): decode float fields through serde_json::Number (#1300)
* fix(model): decode float fields through serde_json::Number When serde_json's arbitrary_precision feature is enabled anywhere in a build, serde replays a buffered decimal as serde_json's private number map, and a plain f32 or f64 field rejects it. Read the model's float fields through serde_json::Number, which accepts both forms. rmcp's dev-dependencies enable the feature, so the tests run with it. Refs openai/codex#38979 * docs(model): shorten the json_float module comment
1 parent 972af86 commit e02efbf

6 files changed

Lines changed: 371 additions & 9 deletions

File tree

‎crates/rmcp/Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,9 @@ tracing-subscriber = { version = "0.3", features = [
216216
] }
217217
async-trait = "0.1"
218218
rstest = "0.27.0"
219+
# Cargo unifies this feature into rmcp whenever any crate in a build enables it,
220+
# so the tests run with it on (see tests/test_deserialization.rs).
221+
serde_json = { version = "1.0", features = ["arbitrary_precision"] }
219222
[[test]]
220223
name = "test_tool_macros"
221224
required-features = ["server", "client"]

‎crates/rmcp/src/model.rs‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,9 +1633,14 @@ const_string!(ProgressNotificationMethod = "notifications/progress");
16331633
pub struct ProgressNotificationParam {
16341634
pub progress_token: ProgressToken,
16351635
/// The progress thus far. This should increase every time progress is made, even if the total is unknown.
1636+
#[serde(deserialize_with = "serde_impl::json_float::f64")]
16361637
pub progress: f64,
16371638
/// Total number of items to process (or total progress required), if known
1638-
#[serde(skip_serializing_if = "Option::is_none")]
1639+
#[serde(
1640+
default,
1641+
deserialize_with = "serde_impl::json_float::option_f64",
1642+
skip_serializing_if = "Option::is_none"
1643+
)]
16391644
pub total: Option<f64>,
16401645
/// An optional message describing the current progress.
16411646
#[serde(skip_serializing_if = "Option::is_none")]
@@ -3036,7 +3041,11 @@ pub struct CreateMessageRequestParams {
30363041
#[serde(skip_serializing_if = "Option::is_none")]
30373042
pub include_context: Option<ContextInclusion>,
30383043
/// Temperature for controlling randomness (0.0 to 1.0)
3039-
#[serde(skip_serializing_if = "Option::is_none")]
3044+
#[serde(
3045+
default,
3046+
deserialize_with = "serde_impl::json_float::option_f32",
3047+
skip_serializing_if = "Option::is_none"
3048+
)]
30403049
pub temperature: Option<f32>,
30413050
/// Maximum number of tokens to generate
30423051
pub max_tokens: u32,
@@ -3225,13 +3234,25 @@ pub struct ModelPreferences {
32253234
#[serde(skip_serializing_if = "Option::is_none")]
32263235
pub hints: Option<Vec<ModelHint>>,
32273236
/// Priority for cost optimization (0.0 to 1.0, higher = prefer cheaper models)
3228-
#[serde(skip_serializing_if = "Option::is_none")]
3237+
#[serde(
3238+
default,
3239+
deserialize_with = "serde_impl::json_float::option_f32",
3240+
skip_serializing_if = "Option::is_none"
3241+
)]
32293242
pub cost_priority: Option<f32>,
32303243
/// Priority for speed/latency (0.0 to 1.0, higher = prefer faster models)
3231-
#[serde(skip_serializing_if = "Option::is_none")]
3244+
#[serde(
3245+
default,
3246+
deserialize_with = "serde_impl::json_float::option_f32",
3247+
skip_serializing_if = "Option::is_none"
3248+
)]
32323249
pub speed_priority: Option<f32>,
32333250
/// Priority for intelligence/capability (0.0 to 1.0, higher = prefer more capable models)
3234-
#[serde(skip_serializing_if = "Option::is_none")]
3251+
#[serde(
3252+
default,
3253+
deserialize_with = "serde_impl::json_float::option_f32",
3254+
skip_serializing_if = "Option::is_none"
3255+
)]
32353256
pub intelligence_priority: Option<f32>,
32363257
}
32373258

‎crates/rmcp/src/model/annotated.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ use super::Role;
1717
pub struct Annotations {
1818
#[serde(skip_serializing_if = "Option::is_none")]
1919
pub audience: Option<Vec<Role>>,
20-
#[serde(skip_serializing_if = "Option::is_none")]
20+
#[serde(
21+
default,
22+
deserialize_with = "super::serde_impl::json_float::option_f32",
23+
skip_serializing_if = "Option::is_none"
24+
)]
2125
pub priority: Option<f32>,
2226
#[serde(skip_serializing_if = "Option::is_none", rename = "lastModified")]
2327
pub last_modified: Option<String>,

‎crates/rmcp/src/model/elicitation_schema.rs‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,15 +256,27 @@ pub struct NumberSchema {
256256
pub description: Option<Cow<'static, str>>,
257257

258258
/// Minimum value (inclusive)
259-
#[serde(skip_serializing_if = "Option::is_none")]
259+
#[serde(
260+
default,
261+
deserialize_with = "super::serde_impl::json_float::option_f64",
262+
skip_serializing_if = "Option::is_none"
263+
)]
260264
pub minimum: Option<f64>,
261265

262266
/// Maximum value (inclusive)
263-
#[serde(skip_serializing_if = "Option::is_none")]
267+
#[serde(
268+
default,
269+
deserialize_with = "super::serde_impl::json_float::option_f64",
270+
skip_serializing_if = "Option::is_none"
271+
)]
264272
pub maximum: Option<f64>,
265273

266274
/// Default value
267-
#[serde(skip_serializing_if = "Option::is_none")]
275+
#[serde(
276+
default,
277+
deserialize_with = "super::serde_impl::json_float::option_f64",
278+
skip_serializing_if = "Option::is_none"
279+
)]
268280
pub default: Option<f64>,
269281
}
270282

‎crates/rmcp/src/model/serde_impl.rs‎

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,41 @@ use super::{
88
RequestOptionalParam,
99
};
1010

11+
/// Float deserializers that also accept the number map serde_json's
12+
/// `arbitrary_precision` feature produces for buffered (untagged/flattened) values.
13+
/// Like JSON itself, they reject NaN and infinities.
14+
pub(crate) mod json_float {
15+
use serde::{Deserialize, Deserializer, de::Error};
16+
use serde_json::Number;
17+
18+
fn to_f64<E: Error>(number: Number) -> Result<f64, E> {
19+
// `None` only for a value beyond `f64`, which serde_json also rejects
20+
// without the feature.
21+
number
22+
.as_f64()
23+
.ok_or_else(|| E::custom(format_args!("number out of range: {number}")))
24+
}
25+
26+
pub(crate) fn f64<'de, D: Deserializer<'de>>(deserializer: D) -> Result<f64, D::Error> {
27+
to_f64(Number::deserialize(deserializer)?)
28+
}
29+
30+
pub(crate) fn option_f64<'de, D: Deserializer<'de>>(
31+
deserializer: D,
32+
) -> Result<Option<f64>, D::Error> {
33+
Option::<Number>::deserialize(deserializer)?
34+
.map(to_f64)
35+
.transpose()
36+
}
37+
38+
pub(crate) fn option_f32<'de, D: Deserializer<'de>>(
39+
deserializer: D,
40+
) -> Result<Option<f32>, D::Error> {
41+
// serde_json also reads an `f32` field as `f64` and narrows it with `as`.
42+
Ok(option_f64(deserializer)?.map(|value| value as f32))
43+
}
44+
}
45+
1146
/// Wire-side view of `params`: the `_meta` map plus the remaining fields.
1247
///
1348
/// All metadata types are transparent wrappers over [`JsonObject`], so the
@@ -848,4 +883,165 @@ mod test {
848883
let output = serde_json::to_value(&req).unwrap();
849884
assert_eq!(input, output);
850885
}
886+
887+
/// The float fields read through [`super::json_float`].
888+
mod json_float {
889+
use std::fmt::Debug;
890+
891+
use rstest::rstest;
892+
use serde::{Deserialize, Serialize, de::DeserializeOwned};
893+
use serde_json::{Value, json};
894+
895+
use crate::model::{
896+
Annotations, CreateMessageRequestParams, ModelPreferences, NumberSchema,
897+
ProgressNotificationParam,
898+
};
899+
900+
/// Untagged, so serde buffers the input before `T` reads it, as it
901+
/// does for every JSON-RPC message.
902+
#[derive(Deserialize)]
903+
#[serde(untagged)]
904+
enum Buffered<T> {
905+
Inner(T),
906+
}
907+
908+
/// Decodes `text` directly and through serde's buffer, checks that both
909+
/// agree, and that the value survives a round trip.
910+
fn decode<T>(text: &str) -> T
911+
where
912+
T: DeserializeOwned + Serialize + PartialEq + Debug,
913+
{
914+
let direct: T = serde_json::from_str(text).unwrap();
915+
let Buffered::Inner(buffered) = serde_json::from_str(text).unwrap();
916+
assert_eq!(direct, buffered, "{text}");
917+
let encoded = serde_json::to_string(&direct).unwrap();
918+
assert_eq!(
919+
serde_json::from_str::<T>(&encoded).unwrap(),
920+
direct,
921+
"{encoded}"
922+
);
923+
direct
924+
}
925+
926+
fn rejects<T: DeserializeOwned>(text: &str) {
927+
assert!(serde_json::from_str::<T>(text).is_err(), "{text}");
928+
assert!(serde_json::from_str::<Buffered<T>>(text).is_err(), "{text}");
929+
}
930+
931+
#[rstest]
932+
#[case::decimal("0.6", 0.6)]
933+
#[case::trailing_zero("0.60", 0.6)]
934+
#[case::exponent("6e-1", 0.6)]
935+
#[case::integer("1", 1.0)]
936+
#[case::integral_decimal("1.0", 1.0)]
937+
#[case::zero("0", 0.0)]
938+
fn fields_read_every_spelling(#[case] n: &str, #[case] expected: f64) {
939+
// An `f32` field holds the `f64` narrowed, as serde_json reads it.
940+
let narrowed = Some(expected as f32);
941+
942+
let annotations: Annotations = decode(&format!(r#"{{"priority":{n}}}"#));
943+
assert_eq!(annotations.priority, narrowed);
944+
945+
let progress: ProgressNotificationParam = decode(&format!(
946+
r#"{{"progressToken":1,"progress":{n},"total":{n}}}"#
947+
));
948+
assert_eq!(progress.progress, expected);
949+
assert_eq!(progress.total, Some(expected));
950+
951+
let params: CreateMessageRequestParams = decode(&format!(
952+
r#"{{"messages":[],"maxTokens":1,"temperature":{n}}}"#
953+
));
954+
assert_eq!(params.temperature, narrowed);
955+
956+
let preferences: ModelPreferences = decode(&format!(
957+
r#"{{"costPriority":{n},"speedPriority":{n},"intelligencePriority":{n}}}"#
958+
));
959+
assert_eq!(preferences.cost_priority, narrowed);
960+
assert_eq!(preferences.speed_priority, narrowed);
961+
assert_eq!(preferences.intelligence_priority, narrowed);
962+
963+
let schema: NumberSchema = decode(&format!(
964+
r#"{{"type":"number","minimum":{n},"maximum":{n},"default":{n}}}"#
965+
));
966+
assert_eq!(schema.minimum, Some(expected));
967+
assert_eq!(schema.maximum, Some(expected));
968+
assert_eq!(schema.default, Some(expected));
969+
}
970+
971+
#[test]
972+
fn decimal_narrows_to_the_nearest_f32() {
973+
let annotations: Annotations = decode(r#"{"priority":0.6}"#);
974+
assert_eq!(annotations.priority, Some(0.6_f32));
975+
}
976+
977+
#[rstest]
978+
#[case::missing(None)]
979+
#[case::null(Some(Value::Null))]
980+
fn optional_fields_read_absent_as_none(#[case] value: Option<Value>) {
981+
// `base` with each of `keys` set to `value`, or left out.
982+
let with = |mut base: Value, keys: &[&str]| {
983+
if let Some(value) = &value {
984+
for key in keys {
985+
base[*key] = value.clone();
986+
}
987+
}
988+
base.to_string()
989+
};
990+
991+
let annotations: Annotations = decode(&with(json!({}), &["priority"]));
992+
assert_eq!(annotations, Annotations::default());
993+
994+
let progress: ProgressNotificationParam = decode(&with(
995+
json!({"progressToken": 1, "progress": 0}),
996+
&["total"],
997+
));
998+
assert_eq!(progress.total, None);
999+
1000+
let params: CreateMessageRequestParams = decode(&with(
1001+
json!({"messages": [], "maxTokens": 1}),
1002+
&["temperature"],
1003+
));
1004+
assert_eq!(params.temperature, None);
1005+
1006+
let preferences: ModelPreferences = decode(&with(
1007+
json!({}),
1008+
&["costPriority", "speedPriority", "intelligencePriority"],
1009+
));
1010+
assert_eq!(preferences, ModelPreferences::new());
1011+
1012+
let schema: NumberSchema = decode(&with(
1013+
json!({"type": "number"}),
1014+
&["minimum", "maximum", "default"],
1015+
));
1016+
assert_eq!(schema, NumberSchema::new());
1017+
}
1018+
1019+
#[test]
1020+
fn progress_is_required() {
1021+
rejects::<ProgressNotificationParam>(r#"{"progressToken":1}"#);
1022+
rejects::<ProgressNotificationParam>(r#"{"progressToken":1,"progress":null}"#);
1023+
}
1024+
1025+
#[rstest]
1026+
#[case::string(r#""high""#)]
1027+
#[case::out_of_range("1e400")]
1028+
fn fields_reject_anything_but_a_finite_number(#[case] n: &str) {
1029+
rejects::<Annotations>(&format!(r#"{{"priority":{n}}}"#));
1030+
rejects::<ProgressNotificationParam>(&format!(
1031+
r#"{{"progressToken":1,"progress":{n}}}"#
1032+
));
1033+
rejects::<ProgressNotificationParam>(&format!(
1034+
r#"{{"progressToken":1,"progress":0,"total":{n}}}"#
1035+
));
1036+
rejects::<CreateMessageRequestParams>(&format!(
1037+
r#"{{"messages":[],"maxTokens":1,"temperature":{n}}}"#
1038+
));
1039+
for key in ["costPriority", "speedPriority", "intelligencePriority"] {
1040+
rejects::<ModelPreferences>(&format!(r#"{{"{key}":{n}}}"#));
1041+
}
1042+
for key in ["minimum", "maximum", "default"] {
1043+
rejects::<NumberSchema>(&format!(r#"{{"type":"number","{key}":{n}}}"#));
1044+
}
1045+
}
1046+
}
8511047
}

0 commit comments

Comments
 (0)