fix: prevent CallToolResult and GetTaskPayloadResult from shadowing CustomResult in untagged enums (#771)

The `#[serde(default)]` on `CallToolResult.content` (added in #752) made
all fields optional, causing `CallToolResult` to greedily match any JSON
object during `#[serde(untagged)]` deserialization of `ServerResult`.
Similarly, `GetTaskPayloadResult(Value)` matched everything before
`CustomResult(Value)` could be reached.

Fix by replacing derived `Deserialize` impls with custom ones:
- `CallToolResult`: require at least one known field to be present
- `GetTaskPayloadResult`: always fail (indistinguishable from
  `CustomResult` in JSON; construct programmatically via `::new()`)
This commit is contained in:
Dale Seo 2026-03-23 15:20:55 -04:00 committed by GitHub
parent 30cdc38c9a
commit c8c0c0cffc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 63 additions and 2 deletions

View file

@ -2691,7 +2691,7 @@ pub type ElicitationCompletionNotification =
///
/// Contains the content returned by the tool execution and an optional
/// flag indicating whether the operation resulted in an error.
#[derive(Default, Debug, Serialize, Deserialize, Clone, PartialEq)]
#[derive(Default, Debug, Serialize, Clone, PartialEq)]
#[serde(rename_all = "camelCase")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[non_exhaustive]
@ -2710,6 +2710,48 @@ pub struct CallToolResult {
pub meta: Option<Meta>,
}
// Custom Deserialize implementation that:
// 1. Defaults `content` to `[]` when the field is missing (lenient per Postel's law)
// 2. Requires at least one known field to be present, so that `CallToolResult` doesn't
// greedily match arbitrary JSON objects when used inside `#[serde(untagged)]` enums
// (e.g. `ServerResult`), which would shadow `CustomResult`.
impl<'de> Deserialize<'de> for CallToolResult {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
#[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
struct Helper {
content: Option<Vec<Content>>,
structured_content: Option<Value>,
is_error: Option<bool>,
#[serde(rename = "_meta")]
meta: Option<Meta>,
}
let helper = Helper::deserialize(deserializer)?;
if helper.content.is_none()
&& helper.structured_content.is_none()
&& helper.is_error.is_none()
&& helper.meta.is_none()
{
return Err(serde::de::Error::custom(
"expected at least one known CallToolResult field \
(content, structuredContent, isError, or _meta)",
));
}
Ok(CallToolResult {
content: helper.content.unwrap_or_default(),
structured_content: helper.structured_content,
is_error: helper.is_error,
meta: helper.meta,
})
}
}
impl CallToolResult {
/// Create a successful tool result with unstructured content
pub fn success(content: Vec<Content>) -> Self {

View file

@ -123,7 +123,7 @@ pub struct GetTaskResult {
/// (e.g., `CallToolResult` for `tools/call`). This is represented as
/// an open object. The payload is the original request's result
/// serialized as a JSON value.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[derive(Debug, Clone, PartialEq, Serialize)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[non_exhaustive]
pub struct GetTaskPayloadResult(pub Value);
@ -135,6 +135,25 @@ impl GetTaskPayloadResult {
}
}
// Custom Deserialize that always fails, so that `GetTaskPayloadResult` is skipped
// during `#[serde(untagged)]` enum deserialization (e.g. `ServerResult`).
// The payload has the same JSON shape as `CustomResult(Value)`, so they are
// indistinguishable. `CustomResult` acts as the catch-all instead.
// `GetTaskPayloadResult` should be constructed programmatically via `::new()`.
impl<'de> serde::Deserialize<'de> for GetTaskPayloadResult {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
// Consume the value so the deserializer state stays consistent.
serde::de::IgnoredAny::deserialize(deserializer)?;
Err(serde::de::Error::custom(
"GetTaskPayloadResult cannot be deserialized directly; \
use CustomResult as the catch-all",
))
}
}
/// Response to a `tasks/cancel` request.
///
/// Per spec, `CancelTaskResult = allOf[Result, Task]` — same shape as `GetTaskResult`.