diff --git a/= b/= new file mode 100644 index 000000000..e69de29bb diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index be911f7f9..fda203bab 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -1230,6 +1230,7 @@ pub struct DiscoverResult { /// How long clients may consider this response fresh, in milliseconds. pub ttl_ms: u64, /// Whether the cached result may be shared across authorization contexts. + #[serde(default, deserialize_with = "deserialize_cache_scope_non_optional")] pub cache_scope: CacheScope, /// Protocol-level response metadata. #[serde(rename = "_meta", skip_serializing_if = "Option::is_none")] @@ -1596,7 +1597,45 @@ pub enum CacheScope { Private, } -/// Normalize a `ttlMs` value during deserialization. +/// Normalize a `cacheScope` value during deserialization (non-optional). +/// +/// Per SEP-2549, `cacheScope` MUST be `"public"`, `"private"`, or omitted. +/// Some servers send an empty string `""`; this tolerates that case by +/// defaulting to `CacheScope::Public` (the spec default). +fn deserialize_cache_scope_non_optional<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + let value = Option::::deserialize(deserializer)?; + match value.as_deref() { + None | Some("") => Ok(CacheScope::Public), + Some(s) => match s { + "public" => Ok(CacheScope::Public), + "private" => Ok(CacheScope::Private), + _ => Err(serde::de::Error::unknown_variant(s, &["public", "private"])), + }, + } +} + +/// Normalize a `cacheScope` value during deserialization (optional). +/// +/// Per SEP-2549, `cacheScope` MUST be `"public"`, `"private"`, or omitted. +/// Some servers send an empty string `""`; this tolerates that case by +/// treating it as `None` (absent) rather than erroring. +fn deserialize_cache_scope<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let value = Option::::deserialize(deserializer)?; + match value.as_deref() { + None | Some("") => Ok(None), + Some(s) => match s { + "public" => Ok(Some(CacheScope::Public)), + "private" => Ok(Some(CacheScope::Private)), + _ => Err(serde::de::Error::unknown_variant(s, &["public", "private"])), + }, + } +} /// /// Per SEP-2549, `ttlMs` MUST be `>= 0`; if a server returns a negative value, /// clients SHOULD treat it as `0` (immediately stale). This tolerates that case @@ -1646,7 +1685,12 @@ macro_rules! paginated_result { /// Scope describing who may cache this result (SEP-2549). /// Required by spec version 2026-07-28, but optional here to maintain compatibility /// with older spec versions. - #[serde(default, skip_serializing_if = "Option::is_none")] + /// Empty string is tolerated as `None` (absent) per SEP-2549. + #[serde( + default, + deserialize_with = "deserialize_cache_scope", + skip_serializing_if = "Option::is_none" + )] pub cache_scope: Option, pub $i_item: $t_item, } @@ -1796,7 +1840,11 @@ pub struct ReadResourceResult { /// Scope describing who may cache this result (SEP-2549). /// Required by spec version 2026-07-28, but optional here to maintain compatibility /// with older spec versions. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "deserialize_cache_scope", + skip_serializing_if = "Option::is_none" + )] pub cache_scope: Option, /// The actual content of the resource pub contents: Vec, diff --git a/crates/rmcp/src/transport/common/client_side_sse.rs b/crates/rmcp/src/transport/common/client_side_sse.rs index e668d63df..c35659692 100644 --- a/crates/rmcp/src/transport/common/client_side_sse.rs +++ b/crates/rmcp/src/transport/common/client_side_sse.rs @@ -227,7 +227,15 @@ impl SseRetryPolicy for ExponentialBackoff { { return None; } - Some(self.base_duration * (2u32.pow(current_times as u32))) + // Saturate the multiplier to avoid u32 overflow when current_times >= 32. + // Without saturation, 2u32.pow(32) panics in debug and wraps in release, + // causing either a panic or an unexpectedly short backoff delay. + let multiplier = if current_times >= 32 { + u32::MAX + } else { + 2u32.pow(current_times as u32) + }; + Some(self.base_duration * multiplier) } } @@ -775,4 +783,34 @@ mod tests { assert!(stream.next().await.is_none()); assert_eq!(attempts.load(Ordering::Relaxed), 0); } + + #[test] + fn exponential_backoff_saturates_at_high_retry_count() { + let backoff = ExponentialBackoff { + max_times: None, + base_duration: Duration::from_millis(1000), + }; + + // Normal case: 2^5 = 32 seconds + assert_eq!(backoff.retry(5), Some(Duration::from_millis(32000))); + + // At current_times=32, 2u32.pow(32) would overflow. + // Should saturate to u32::MAX instead of panicking. + let result = backoff.retry(32); + assert!(result.is_some(), "should still return Some at retry 32"); + assert_eq!( + result, + Some(Duration::from_millis(1000) * u32::MAX), + "should saturate at u32::MAX multiplier" + ); + + // Even higher values should still work (saturated) + let result = backoff.retry(100); + assert!(result.is_some(), "should still return Some at retry 100"); + assert_eq!( + result, + Some(Duration::from_millis(1000) * u32::MAX), + "should remain saturated at u32::MAX" + ); + } }